Set current uri #55
Set current uri #55
Conversation
Also, the depth default value is now 1 instead of null, which is IMO the most common use case.
* Default of the depth option is back to "null"
To make adding the class current to the current menu item work, there needs to be a way to inform the menu of the current request uri. This can be done, when the menu is fetched from the menu provider in the helper.
| @@ -37,7 +37,9 @@ class MenuHelper extends Helper implements \ArrayAccess | |||
| */ | |||
| public function get($name) | |||
| { | |||
| return $this->provider->getMenu($name); | |||
| $menu = $this->provider->getMenu($name); | |||
| $menu->setCurrentUri($this->container->get('request')->getRequestUri()); | |||
There was a problem hiding this comment.
this causes an issue when rendering the menu in a subrequest as the request uri will be the wrong one and you force using it.
There was a problem hiding this comment.
Would it make sense to make this configurable? I think there are cases where no subrequests are used ... or is there a better way to find out the original uri?
There was a problem hiding this comment.
From a subrequest, you cannot know the main request uri, even more when the subrequest is delayed by using ESI.
There was a problem hiding this comment.
So what do you think about making that configurable? From what you said I guess marking the current menu item automatically will never work when using sub request.
There was a problem hiding this comment.
did you find a solution to the issue? or how could an ESI sub request for the menu with correct highlighting work? pass a magical parameter to highlight the right entry? maybe we could add support for passing the highlighted path as a get parameter with configurable name?...
There was a problem hiding this comment.
yes, makes more sense than randomly adding something and the users of the lib needing to roll their own thing. shall we create an issue on the issue tracker for this topic?
There was a problem hiding this comment.
yeah probably. It would be easier to track it that way than commenting on another PR.
There was a problem hiding this comment.
I think this PR is just about lazyness to avoid having to set the current uri when building the menu :)
This PR extends #53 to automatically set the current uri from the request in MenuHelper.