Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OptOutManager #8229

Merged
merged 4 commits into from Jul 12, 2015
Merged

Add OptOutManager #8229

merged 4 commits into from Jul 12, 2015

Conversation

Zeichen32
Copy link
Contributor

In relation to PR #7979

This OptOut Manager allows plugins to add Javascripts and Stylesheets to the OptOut View.

  • Add a new OptOutManager
  • Remove opt out logic from CoreAdminHome Controller
  • Change OptOut View

@Zeichen32 Zeichen32 mentioned this pull request Jun 26, 2015
@mnapoli
Copy link
Contributor

mnapoli commented Jun 26, 2015

Could you sum up how plugins would add CSS and JS to the form? I see the OptOutManager:: addStylesheet() and OptOutManager:: addJavascript() methods but where/when are plugins supposed to call them?

@Zeichen32
Copy link
Contributor Author

My current idea is to create a EventListener for the "Controller.CoreAdminHome.optOut" Event.
But maybe it is better to dispatch a seperate event?

    /**
     * @see Piwik\Plugin::getListHooksRegistered
     */
    public function getListHooksRegistered()
    {
        return array(
            'Controller.CoreAdminHome.optOut' => 'addOptOutStyles'
        );
    }

    public function addOptOutStyles()
    {
        StaticContainer::get('Piwik\Plugins\CoreAdminHome\OptOutManager')
            ->addStylesheet('body { background: black; }');
    }

* @return View
* @throws \Exception
*/
public function createView()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if the code of this method could be as well in the controller? Or is it supposed to be here so one can override it? Do you need this feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, i didn't need to to override the optout manager, but the "createView" method include the whole optout logic. In my opinion the right place for this code is in the OptOut Manager, not the controller.

The other reason because i have moved the code into the OptOut Manager is that plugins can extends or use the OptOut Manager for their own needs.

@tsteur
Copy link
Member

tsteur commented Jun 28, 2015

It will be nice to have an OptOutManager soon. With Piwik 3.0 we should even consider making it public API IMO.

@Zeichen32
Copy link
Contributor Author

@tsteur I have added a new commit.

@tsteur
Copy link
Member

tsteur commented Jul 1, 2015

Cheers! @mattab @mnapoli @diosmosis does any of you mind to have a quick look as well?

@Zeichen32 Zeichen32 changed the title [WIP] Add OptOutManager Add OptOutManager Jul 6, 2015
@mattab mattab added this to the 2.14.1 milestone Jul 7, 2015
@mattab
Copy link
Member

mattab commented Jul 12, 2015

Looks good to me 👍

@Zeichen32 you will be able to test this in 2.14.1-b1

mattab pushed a commit that referenced this pull request Jul 12, 2015
@mattab mattab merged commit 286a5fb into matomo-org:master Jul 12, 2015
@Zeichen32
Copy link
Contributor Author

👍 thanks for merge 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants