From 364ecc6c1c57341c303ec650670cd72e9d5e62f2 Mon Sep 17 00:00:00 2001
From: Lauri Eskola <lauri.eskola@acquia.com>
Date: Tue, 12 Oct 2021 16:53:51 +0300
Subject: [PATCH] Issue #3225241 by mherchel, andy-blum, Gauravmahlawat, shaal,
jwitkowski79, nod_: Olivero primary menu JS completely breaks if site-builder
specifies over 2 levels deep
---
.../olivero_test/olivero_test.links.menu.yml | 6 +++
.../Tests/Olivero/oliveroDesktopMenuTest.js | 28 +++++++----
.../Tests/Olivero/oliveroMobileMenuTest.js | 4 +-
.../css/components/navigation/nav-primary.css | 11 +++++
.../navigation/nav-primary.pcss.css | 10 ++++
.../navigation/menu--primary-menu.html.twig | 46 ++++++++++---------
6 files changed, 73 insertions(+), 32 deletions(-)
diff --git a/core/modules/system/tests/modules/olivero_test/olivero_test.links.menu.yml b/core/modules/system/tests/modules/olivero_test/olivero_test.links.menu.yml
index ea1eeb9f959b..ed4614a12d1f 100644
--- a/core/modules/system/tests/modules/olivero_test/olivero_test.links.menu.yml
+++ b/core/modules/system/tests/modules/olivero_test/olivero_test.links.menu.yml
@@ -10,6 +10,12 @@ olivero_test.child:
parent: olivero_test.front_page
menu_name: main
weight: 100
+olivero_test.child_child:
+ title: Child link
+ route_name: <front>
+ parent: olivero_test.child
+ menu_name: main
+ weight: 100
olivero_test.button:
title: Button
route_name: <button>
diff --git a/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDesktopMenuTest.js b/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDesktopMenuTest.js
index faca431341d6..b23627d353da 100644
--- a/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDesktopMenuTest.js
+++ b/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroDesktopMenuTest.js
@@ -1,15 +1,26 @@
const headerNavSelector = '#header-nav';
-const linkSubMenuId = 'home-submenu-1';
-const buttonSubMenuId = 'button-submenu-2';
+const linkSubMenuId = 'primary-menu-item-1';
+const buttonSubMenuId = 'primary-menu-item-12';
module.exports = {
'@tags': ['core', 'olivero'],
before(browser) {
- browser.drupalInstall({
- setupFile:
- 'core/tests/Drupal/TestSite/TestSiteOliveroInstallTestScript.php',
- installProfile: 'minimal',
- });
+ browser
+ .drupalInstall({
+ setupFile:
+ 'core/tests/Drupal/TestSite/TestSiteOliveroInstallTestScript.php',
+ installProfile: 'minimal',
+ })
+ // Login and change max-nesting depth so we can verify that menu levels
+ // greater than 2 do not break the site.
+ .drupalLoginAsAdmin(() => {
+ browser
+ .drupalRelativeURL('/admin/structure/block/manage/olivero_main_menu')
+ .waitForElementVisible('[data-drupal-selector="edit-settings-depth"]')
+ .setValue('[data-drupal-selector="edit-settings-depth"]', 'Unlimited')
+ .click('[data-drupal-selector="edit-actions-submit"]')
+ .waitForElementVisible('body');
+ });
browser.resizeWindow(1600, 800);
},
after(browser) {
@@ -32,7 +43,8 @@ module.exports = {
'aria-expanded',
'true',
)
-
+ // Verify tertiary menu item exists.
+ .assert.visible('#primary-menu-item-11 .primary-nav__menu-link--level-3')
// Test interactions for route:<button> menu links.
.assert.not.visible(`#${buttonSubMenuId}`)
.assert.attributeEquals(
diff --git a/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroMobileMenuTest.js b/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroMobileMenuTest.js
index bd0aa3df82a2..50d60b736760 100644
--- a/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroMobileMenuTest.js
+++ b/core/tests/Drupal/Nightwatch/Tests/Olivero/oliveroMobileMenuTest.js
@@ -1,7 +1,7 @@
const mobileNavButtonSelector = 'button.mobile-nav-button';
const headerNavSelector = '#header-nav';
-const linkSubMenuId = 'home-submenu-1';
-const buttonSubMenuId = 'button-submenu-2';
+const linkSubMenuId = 'primary-menu-item-1';
+const buttonSubMenuId = 'primary-menu-item-12';
/**
* Sends arbitrary number of tab keys, and then checks that the last focused
diff --git a/core/themes/olivero/css/components/navigation/nav-primary.css b/core/themes/olivero/css/components/navigation/nav-primary.css
index 6d7acd3b3916..4ef04d9538dd 100644
--- a/core/themes/olivero/css/components/navigation/nav-primary.css
+++ b/core/themes/olivero/css/components/navigation/nav-primary.css
@@ -314,6 +314,17 @@
}
}
+/*
+ * Olivero doesn't officially support nested tertiary submenus, but this
+ * ensures that it doesn't break all the way.
+ *
+ * @see https://www.drupal.org/project/drupal/issues/3221399
+ */
+
+.primary-nav__menu--level-2 .primary-nav__menu-item--has-children {
+ display: block;
+}
+
.primary-nav__menu-link--level-2 {
font-size: 1rem;
font-weight: normal;
diff --git a/core/themes/olivero/css/components/navigation/nav-primary.pcss.css b/core/themes/olivero/css/components/navigation/nav-primary.pcss.css
index b897d9f770bd..d8c68795dab3 100644
--- a/core/themes/olivero/css/components/navigation/nav-primary.pcss.css
+++ b/core/themes/olivero/css/components/navigation/nav-primary.pcss.css
@@ -167,6 +167,16 @@
}
}
+/*
+ * Olivero doesn't officially support nested tertiary submenus, but this
+ * ensures that it doesn't break all the way.
+ *
+ * @see https://www.drupal.org/project/drupal/issues/3221399
+ */
+.primary-nav__menu--level-2 .primary-nav__menu-item--has-children {
+ display: block;
+}
+
.primary-nav__menu-link--level-2 {
font-size: 16px;
font-weight: normal;
diff --git a/core/themes/olivero/templates/navigation/menu--primary-menu.html.twig b/core/themes/olivero/templates/navigation/menu--primary-menu.html.twig
index 0e4869257811..4d2c73bb73ff 100644
--- a/core/themes/olivero/templates/navigation/menu--primary-menu.html.twig
+++ b/core/themes/olivero/templates/navigation/menu--primary-menu.html.twig
@@ -27,11 +27,12 @@
@see https://twig.symfony.com/doc/1.x/tags/macro.html
#}
{% set attributes = attributes.addClass('menu') %}
-{{ menus.menu_links(items, attributes, 0) }}
+{{ menus.menu_links(items, attributes, 0, 'primary-menu-item-') }}
-{% macro menu_links(items, attributes, menu_level) %}
+{% macro menu_links(items, attributes, menu_level, aria_id) %}
{% set primary_nav_level = 'primary-nav__menu--level-' ~ (menu_level + 1) %}
- {% set drupal_selector_primary_nav_level = 'primary-nav-menu--level-' ~ (menu_level + 1) %}
+ {% set drupal_selector_primary_nav_level = menu_level <= 1 ? 'primary-nav-menu--level-' ~ (menu_level + 1) : false %}
+ {% set is_top_level_menu = menu_level == 0 %}
{% import _self as menus %}
{% if items %}
@@ -73,21 +74,21 @@
]
%}
- <li{{ item.attributes.addClass(item_classes).setAttribute('data-drupal-selector', item.below ? 'primary-nav-menu-item-has-children' : false) }}>
+ <li{{ item.attributes.addClass(item_classes).setAttribute('data-drupal-selector', is_top_level_menu and item.below ? 'primary-nav-menu-item-has-children': false) }}>
{#
A unique HTML ID should be used, but that isn't available through
Twig yet, so the |clean_id filter is used for now.
@see https://www.drupal.org/project/drupal/issues/3115445
#}
- {% set aria_id = (item.title ~ '-submenu-' ~ loop.index )|clean_id %}
+ {% set aria_id = (aria_id ~ loop.index )|clean_id %}
{% set link_title %}
- <span class="primary-nav__menu-link-inner">{{ item.title }}</span>
+ <span class="primary-nav__menu-link-inner primary-nav__menu-link-inner--level-{{ menu_level + 1 }}">{{ item.title }}</span>
{% endset %}
{% if menu_item_type == 'link' or menu_item_type == 'nolink' %}
{{ link(menu_item_type == 'link' ? link_title : item.title, item.url, {
'class': link_classes,
- 'data-drupal-selector': 'primary-nav-menu-link-has-children',
+ 'data-drupal-selector': is_top_level_menu ? 'primary-nav-menu-link-has-children' : false,
})
}}
@@ -97,36 +98,37 @@
but still visible in non-JS environments so that chevron can indicate presence of
drop menu).
#}
- {%
- set toggle_button_attributes = create_attribute({
+ {% if is_top_level_menu %}
+ {% set toggle_button_attributes = create_attribute({
'class': 'primary-nav__button-toggle',
'data-drupal-selector': 'primary-nav-submenu-toggle-button',
'aria-controls': aria_id,
'aria-expanded': 'false',
'aria-hidden': 'true',
'tabindex': '-1',
- })
- %}
- <button{{ toggle_button_attributes }}>
- <span class="visually-hidden">{{ '@title sub-navigation'|t({'@title': item.title}) }}</span>
- <span class="icon--menu-toggle"></span>
- </button>
+ }) %}
+
+ <button{{ toggle_button_attributes }}>
+ <span class="visually-hidden">{{ '@title sub-navigation'|t({'@title': item.title}) }}</span>
+ <span class="icon--menu-toggle"></span>
+ </button>
+ {% endif %}
{% set attributes = attributes.setAttribute('id', aria_id) %}
- {{ menus.menu_links(item.below, attributes, menu_level + 1) }}
+ {{ menus.menu_links(item.below, attributes, menu_level + 1, aria_id) }}
{% endif %}
{% elseif menu_item_type == 'button' %}
+
{{ link(link_title, item.url, {
'class': link_classes,
- 'aria-controls': item.below ? aria_id : false,
- 'aria-expanded': item.below ? 'false' : false,
- 'data-drupal-selector': item.below ? 'primary-nav-submenu-toggle-button' : false,
- })
- }}
+ 'aria-controls': is_top_level_menu and item.below ? aria_id : false,
+ 'aria-expanded': is_top_level_menu and item.below ? 'false' : false,
+ 'data-drupal-selector': is_top_level_menu and item.below ? 'primary-nav-submenu-toggle-button' : false,
+ }) }}
{% set attributes = attributes.setAttribute('id', aria_id) %}
- {{ menus.menu_links(item.below, attributes, menu_level + 1) }}
+ {{ menus.menu_links(item.below, attributes, menu_level + 1, aria_id) }}
{% endif %}
</li>
{% endfor %}
--
GitLab