From a5a8967cd48ac86ddba619449c921306c879365c Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Fri, 13 Sep 2024 18:36:51 +0100
Subject: [PATCH] Issue #3447837 by finnsky, m4olivei, plopesc, kostyashupenko,
kanchan bhogade, smustgrave, alexpott: Special Menu items are rendered as
empty links in navigation
(cherry picked from commit 1c67d3b55935ed8cb993bd49966d0603e62175e3)
---
.../templates/navigation-menu.html.twig | 50 +++++---
.../templates/toolbar-button.html.twig | 2 +-
...ckTest.php => NavigationMenuBlockTest.php} | 120 ++++++++++++++----
.../src/Kernel/NavigationMenuMarkupTest.php | 10 +-
4 files changed, 139 insertions(+), 43 deletions(-)
rename core/modules/navigation/tests/src/Kernel/{SystemMenuNavigationBlockTest.php => NavigationMenuBlockTest.php} (66%)
diff --git a/core/modules/navigation/templates/navigation-menu.html.twig b/core/modules/navigation/templates/navigation-menu.html.twig
index f1242c9b58a3..7e9ec356e9a6 100644
--- a/core/modules/navigation/templates/navigation-menu.html.twig
+++ b/core/modules/navigation/templates/navigation-menu.html.twig
@@ -8,16 +8,33 @@
{% macro menu_items(items, attributes, menu_level) %}
{% for item in items %}
+
+ {% set item_link_tag = 'a' %}
+
+ {% if item.url.isRouted %}
+ {% if item.url.routeName == '<nolink>' %}
+ {% set item_link_tag = constant('\\Drupal\\Core\\GeneratedNoLink::TAG') %}
+ {% elseif item.url.routeName == '<button>' %}
+ {% set item_link_tag = constant('\\Drupal\\Core\\GeneratedButton::TAG') %}
+ {% endif %}
+ {% endif %}
+
+ {% if item.url.options.attributes is empty %}
+ {% set item_link_attributes = create_attribute() %}
+ {% else %}
+ {% set item_link_attributes = create_attribute(item.url.options.attributes) %}
+ {% endif %}
+
{% set item_id = ('navigation-link--' ~ item.original_link.pluginId)|clean_unique_id %}
{% if menu_level == 0 %}
- {% if item.below is empty %}
+ {% if item.below is empty %}
<li id="{{ item_id }}" class="toolbar-block__list-item">
{% include '@navigation/toolbar-button.html.twig' with {
- attributes: create_attribute({ 'href': item.url|render, 'data-drupal-tooltip': item.title, 'data-drupal-tooltip-class': 'admin-toolbar__tooltip' }),
+ attributes: item_link_attributes.setAttribute('href', item.url|render|default(null)).setAttribute('data-drupal-tooltip', item.title).setAttribute('data-drupal-tooltip-class', 'admin-toolbar__tooltip'),
icon: item.class|clean_class,
- html_tag: 'a',
+ html_tag: item_link_tag,
text: item.title,
- extra_classes: 'toolbar-button--collapsible',
+ extra_classes: 'toolbar-button--collapsible ' ~ (item_link_tag == 'span' ? 'toolbar-button--non-interactive'),
} only %}
</li>
{% else %}
@@ -32,7 +49,7 @@
} only %}
<div class="toolbar-popover__wrapper" data-toolbar-popover-wrapper inert>
{% if item.url %}
- {{ link(item.title, item.url, create_attribute({'class': ['toolbar-popover__header', 'toolbar-button', 'toolbar-button--large', 'toolbar-button--dark']})) }}
+ {{ link(item.title, item.url, item_link_attributes.addClass(['toolbar-popover__header', 'toolbar-button', 'toolbar-button--large', 'toolbar-button--dark'])) }}
{% else %}
<span class="toolbar-popover__header toolbar-button toolbar-button--large toolbar-button--dark toolbar-button--non-interactive">{{ item.title }}</span>
{% endif %}
@@ -46,11 +63,12 @@
{% elseif menu_level == 1 %}
<li class="toolbar-menu__item toolbar-menu__item--level-{{ menu_level }}">
{% if item.below is empty %}
- <a
- href="{{ item.url }}"
- class="toolbar-button"
- data-index-text="{{ item.title|first|lower }}"
- >{{ item.title }}</a>
+ {% include '@navigation/toolbar-button.html.twig' with {
+ attributes: item_link_attributes.setAttribute('href', item.url|render|default(null)),
+ text: item.title,
+ html_tag: item_link_tag,
+ extra_classes: item_link_tag == 'span' ? 'toolbar-button--non-interactive',
+ } only %}
{% else %}
<button
class="toolbar-button toolbar-button--expand--down"
@@ -69,11 +87,13 @@
{% else %}
<li class="toolbar-menu__item toolbar-menu__item--level-{{ menu_level }}">
{% if item.below is empty %}
- <a
- href="{{ item.url }}"
- class="toolbar-menu__link toolbar-menu__link--{{ menu_level }}"
- data-index-text="{{ item.title|first|lower }}"
- >{{ item.title }}</a>
+ {{ link(item.title, item.url, {
+ 'class': [
+ 'toolbar-menu__link',
+ 'toolbar-menu__link--' ~ menu_level,
+ ],
+ 'data-index-text': item.title|first|lower
+ }) }}
{% else %}
<button
class="toolbar-menu__link toolbar-menu__link--{{ menu_level }}"
diff --git a/core/modules/navigation/templates/toolbar-button.html.twig b/core/modules/navigation/templates/toolbar-button.html.twig
index 7ba0fc8d185f..a209398068c9 100644
--- a/core/modules/navigation/templates/toolbar-button.html.twig
+++ b/core/modules/navigation/templates/toolbar-button.html.twig
@@ -10,7 +10,7 @@ appear after main classes #}
<{{html_tag|default('button')}}
{{ attributes.addClass(classes) }}
data-index-text="{{ text|first|lower }}"
- data-icon-text={{- text|render|trim|slice(0, 2)|join('') }}
+ data-icon-text="{{- text|render|trim|slice(0, 2)|join('') -}}"
>
{% if avatar %}
<span class="toolbar-button__avatar">{{~ avatar ~}}</span>
diff --git a/core/modules/navigation/tests/src/Kernel/SystemMenuNavigationBlockTest.php b/core/modules/navigation/tests/src/Kernel/NavigationMenuBlockTest.php
similarity index 66%
rename from core/modules/navigation/tests/src/Kernel/SystemMenuNavigationBlockTest.php
rename to core/modules/navigation/tests/src/Kernel/NavigationMenuBlockTest.php
index d591e2723855..d6ccfc6983b0 100644
--- a/core/modules/navigation/tests/src/Kernel/SystemMenuNavigationBlockTest.php
+++ b/core/modules/navigation/tests/src/Kernel/NavigationMenuBlockTest.php
@@ -6,7 +6,9 @@
use Drupal\block\Entity\Block;
use Drupal\Core\Render\Element;
+use Drupal\Core\Render\MetadataBubblingUrlGenerator;
use Drupal\Core\Routing\RouteObjectInterface;
+use Drupal\Core\Routing\UrlGenerator;
use Drupal\KernelTests\KernelTestBase;
use Drupal\navigation\Plugin\Block\NavigationMenuBlock;
use Drupal\system\Entity\Menu;
@@ -20,7 +22,7 @@
use Symfony\Component\Routing\RouteCollection;
/**
- * Tests \Drupal\navigation\Plugin\Block\SystemMenuNavigationBlock.
+ * Tests \Drupal\navigation\Plugin\Block\NavigationMenuBlock.
*
* @group navigation
* @see \Drupal\navigation\Plugin\Derivative\SystemMenuNavigationBlock
@@ -28,7 +30,7 @@
* @todo Expand test coverage to all SystemMenuNavigationBlock functionality,
* including block_menu_delete().
*/
-class SystemMenuNavigationBlockTest extends KernelTestBase {
+class NavigationMenuBlockTest extends KernelTestBase {
/**
* {@inheritdoc}
@@ -102,19 +104,31 @@ protected function setUp(): void {
$routes = new RouteCollection();
$requirements = ['_access' => 'TRUE'];
$options = ['_access_checks' => ['access_check.default']];
- $routes->add('example1', new Route('/example1', [], $requirements, $options));
+ $special_options = $options + ['_no_path' => TRUE];
$routes->add('example2', new Route('/example2', [], $requirements, $options));
- $routes->add('example3', new Route('/example3', [], $requirements, $options));
$routes->add('example4', new Route('/example4', [], $requirements, $options));
- $routes->add('example5', new Route('/example5', [], $requirements, $options));
- $routes->add('example6', new Route('/example6', [], $requirements, $options));
- $routes->add('example7', new Route('/example7', [], $requirements, $options));
- $routes->add('example8', new Route('/example8', [], $requirements, $options));
$routes->add('example9', new Route('/example9', [], $requirements, $options));
+ $routes->add('example11', new Route('/example11', [], $requirements, $options));
+ // Mock special routes defined in system.routing.yml.
+ $routes->add('<nolink>', new Route('', [], $requirements, $special_options));
+ $routes->add('<button>', new Route('', [], $requirements, $special_options));
+
+ // Define our RouteProvider mock.
$mock_route_provider = new MockRouteProvider($routes);
$this->container->set('router.route_provider', $mock_route_provider);
+ // Define our UrlGenerator service that use the new RouteProvider.
+ $url_generator_non_bubbling = new UrlGenerator(
+ $mock_route_provider,
+ $this->container->get('path_processor_manager'),
+ $this->container->get('route_processor_manager'),
+ $this->container->get('request_stack'),
+ $this->container->getParameter('filter_protocols')
+ );
+ $url_generator = new MetadataBubblingUrlGenerator($url_generator_non_bubbling, $this->container->get('renderer'));
+ $this->container->set('url_generator', $url_generator);
+
// Add a new custom menu.
$menu_name = 'mock';
$label = $this->randomMachineName(16);
@@ -127,27 +141,33 @@ protected function setUp(): void {
$this->menu->save();
// This creates a tree with the following structure:
- // - 1
+ // - 1 (nolink)
// - 2
- // - 3
+ // - 3 (nolink)
// - 4
// - 9
- // - 5
- // - 7
+ // - 5 (button)
+ // - 7 (button)
+ // - 10 (nolink)
// - 6
- // - 8
+ // - 8 (nolink)
+ // - 11
+ // - 12 (button)
// With link 6 being the only external link.
// phpcs:disable
$links = [
- 1 => MenuLinkMock::create(['id' => 'test.example1', 'route_name' => 'example1', 'title' => 'title 1', 'parent' => '', 'weight' => 0]),
+ 1 => MenuLinkMock::create(['id' => 'test.example1', 'route_name' => '<nolink>', 'title' => 'title 1', 'parent' => '', 'weight' => 0]),
2 => MenuLinkMock::create(['id' => 'test.example2', 'route_name' => 'example2', 'title' => 'title 2', 'parent' => '', 'route_parameters' => ['foo' => 'bar'], 'weight' => 1]),
- 3 => MenuLinkMock::create(['id' => 'test.example3', 'route_name' => 'example3', 'title' => 'title 3', 'parent' => 'test.example2', 'weight' => 2]),
+ 3 => MenuLinkMock::create(['id' => 'test.example3', 'route_name' => '<nolink>', 'title' => 'title 3', 'parent' => 'test.example2', 'weight' => 2]),
4 => MenuLinkMock::create(['id' => 'test.example4', 'route_name' => 'example4', 'title' => 'title 4', 'parent' => 'test.example3', 'weight' => 3]),
- 5 => MenuLinkMock::create(['id' => 'test.example5', 'route_name' => 'example5', 'title' => 'title 5', 'parent' => '', 'expanded' => TRUE, 'weight' => 4]),
- 6 => MenuLinkMock::create(['id' => 'test.example6', 'route_name' => '', 'url' => 'https://www.drupal.org/', 'title' => 'title 6', 'parent' => '', 'weight' => 5]),
- 7 => MenuLinkMock::create(['id' => 'test.example7', 'route_name' => 'example7', 'title' => 'title 7', 'parent' => 'test.example5', 'weight' => 6]),
- 8 => MenuLinkMock::create(['id' => 'test.example8', 'route_name' => 'example8', 'title' => 'title 8', 'parent' => '', 'weight' => 7]),
+ 5 => MenuLinkMock::create(['id' => 'test.example5', 'route_name' => '<button>', 'title' => 'title 5', 'parent' => '', 'expanded' => TRUE, 'weight' => 4]),
+ 6 => MenuLinkMock::create(['id' => 'test.example6', 'route_name' => '', 'url' => 'https://www.drupal.org/', 'title' => 'title 6', 'parent' => '', 'weight' => 5, 'options' => ['attributes' => ['target' => '_blank', 'class' => ['external-link']]]]),
+ 7 => MenuLinkMock::create(['id' => 'test.example7', 'route_name' => '<button>', 'title' => 'title 7', 'parent' => 'test.example5', 'weight' => 6]),
+ 8 => MenuLinkMock::create(['id' => 'test.example8', 'route_name' => '<nolink>', 'title' => 'title 8', 'parent' => '', 'weight' => 7]),
9 => MenuLinkMock::create(['id' => 'test.example9', 'route_name' => 'example9', 'title' => 'title 9', 'parent' => 'test.example4', 'weight' => 7]),
+ 10 => MenuLinkMock::create(['id' => 'test.example10', 'route_name' => '<nolink>', 'title' => 'title 10', 'parent' => 'test.example7', 'weight' => 7]),
+ 11 => MenuLinkMock::create(['id' => 'test.example11', 'route_name' => 'example11', 'title' => 'title 11', 'parent' => 'test.example8', 'weight' => 7]),
+ 12 => MenuLinkMock::create(['id' => 'test.example12', 'route_name' => '<button>', 'title' => 'title 12', 'parent' => 'test.example11', 'weight' => 7]),
];
// phpcs:enable
foreach ($links as $instance) {
@@ -218,9 +238,12 @@ public function testConfigLevelDepth(): void {
$expectations['level_2_only'] = [
'test.example3' => [],
'test.example7' => [],
+ 'test.example11' => [],
];
$expectations['level_3_only'] = [
'test.example4' => [],
+ 'test.example10' => [],
+ 'test.example12' => [],
];
$expectations['level_1_and_beyond'] = [
'test.example1' => [],
@@ -230,10 +253,16 @@ public function testConfigLevelDepth(): void {
],
],
'test.example5' => [
- 'test.example7' => [],
+ 'test.example7' => [
+ 'test.example10' => [],
+ ],
],
'test.example6' => [],
- 'test.example8' => [],
+ 'test.example8' => [
+ 'test.example11' => [
+ 'test.example12' => [],
+ ],
+ ],
];
$expectations['level_2_and_beyond'] = [
'test.example3' => [
@@ -241,12 +270,19 @@ public function testConfigLevelDepth(): void {
'test.example9' => [],
],
],
- 'test.example7' => [],
+ 'test.example7' => [
+ 'test.example10' => [],
+ ],
+ 'test.example11' => [
+ 'test.example12' => [],
+ ],
];
$expectations['level_3_and_beyond'] = [
'test.example4' => [
'test.example9' => [],
],
+ 'test.example10' => [],
+ 'test.example12' => [],
];
// Scenario 1: test all navigation block instances when there's no active
// trail.
@@ -277,6 +313,46 @@ public function testConfigLevelDepth(): void {
}
}
+ /**
+ * Tests the generated HTML markup.
+ */
+ public function testHtmlMarkup() {
+ $block = $this->blockManager->createInstance('navigation_menu:' . $this->menu->id(), [
+ 'region' => 'content',
+ 'id' => 'machine_name',
+ 'level' => 1,
+ 'depth' => NavigationMenuBlock::NAVIGATION_MAX_DEPTH - 1,
+ ]);
+
+ $block_build = $block->build();
+ $render = \Drupal::service('renderer')->renderRoot($block_build);
+
+ $dom = new \DOMDocument();
+ $dom->loadHTML((string) $render);
+ $xpath = new \DOMXPath($dom);
+
+ $items_query = [
+ "//li[contains(@class,'toolbar-block__list-item')]/span/span[text()='title 1']",
+ "//li[contains(@class,'toolbar-block__list-item')]/button/span[text()='title 2']",
+ "//li[contains(@class,'toolbar-menu__item--level-1')]/button/span[text()='title 3']",
+ "//li[contains(@class,'toolbar-menu__item--level-2')]/a[text()='title 4']",
+ "//li[contains(@class,'toolbar-block__list-item')]/button/span[text()='title 5']",
+ "//li[contains(@class,'toolbar-block__list-item')]/a/span[text()='title 6']",
+ "//li[contains(@class,'toolbar-block__list-item')]/a[contains(@class, 'external-link')]",
+ "//li[contains(@class,'toolbar-block__list-item')]/a[contains(@class, 'external-link')]",
+ "//li[contains(@class,'toolbar-block__list-item')]/a[@target='_blank']",
+ "//li[contains(@class,'toolbar-menu__item--level-1')]/button/span[text()='title 7']",
+ "//li[contains(@class,'toolbar-block__list-item')]/button/span[text()='title 8']",
+ "//li[contains(@class,'toolbar-menu__item--level-2')]/span[text()='title 10']",
+ "//li[contains(@class,'toolbar-menu__item--level-1')]/button/span[text()='title 11']",
+ "//li[contains(@class,'toolbar-menu__item--level-2')]/button[text()='title 12']",
+ ];
+ foreach ($items_query as $query) {
+ $span = $xpath->query($query);
+ $this->assertEquals(1, $span->length, $query);
+ }
+ }
+
/**
* Helper method to allow for easy menu link tree structure assertions.
*
diff --git a/core/modules/navigation/tests/src/Kernel/NavigationMenuMarkupTest.php b/core/modules/navigation/tests/src/Kernel/NavigationMenuMarkupTest.php
index d1c3d2fd9b7c..23cb623f9f43 100644
--- a/core/modules/navigation/tests/src/Kernel/NavigationMenuMarkupTest.php
+++ b/core/modules/navigation/tests/src/Kernel/NavigationMenuMarkupTest.php
@@ -118,7 +118,7 @@ protected function setUp(): void {
$links = [
1 => MenuLinkMock::create(['id' => 'test.example1', 'route_name' => 'example1', 'title' => 'title 1', 'parent' => '', 'weight' => 0]),
2 => MenuLinkMock::create(['id' => 'test.example2', 'route_name' => 'example2', 'title' => 'Another title', 'parent' => '', 'route_parameters' => ['foo' => 'bar'], 'weight' => 1]),
- 3 => MenuLinkMock::create(['id' => 'test.example3', 'route_name' => 'example3', 'title' => 'A menu link', 'parent' => 'test.example2', 'weight' => 2]),
+ 3 => MenuLinkMock::create(['id' => 'test.example3', 'route_name' => 'example3', 'title' => 'Nested menu link', 'parent' => 'test.example2', 'weight' => 2]),
];
// phpcs:enable
foreach ($links as $instance) {
@@ -149,12 +149,12 @@ public function testToolbarButtonAttributes(): void {
"//li[contains(@class,'toolbar-block__list-item')]/a[@data-icon-text='ti']",
"//li[contains(@class,'toolbar-block__list-item')]/button[@data-index-text='a']",
"//li[contains(@class,'toolbar-block__list-item')]/button[@data-icon-text='An']",
- "//li[contains(@class,'toolbar-menu__item--level-1')]/a[@data-index-text='a']",
- "//li[contains(@class,'toolbar-menu__item--level-1')]/a[not(@data-icon-text)]",
+ "//li[contains(@class,'toolbar-menu__item--level-1')]/a[@data-index-text='n']",
+ "//li[contains(@class,'toolbar-menu__item--level-1')]/a[@data-icon-text='Ne']",
];
foreach ($items_query as $query) {
- $span = $xpath->query($query);
- $this->assertEquals(1, $span->length, $query);
+ $element = $xpath->query($query);
+ $this->assertEquals(1, $element->length, $query);
}
}
--
GitLab