Verified Commit 1c67d3b5 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3447837 by finnsky, m4olivei, plopesc, kostyashupenko, kanchan bhogade,... 

Issue #3447837 by finnsky, m4olivei, plopesc, kostyashupenko, kanchan bhogade, smustgrave, alexpott: Special Menu items are rendered as empty links in navigation
parent b1e8ca59

core/modules/navigation/templates/navigation-menu.html.twig

+35 −15
Original line number Diff line number Diff line
@@ -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 %}

        <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 }}"

core/modules/navigation/templates/toolbar-button.html.twig

+1 −1
Original line number Diff line number Diff line
@@ -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>

core/modules/navigation/tests/src/Kernel/SystemMenuNavigationBlockTest.php→core/modules/navigation/tests/src/Kernel/NavigationMenuBlockTest.php

+98 −22
Original line number Diff line number Diff line
@@ -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.

   *

core/modules/navigation/tests/src/Kernel/NavigationMenuMarkupTest.php

+5 −5
Original line number Diff line number Diff line
@@ -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);

    }

  }