When I'm visiting a page that is linked as a level three menu item, I would expect both the level 1 and 2 items to be highlighting the active trail. This includes keeping the second level menu item open by default to be able to show the active menu item:
On top of this, we should make sure that the active trail is shown in the menu even when user is on a page that is not directly linked from the navigation. This was fixed recently in the core Toolbar module, which we could potentially use as an inspiration for the fix: #3371005: Toolbar doesn't indicate active menu trail for pages not included in Toolbar.
Link to designs on Figma to see all the states.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | afa7658fcae48db2a291cda6c49a847c.gif | 816.02 KB | finnsky |
| #24 | ee5739c21b8a025cc5369f4e0b51f124.png | 110.63 KB | finnsky |
| #13 | Screenshot 2024-04-16 at 2.46.53 PM.png | 31.9 KB | meghasharma |
| #13 | Screenshot 2024-04-16 at 2.46.36 PM.png | 44.53 KB | meghasharma |
| #9 | active-design.png | 20.04 KB | ckrina |
Show commandsStart 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:
Show commandsStart 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:
changes, plain diff MR !234changes, plain diff MR !230
Comments
Comment #2
abh1shek chauhan commentedI am working on this issue.
Comment #4
abh1shek chauhan commentedFixed the issue, please review.
Comment #8
ckrinaMoved the work to branch that follow naming conventions to avoid confusions.
Comment #9
ckrinaApart from the feedback on the JS strategy, the active state should have the dark circle when it's active. You can explore the designs in Figma here. And I've added a link to the issue summary.
Comment #10
ckrinaComment #11
meghasharma commentedComment #12
meghasharma commentedAdded dark circle to active state when it's active as per the figma design.
Please review
Thanks!
Comment #13
meghasharma commentedComment #14
meghasharma commentedComment #15
finnsky commentedThis should be rebased first i guess.
Comment #16
meghasharma commentedComment #17
meghasharma commentedComment #18
ckrinaComment #19
ckrinaComment #22
gauravvvv commentedComment #23
smustgrave commentedWith the js change believe something that should have test coverage.
Comment #24
finnsky commentedThank you!
MR contains lot of css changes which breaks layout
Also i believe it does not need rework of all JS, Cause all events already exist. Gonna check
Comment #26
finnsky commentedIt needs design and UI decision.
In the original version of the menu, we had limited space in the popover and closed the child menu when changing the popover. Now I think
this is completely unnecessary because there is enough space in the popover to display the children's menu open.
This MR resolve it. But not works in case of deeper levels. We need to rewrite events system a bit to achieve this.
Comment #27
finnsky commentedNow works with any level of nesting.
Comment #29
finnsky commentedWe need to add extra css using `:has` selector
https://developer.mozilla.org/en-US/docs/Web/CSS/:has
Examples presented for higher levels of menu
Comment #30
sjothivelu commentedHello,
I am looking into this.