Add rootAttributes rendering option

[dirkluijk]

trafficstars

Hi,

I want to set the root attributes of a menu from inside a Twig template. I am working on a project where different themes need to have different CSS classes for the menu, and it seems quite ugly to override the MenuBuilder just to change the childAttributes of the root item.

My suggestion is to add a rootAttributes option to the default renderers.

Example usage (Twig renderer):

{# render Bootstrap nav #}
{{ knp_menu_render('main', {
    'currentClass': 'active',
    'rootAttributes': { class: 'nav navbar-nav' }
}) }}

If you agree, I could submit a PR with all necessary changes to the default renderers, docs and twig template.

[Nek-]

:+1:

[jeremylivingston]

:+1:

I've searched up and down for a way to add a class to the root element from knp_menu_render, but come up with nothing. Is this truly not possible currently?

It seems like this should be a standard feature of the library. If this capability does not exist, I would definitely encourage the addition of a rootAttributes option.

I'd be happy to do the work to accomplish this, if necessary.

[dcarbone]

It seems the main issues are here:

• https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Renderer/ListRenderer.php#L46 and
• https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Resources/views/knp_menu.html.twig#L18

This means that even if you define a Builder as such:

$menu = $factory->createItem('root', $options);

 $menu->setAttribute('class', 'root-class');
 $menu->setAttribute('id', 'root_id');
 $menu->setChildrenAttribute('class', 'child-class');

... the $menu->setAttribute() call is utterly ignored, instead passing an array of the children's attributes into:

• https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Renderer/ListRenderer.php#L55 or
• https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Resources/views/knp_menu.html.twig#L25

I assume this was done to enable root-level setting of child attributes, and it can be overcome fairly easily with a custom renderer or by extending the base twig template. If this is not the reason why, I am definitely curious to hear it.

[stof]

This is because attributes are for the <li> tag and childrenAttributes are for the <ul> tag with the list of children. For the root node, there is no <li> tag, but we decided to keep consistency by always using childrenAttributes for the <ul> even at the root.

[dcarbone]

@stof: Ah excellent! Thank you for the clarification. Is this mentioned somewhere in documentation that I missed?

[stof]

see the note in https://github.com/KnpLabs/KnpMenu/blob/master/doc/01-Basic-Menus.md#menu-attributes

[dcarbone]

Thank you, sorry for commenting on such an old issue.

[aur1mas]

wether problem was solved? because I have same use case as @dirkluijk mentioned. And from the code I see that is impossible to pass attributes from twig.

[stof]

@aur1mas knp_menu_render options are renderer options. These options will not alter the menu being rendered in any way (I will keep rejected any PR trying to do this). There are already several way to influence the menu attributes:

• changing the children attributes of the root menu from the template:

  {% set menu = knp_menu_get('main') %}
  {% do menu.setChildrenAttribute('class', 'child-class') %}
  {% knp_menu_render(menu, {'activeClass': 'active'}) %}
  

• passing an option to the menu builder and use it in the builder to change the way the menu is built

  {% set menu = knp_menu_get('main', options={'root_class': 'child-class'}) %}
  {% knp_menu_render(menu, {'activeClass': 'active'}) %}
  

  class MenuBuilder
  {
      // ...
  
      public function buildMainMenu(array $options)
      {
          $menu = $this->factory->createItem('main', array(
              'childrenAttributes' => array(
                  'class' => isset($options['root_class']) ? $options['root_class'] : null,
              ),
          ));
  
          // Continue building the menu
  
          return $menu;
      }
  }
  

Note that the second way does not work when registering menu themselves as services (which has several drawbacks, including this one). You need to use either the convention-based provider (AppBundle:MenuBuilder:buildMainMenu here) or the new way to register builders as service once https://github.com/KnpLabs/KnpMenuBundle/pull/273 will be merged and released (hint: very soon)

[Seb33300]

👍 Need an option to override the root class from renderer function

[stof]

I gave 2 solutions just above, which don't require new features (and don't require adding new responsibilities in the renderer)

[martixy]

I myself tried to do this just now. I was setting the attributes option on the root component of my menu(the ul), and kept wondering why its not working. After looking at the code I discovered that the it doesn't render from attributes, but from childrenAttributes for some reason. https://github.com/KnpLabs/KnpMenu/blob/e11c6cdfda1d8e5964960d6c3ed517ce68ba5bf5/src/Knp/Menu/Resources/views/knp_menu.html.twig#L17-L29

I have 2 questions:

1. What are attributes used for?
2. What does the children part of childrenAttributes refer to? Children of what?

IMO documenting the ItemInterface at least would be a good idea.

[stof]

@martixy the answer to this question is given in a previous comment in this discussion: https://github.com/KnpLabs/KnpMenu/issues/166#issuecomment-72045304

[bennukem]

{% set menu = knp_menu_get('main') %} {% do menu.setChildrenAttribute('class', 'child-class') %} {% knp_menu_render(menu, {'activeClass': 'active'}) %}

If menu as a service and need to be used with two different ways, by exemple one render with class on ul and do nothing on the other render. do menu.setChildrenAttribute() will work on the service and no on only the display.

Actually the only way is to make a template for each render with special need... :(