Problem/Motivation

When a menu item is active, an 'active' class is provided to the menu item, but it doesn't append to the array so clobbers any pre-existing classes.

This particular use case was employing Menu Attributes to provide a custom class on the menu item (<li>). When the page is visited and becomes 'active', that custom class is stripped.

Proposed resolution

In function radix_preprocess_menu() of /includes/menu.inc append to the array instead:

$variables['items'][$key]['attributes']['class'] = 'active';

to

$variables['items'][$key]['attributes']['class'][] = 'active';

CommentFileSizeAuthor
#12 radix-menu-active-class-stripping-3151018-12-5.0.x.patch1.6 KBcrzdev
#12 radix-menu-active-class-stripping-3151018-12.patch1.59 KBcrzdev
#5 radix-menu-active-class-stripping-3151018-5.patch1.06 KBcrzdev
#3 radix-menu-active-class-stripping-3151018-3.patch485 byteschroid

Comments

Chroid created an issue. See original summary.

somatick’s picture

somatick commented

The above fix has been tested on a client site, this appears to rectify the issue

chroid’s picture

chroid commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new radix-menu-active-class-stripping-3151018-3.patch485 bytes

Patch provided to ensure classes aren't being stripped when menu item is active. Thanks @somatick for the initial investigation and fix!

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented

RTBC +1 :)

crzdev’s picture

crzdev commented
StatusFileSize
new radix-menu-active-class-stripping-3151018-5.patch1.06 KB

I was getting this notice due to attributes is an attribute class instance instead of an array:
"Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect in radix_preprocess_menu()..."

Here goes new patch taking that into account.

crzdev’s picture

crzdev commented
Status: Reviewed & tested by the community » Needs review
chroid’s picture

chroid commented

@CRZDEV, thanks!

I can see the same notice in the logs, but it hasn't occurred for a number of months on our dev site. Do you have any steps to reproduce the error so I could test the patch in a targeted manner?

Cheers

crzdev’s picture

crzdev commented

Not sure about the exact combination of modules influencing into that but it can happen (most probbably is "link_attributes"), so i decided to contribute the patch, and it should also work in case the attributes is a simple array too.

Thanks @Chroid for the feedback.

yi_jiang’s picture

yi_jiang commented

right, so bootstrap storybook came from here:

https://www.drupal.org/project/bootstrap_storybook/issues/3167086
My fixing

https://www.drupal.org/files/issues/2020-08-27/nav-menu-in_active_trail-...

chroid’s picture

chroid commented

Can we RTBC this?

chroid’s picture

chroid commented
Status: Needs review » Reviewed & tested by the community

@CRZDEV - Was finally able to replicate this error (more that it just started popping up unannounced).

Confirming that the patch in #5 resolves this problem (Indirect modification of overloaded element)!

RTBC'ing

Cheers.

crzdev’s picture

crzdev commented
StatusFileSize
new radix-menu-active-class-stripping-3151018-12.patch1.59 KB
new radix-menu-active-class-stripping-3151018-12-5.0.x.patch1.6 KB

Looking at referenced issue "https://www.drupal.org/project/radix/issues/3147154", here goes another patch for the "8.x-4.x" branch in order to not require a re-rol to apply.
With this change, all item attributes should be added correctly and won't be lost by not using then into nav component template.

Also adding another patch to add this fix also to "5.0.x" branch.

crzdev’s picture

crzdev commented
Status: Reviewed & tested by the community » Needs review

Returning to needs review to evaluate both changes.

crzdev’s picture

crzdev commented
Version: 8.x-4.0 » 8.x-4.x-dev

  • ckng committed 86fb8a6 on 5.0.x 
    Issue #3151018 by ckng, CRZDEV, Chroid: Menu '\''active'\'' link is not...

  • ckng committed a7a934f on 8.x-4.x 
    Issue #3151018 by ckng, CRZDEV, Chroid: Menu '\''active'\'' link is not...
ckng’s picture

ckng
Primary language English
commented
Version: 8.x-4.x-dev » 5.0.x-dev
Status: Needs review » Fixed

Thanks for all the patches. Unfortunately, the correct one is mentioned in #3167086: nav menu in_active_trail attribute isnt really set.

ckng’s picture

ckng
Primary language English
commented
Version: 5.0.x-dev » 8.x-4.x-dev

Backported to 8.x-4.x.

  • ckng committed a0d0b2d on 5.0.x 
    Revert "Issue #3151018 by ckng, CRZDEV, Chroid: Menu '\''active'\'' link...
  • ckng committed dc2c36e on 5.0.x
    Issue #3151018 by ckng, CRZDEV, Chroid: Menu '\''active'\'' link is not...

  • ckng committed 1762f54 on 8.x-4.x 
    Issue #3151018 by ckng, CRZDEV, Chroid: Menu '\''active'\'' link is not...
  • ckng committed 85ecca4 on 8.x-4.x
    Revert "Issue #3151018 by ckng, CRZDEV, Chroid: Menu '\''active'\'' link...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.