Add ability to track active menu items in menu source

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

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)

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.

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.

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

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.

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?

@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();

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 :)

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?

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?

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

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

This issue was recently merged into 2.x. We'll need to review changes and revise the patch.

Hello all, and thank you @mortona2k for the updated work.

I opened a new MR instead of pushing over !363, because the approach changes a bit:
(a) the links prop type now carries the active trail information: in_active_trail is declared in the schema, kept only when TRUE;
(b) the MenuSource mimics the recent SystemMenuBlock updates, with the same settings: "Expand all menu links" and "Set the active trail".

Users can now opt in to the active trail when injecting a menu into a links prop, and the items in the active trail can be properly themed in SDC components.

About caching, the route.menu_active_trails cache context is only added when the active trail is used, like core menu blocks do.

Also in the MR, alterComponent() reads the menu ID from the settings (fixes #20)

One difference with core: we keep expanding all menu items by default, so existing ui_patterns configurations keep the same behavior.

Kernel tests added.

Needs review, especially (a) and (b)

With the addition of set_active_trail and expand_all_items, MenuSource is becoming a copy of SystemMenuBlock but with the data logic decoupled from the rendering. That's cool.

However, why not using the menu.active_trail service to do the calculation, like SystemMenuBlock is doing?

menu.active_trail est présent depuis Drupal 8.0: #2301273: MenuLinkNG part2 (no UI or conversions): MenuLinkTree API and unit tests

Good point @pdureau :)

We were already using it indirectly: getCurrentRouteMenuTreeParameters() is a thin wrapper around menu.active_trail.
But when marking the trail on a fully expanded tree, it also had one useless database query per render.
So i fixed that with a little refactor (it costs 22 more lines) but saves that query on every render of a marking menu.

So i fixed that with a little refactor (it costs 22 more lines) but saves that query on every render of a marking menu.

The goal was to remove lines 😂

My guts tell me than MenuTreeSourceTrait::buildMenuTreeParameters() is overly complex and doesn't belong here, but that's only my guts. If you are comfortable with the change, go for it.

yes we wanted to remove lines when thinking about using the service directly, but having config knobs leads us unfortunately to add some code to cover the different cases...

by the way, listening to the guts is important, vital. i read it again, and yes it is a bit long, but phpmd is happy so complexity seems ok. About its place (e.g. "doesn't belong here"), we are just using the Drupal APIs to process menu tree according to our input knobs. so let's keep it here for the moment, open for future optimizations..

Better late than never, merged :)

we would like to give a warm thank you to all the people for their valuable inputs and comments.
Team work like we enjoy it.

Now the links prop type and ui_patterns will provide a natural way to deal with active links in menus,
and writing SDC component with active links is now less complex.