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

Make it easier to define URLs for Menu and remove an unnessary parameter #6140

Closed
tsteur opened this Issue Sep 6, 2014 · 0 comments

Comments

Projects
None yet
1 participant
@tsteur
Member

tsteur commented Sep 6, 2014

Already for a while I do not like the way we are defining the URL of a menu item as well as by the $displayForCurrentUser param of the MenuAbstract::add() method:

 $urlParams = array(
     'module' => 'Actions',
     'action' => 'menuGetPageUrls'
);
$menu->add('Menu', 'Submenu', $urlParams, $displayForCurrentUser = true, 15);

I just wanted to write a blog post about Controller + Menu and noticed this ones more so I am going to make this easier now. Therefore I will deprecate the add() method in favor of new addItem() method which no longer has the $displayForCurrentUser argument. The add() method will be removed in Piwik 3.0 I think. It is easy to stay backwards compatible here. Added a test for this.

One should use an if condition instead of the boolean parameter:

if (Piwik::isUserNotAnonymous()) {
   $menu->addItem('Menu', 'Submenu', $urlParams, 15);
}

To make the URL parameter easier I am going to introduce three new methods:

urlForDefaultAction($additionalParams = array())
urlForAction($controllerAction, $additionalParams = array())
urlForModuleAction($module, $controllerAction, $additionalParams = array())

The first two methods will detect the current module automatically so we remove a lot of duplicated code. The third method will make sure the module actually exist and is activated. The methods are also better readable than the array structure and allows us to change the generated array in the future without having to change the plugins.

Example:

$menu->addItem('Menu', 'Submenu', $this->urlForAction('menuGetPageUrls'), 15);

Note: A while ago I added also new methods to no longer having to know the translation keys of menu items so it is even shorter:

$menu->addManageItem('Title', $this->urlForAction('menuGetPageUrls'), 15);
$menu->addActionsItem('Title', $this->urlForAction('menuGetPageUrls'), 15);
...

tsteur added a commit that referenced this issue Sep 6, 2014

refs #6140 easier way to define URLs for menu items and introducing a…
… method to addItem without boolean parameter

tsteur added a commit that referenced this issue Sep 6, 2014

tsteur added a commit that referenced this issue Sep 6, 2014

@tsteur tsteur added Enhancement and removed Enhancement labels Sep 6, 2014

@tsteur tsteur modified the milestones: Mid term, Piwik 2.7.0 Sep 6, 2014

@tsteur tsteur self-assigned this Sep 6, 2014

@tsteur tsteur closed this Sep 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment