Templating helper now does something with the depth option ;-) #53

Merged
merged 2 commits into from Jul 30, 2011

Conversation

Projects
None yet
5 participants
@benjamindulau
Contributor

benjamindulau commented Jul 27, 2011

Also, the depth default value is now 1 instead of null, which is IMO the most common use case.

When rendering the menu through a twig template, the depth option.
Also, the depth default value is now 1 instead of null, which is IMO the most common use case.
@stof

View changes

Templating/Helper/MenuHelper.php
@@ -89,7 +89,7 @@ class MenuHelper extends Helper implements \ArrayAccess
* @param string $template (optional)
* @return string
*/
- public function render($name, $path = null, $depth = null, $template = null)
+ public function render($name, $path = null, $depth = 1, $template = null)

This comment has been minimized.

@stof

stof Jul 27, 2011

Collaborator

it should not set a default depth but render the whole tree when set to null, to match the behavior of the RendererInterface

@stof

stof Jul 27, 2011

Collaborator

it should not set a default depth but render the whole tree when set to null, to match the behavior of the RendererInterface

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Jul 27, 2011

Collaborator

btw, the $path variable should be removed from the helpers as the initialize method does not exist (and does not seem to have existed anywhere in the history IIRC)

Collaborator

stof commented Jul 27, 2011

btw, the $path variable should be removed from the helpers as the initialize method does not exist (and does not seem to have existed anywhere in the history IIRC)

@rubensayshi

This comment has been minimized.

Show comment
Hide comment
@rubensayshi

rubensayshi Jul 28, 2011

Indeed $path does nothing and the comments on the methods even skip it when defining the @ param's

Indeed $path does nothing and the comments on the methods even skip it when defining the @ param's

* Removed unused "path" option in MenuExtension and MenuHelper
* Default of the depth option is back to "null"
@benjamindulau

This comment has been minimized.

Show comment
Hide comment
@benjamindulau

benjamindulau Jul 28, 2011

Contributor

I have updated the PR.
Default value for the "depth" option is back to "null" and i've removed the unused "path" option

Contributor

benjamindulau commented Jul 28, 2011

I have updated the PR.
Default value for the "depth" option is back to "null" and i've removed the unused "path" option

@uwej711

This comment has been minimized.

Show comment
Hide comment
@uwej711

uwej711 Jul 30, 2011

Contributor

Hi,

i build PR knplabs#55 based on this to automatically set the current uri in the MenuHelper class. Without this PR here the MenuFactory will not work since MenuHelper calls initialize which is not implemented in the MenuItem class that is used by the MenuFactory

Contributor

uwej711 commented Jul 30, 2011

Hi,

i build PR knplabs#55 based on this to automatically set the current uri in the MenuHelper class. Without this PR here the MenuFactory will not work since MenuHelper calls initialize which is not implemented in the MenuItem class that is used by the MenuFactory

@mbontemps mbontemps merged commit 499b7a3 into KnpLabs:master Jul 30, 2011

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