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

How to display children, if parent is current. #84

Closed
catchamonkey opened this issue Dec 5, 2012 · 8 comments
Closed

How to display children, if parent is current. #84

catchamonkey opened this issue Dec 5, 2012 · 8 comments

Comments

@catchamonkey
Copy link

Hi,

Loving the library here, stopped me porting my Symfony 1.4 plugin over so thanks!

One thing I can't work out though, is if you define a few menu items, and some of them have children, how can you render the children of only the current menu item?

$factory = new MenuFactory();
$menu = $factory->createItem('My menu');
$menu->addChild('Home', array('uri' => '/'));
$menu->addChild('Comments');

$menu['Comments']->setUri('/comments');
$menu['Comments']->addChild('My comments', array('uri' => '/my_comments'));

I only want My Comments (and it's siblings ul) to be rendered if Comments is the active menu item.

Any ideas?

@catchamonkey
Copy link
Author

From my tests it appears that the item in question has not been evaluated as current yet by the time the children are rendered. Is that correct?

@catchamonkey
Copy link
Author

I've found a way to do it, just need to find a way to make it a config option.

If you change line 74 of the twig template from

https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Resources/views/knp_menu.html.twig#L74

to

{% if matcher.isCurrent(item) %}
    {{ block('list') }}
{% endif %}

Then the children are only displayed for current parent items.
But at this stage the children have all been evaluated which is wasteful.

Your thoughts?

@catchamonkey
Copy link
Author

Scratch that, that has a problem, as soon as you select a child item, the parent is no longer current and they are not rendered.

Hmmmm.

So what is really needed is to render children if any of the following are true.

  • Parent is current
  • Child is current
  • Any sibling is current

@catchamonkey
Copy link
Author

So this is working, but how is best to make the functionality part of the core menu code?
This involves iterating the children in advance of rendering the parent item so it could have performance issues with large menu structures.

{% set childIsActive = false %}
{% for child in item.children if matcher.isCurrent(child) %}
    {% set childIsActive = true %}
{% endfor %}
{% if matcher.isCurrent(item) or childIsActive %}
    {{ block('list') }}
{% endif %}

@stof
Copy link
Collaborator

stof commented Dec 5, 2012

@catchamonkey the matcher has isAncestor to check if it is the ancestor of a current item: https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Matcher/MatcherInterface.php

@catchamonkey
Copy link
Author

@stof Hi,

Thanks for that, much cleaner here;

{% if matcher.isCurrent(item) or matcher.isAncestor(item) %}
    {{ block('list') }}
{% endif %}

Would be great to make that a configurable part of the menu system, works better for nested nav in some contexts.

@catchamonkey
Copy link
Author

Hi @stof I'm going to put in a PR to add this to the menu item so it's available to all.
Will add Tests and update docs before I submit it, but here it is so far.

https://github.com/catchamonkey/KnpMenu/compare/display_children_if_ancestor_current

catchamonkey added a commit to catchamonkey/KnpMenu that referenced this issue Dec 5, 2012
 children of a menu item, if the item itself, or the ancestor
 is 'current'.

Closes KnpLabs#84
@catchamonkey
Copy link
Author

PR opened, closing this now.

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 a pull request may close this issue.

2 participants