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.
Problem/Motivation
Issue 1: Code for non-existent regions
Basic contains the following twig code in page.html.twig:
{% set main_menu = page.primary_menu|render %}
{% set secondary_menu = page.secondary_menu|render %}
{% if main_menu or secondary_menu %}
<nav id="navigation" class="menu{% if main_menu %} with-primary{% endif %}{% if secondary_menu %} with-secondary{% endif %}">
<div class="container">
{{ main_menu }}
{{ secondary_menu }}
</div>
</nav><!-- /#navigation -->
{% endif %}
But it does not implement the primary_menu or secondary_menu regions to make use of the code.
Issue 2: Menu blocks already implement the <nav> tag
Since menu blocks already implement the nav tag, it is not required in the region's wrapper html.
Proposed resolution
The simplest solution is removing the twig navigation region html and associated SASS/CSS code.
The solution I prefer is fully implementing the primary_menu or secondary_menu regions. My reasons are:
- The Drupal core implementation templates and core front-end themes provide regions for Primary and Secondary menus
- I've been exploring themes and had downloaded a few to look at. 3 of the 4 implemented Primary and Secondary menu regions. The 4th implemented a Navigation region. It appears to be the more common experience. Without these regions Basic may not do as well in comparisons because of a perceived deficit.
- People coming from Drupal 7 will have an expectation of menu handling
- It's easier deleting the regions if not required than it is adding them
- It's a kinder starting point for novice themers
- For experienced themers it's arguably no more of a burden than the current situation
If this is the agreed direction, the double <nav> tag issue would also be fixed.
I think there is no clear "correct" answer and discussion is warranted.
Comment | File | Size | Author |
---|---|---|---|
#2 | basic-menu-regions-2901386-2-no-nav.patch | 2.08 KB | dale42 |
#2 | basic-menu-regions-2901386-2-menu-region.patch | 1.97 KB | dale42 |
Comments
Comment #2
dale42Here are patches for what I envision each option looking like.
Comment #3
joelpittetWould need to think about this more but my gut is to remove menu specific regions.