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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dale42 created an issue. See original summary.

dale42’s picture

Here are patches for what I envision each option looking like.

joelpittet’s picture

Would need to think about this more but my gut is to remove menu specific regions.