Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Follow-up to #2443361: Remove theme_book_link, make book tree align with MenuLinkTree build, where the need for this became obvious.
Problem/Motivation
The template file core/modules/system/templates/menu.html.twig includes this twig macro:
{% macro menu_links(items, attributes, menu_level) %}
{% import _self as menus %}
{% if items %}
{% if menu_level == 0 %}
<ul{{ attributes.addClass('menu') }}>
{% else %}
<ul class="menu">
{% endif %}
{% for item in items %}
<li{{ item.attributes }}>
{{ link(item.title, item.url) }}
{% if item.below %}
{{ menus.menu_links(item.below, attributes, menu_level + 1) }}
{% endif %}
</li>
{% endfor %}
</ul>
{% endif %}
{% endmacro %}This is very useful for rendering not only menus, but any tree of links. However, this macro is not currently reusable, as this file is actually a template.
We could all make good use of the code if we tackle this, both within core and contrib.
See documentation on twig macros: http://twig.sensiolabs.org/doc/tags/macro.html
Proposed resolution
Exact solution is up for discussion. But any solution must be re-usable by book-tree.html.twig and menu--toolbar.html.twig at the very least.
Remaining tasks
- Generalize the macro. Ensure backwards-compatibility with old version.
- Figure out where else we can use this in core. For now we know:
- core/modules/book/templates/book-tree.html.twig
- core/modules/system/templates/menu.html.twig
- core/modules/toolbar/templates/menu--toolbar.html.twig
- + core/themes/stable versions of the above
- core/themes/classy/templates/navigation/book-tree.html.twig
- core/themes/classy/templates/navigation/menu.html.twig
- core/themes/classy/templates/navigation/menu--toolbar.html.twig
- Make the changes on all those places.
User interface changes
None
API changes
None, adding a general core twig macro, which means a new tool for themers to use.
Beta phase evaluation
| Issue category | Bug/Task/Feature because ... |
|---|---|
| Issue priority | Major because ... Critical/Not critical because ... |
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 2490308-12-menu-macro.patch | 2.28 KB | JohnAlbin |
Comments
Comment #1
joelpittetSince all other template files are defined via
hook_theme()and this macro would likely live outside of that? we may need to explain how people can override it by likely copying it to their template and make sure the @bartik/menu-macro.html.twig macro would work the same way if possible.Thanks for making this follow-up @Manuel Garcia!
I think it should work well. Still debating the attributes parameter in my head...
Comment #2
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer commentedComment #3
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer commentedYeah it would be outside of hook_theme... this would be just a twig macro. I could see other modules using it as well, think taxonomy_menu for example.
Comment #4
star-szrJust want to mention, based on all the template overrides of this already the macro would still need to support setting classes on the top level ul, that seems to be the primary thing that changes, at least in core :)
Comment #5
joelpittetGood point, @sqndr were pondering that too.
Anybody in contrib could just replace the macro include with their own which is cool if they want additional functionality but we should cover our core use case like @Cottser mentioned.
Comment #6
Wim Leers#4++ — exactly!
Comment #7
davidhernandezSo, if this thing exists essentially outside the theme registry, where does it live? If someone wants to change it, they need to hack core to change the import path or override every template that uses it just to change all the paths.
...or are you talking about having one per theme? That sounds a bit less reusable.
Comment #8
star-szrHaven't thought about this too much but we may actually want to just add it to the theme registry and then let the registry loader load it so it can be overridden like you would any other template.
If we don't want to do that we should still be able to use the
@namespace/menu-macro.html.twigapproach.Comment #9
joelpittetI could be wrong but I think we may be able to tackle this in 8.1.x if not then 9 depending on the scope of this.
Comment #10
lauriiiComment #12
JohnAlbinThe docs for importing macros say you can put macros in templates. http://twig.sensiolabs.org/doc/tags/import.html
I just tested this and it works fine:
So that solves that problem. The macro can stay where it is and can be overridden like any other theme hook.
I actually wrote a much more extensible menu macro a couple days ago for a project, but I threw it away in favor of using a more powerful recursive template that includes twig blocks. Here it is in case someone finds it useful.
Comment #14
JohnAlbinThe macro needs to be backwards compatible.