Problem/Motivation

Navigation components in Design systems often display differently active links (which link is same as current page). Example: menu, bottom navigation, ...

Example 1: Menu in DaisyUI (https://v5.daisyui.com/components/menu/#menu-with-active-item)

<ul class="menu bg-base-200 w-56">
  <li><a>Item 1</a></li>
  <li><a class="menu-active">Item 2</a></li>
  <li><a>Item 3</a></li>
</ul>

Example 2: Dock in DaisyUI (https://v5.daisyui.com/components/dock/#dock)

<div class="dock">
  <button>
    <svg>...</svg>
    <span class="dock-label">Home</span>
  </button>
  
  <button class="dock-active">
    <svg>...</svg>
    <span class="dock-label">Inbox</span>
  </button>
  
  <button>
    <svg>...</svg>
    <span class="dock-label">Settings</span>
  </button>
</div>

With menu source it's not possible, as in core block menu (boolean in_active_trail variable), to differentiate active menu links. See
menu.html.twig documentation as reference: https://api.drupal.org/api/drupal/core%21modules%21system%21templates%21...

Proposed resolution

Add a new information to store that in getMenuItems() (MenuSource.php).

So that it will possible to do something like: 

{% for item in items %}
  {% if item.in_active_trail %}
    <div class="dock-active">
    {{ item.title }}
    </div>
  {% else%}
    ...
  {% endif %}
{% endfor %}

Issue fork ui_patterns-3508748

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

g4mbini created an issue. See original summary.

g4mbini’s picture

g4mbini
Primary language French
Location Ruvo di Puglia (BA)
commented
Issue summary: View changes
g4mbini’s picture

g4mbini
Primary language French
Location Ruvo di Puglia (BA)
commented
Issue summary: View changes
g4mbini’s picture

g4mbini
Primary language French
Location Ruvo di Puglia (BA)
commented

@just_like_good_vibes I'll be happy to show you my issue with menus !

mortona2k made their first commit to this issue’s fork.

mortona2k opened merge request !363

mortona2k’s picture

mortona2k commented
Status: Active » Needs review

Loaded the menu.active_trail service to use getActiveTrailIds, and then $parameters->setActiveTrail().

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Should it be done in the menu source or in the links prop type?

Currently the active trail is put as attribute in Links prop type, then I would say you play with that in your templates.

In UI Suite Bootstrap, I kind of follow what core does with either JS for authenticated user or event subscriber for anonymous users (need the companion module in #3493509: [2025-12] blocked implementations and generic stuff)

mortona2k’s picture

mortona2k commented

MenuSource is where the links are being loaded by MenuLinkTree->getMenuItems().

That's where I adjusted the parameters being passed in.

This is just like the default system menu block:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/syste...

I recall finding out that active links are not the same as active menu trail. The active tab/local task is a good example of where .is-active is used. But I think active menu trail classes are generated by the menu tree, and don't need the js for anonymous users.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello,

Stuff like item.in_active_trail is already possible.

When using a pattern with menu source. Active trail is present but not set properly.

Tested the MR and it fixes the issue.

Thanks.

Not merging because main UIP2 maintainers are now @just_like_good_vices and @christian.wiedemann, so maybe needs automated tests.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Changing to needs work, Tests in tests/src/Kernel/Source/MenuSourceTest.php should be completed I think.

pdureau’s picture

pdureau commented
Title: Add ability to track active menu items in menu source » [2.0.4] Add ability to track active menu items in menu source
pdureau’s picture

pdureau commented
Title: [2.0.4] Add ability to track active menu items in menu source » [2.0.x] Add ability to track active menu items in menu source
svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Priority: Normal » Major

As reported in #3526471: [5.0.0-beta2] Menu items do not get is-active class, this MR fixes the issue for me in ui_suite_daisyui for me, after adding some extra logic in the menu component of that theme.

I think having active menu items is pretty basic expected behaviour for a theme, so would be good to get this included sooner rather than later. Having to patch both a module and theme to get a visual indicator of the active menu item, isn't the best situation for getting adoption IMHO. Therefor I took the liberty to increase the priority of this issue. Feel free to lower priority if you disagree.

g4mbini’s picture

g4mbini
Primary language French
Location Ruvo di Puglia (BA)
commented

This issue as been tested 2 times by @Grimreaper and @svendecabooter and only needs tests now ...

Do you think it could be added to 2.0.6 scope as it blocks some theme implementations ?

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

Hello,
just to add some information about the current implementation, i am questioning here the way it is done,
in comparison to the code in SystemMenuBlock::build

we could use menuTree->getCurrentRouteMenuTreeParameters(,
any thoughts?

steffenr’s picture

steffenr
Primary language German
Location Bremen
commented

@just_like_good_vibes
I run into another issue, with displaying sub menu items of a given parent.

If you use the default UI Patterns menu component (in my case ui_suite_daisyui), you would get all the submenu items and not only the ones related to the parent.

This can be fixed in MenuSource->getMenuItems by using:

$parameters = $this->menuLinkTree->getCurrentRouteMenuTreeParameters($this->menuId);
instead of
$parameters = new MenuTreeParameters();

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Status: Needs work » Needs review

Hello guys,
i rebased the MR and added some modifications.
still no time to add some tests about that.

so i am asking, what kind of minimal tests do we here to test that feature?
i guess that should be functional tests to have the ability to have the active trail context.

i was tempted to merge, but i put it for review right now, unassigned, someone can take it and review please :)

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

i want to add also this : if we take care of the active status of links here, then the data has some cache metadata,
to simplify we could say the data would depend on the current url ?
that part was not added in my MR. it brings some complexity right? that part comes at a cost :)
let's discuss this in the next weekly?

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
commented

Talking about cache I discovered a bug about the menu items being stale because menuId is not initialized yet when `alterComponent()` is called.

I suggest getting rid of the property class menuId and access the configuration every time. What do you think?

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
commented

I went ahead and made the change fixing a phpstan warning along the way.

Leaving it in Needs review for the removal of the class property but still needs tests

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

Hello,
thank you for the work done.
i think here we need testers, someone wants to take assignments and make some manual tests?