Changing the HTML strcture of a menu should be easy

[gremo]

ATM if you need to change the HTML structure - i.e. from ul/li to div/div - you need to copy and past the whole knp_menu.html.twig. For the list block this is a matter of a few lines, but for the itemblock there a lot of lines.

This seems uncomfortable to me. How about adding a block for just print the HTML tag? This way when knp_menu.html.twig will be update my menu will be update too.

[dbu]

semantically the ul / li seems the correct thing to do. but if there is a good use case for more flexibility, we should make the template more flexible, yes. probably like its done for ul already, see knp_menu_ordered.html.twig can you explain why you need div? and does needing div not mean that the rest of your template also needs to be specific because you use the menu in some different way?

[gremo]

Hi @dbu, I agree that generally the ul/li is the common way to rendere a menu. However sometimes a differente structure is needed, for example when the menu should look like a Bootstrap grid. In that case the root is usually a div.container, the list is a div.row and the element is a div.col-*. If you use a ul/li you need some CSS fixes to make the list act like a grid. Repeat does fixes for every menu that should look like a Bootstrap grid.

Another good reason to make a start/end tag block is to force CSS classes, without writing classes in PHP itself. In this case you can set classes right before opening the tag. Right now I'm using a custom base menu template with 4 new blocks:

{% block listOpenElement %}
{% import _self as knp_menu %}
<ul{{ knp_menu.attributes(listAttributes) }}>
{% endblock %}

{% block listCloseElement %}
</ul>
{% endblock %}

{% block itemOpenElement %}
{% import _self as knp_menu %}
<li{{ knp_menu.attributes(attributes) }}>
{% endblock %}

{% block itemCloseElement %}
</li>
{% endblock %}

[dbu]

@stof how would you feel about a refactoring as gremo proposes? it seems safe enough to me, and would be BC as its just adding new blocks.

[gremo]

Any update on this? Thanks.

[stof]

As the list block only contains the <ul></ul> tag, and calls another block for all the content inside it, I think the existing block is enough.

Adding 2 extra blocks around the opening and closing tags looks overkill to me. And it would have a performance impact: any Twig block involves a function call (actually several ones due to first reaching some Twig internal layer which is then calling the function rendering the block). As these blocks are called a lot when rendering a menu, I'm not sure this is a good idea.