-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a current-menu-ancestor class to navigation items #40778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we depend on current-menu-item
and wp-block-navigation-item__content
, let's add a test to make sure this works in case anybody fiddles with those class names.
I would assume that when someone changes something like a class, they'd search the repo to see if it's used anywhere else... 🤔 |
But that's assuming everyone is as thoughtful and diligent as you are @aristath! I think it might be highly optimistic.
The class is defined in gutenberg/packages/block-library/src/navigation-link/index.php Lines 155 to 161 in 895ca1f
So we could add a e2e test for that block which checks if the class exists. Usually I'd say we shouldn't test developer APIs such as classes, but in this case it makes sense. |
i've just cross linked duplicated issues, so you can close all fo them when merged ;-) less is more |
I'm pretty sure classic menus also applied the class for a link to the Posts page as well when viewing a single post. If you viewed a post, the link in a menu for the "Posts page" would get the ancestor class as well. Assume this would apply for any custom post as well, if linking to the custom post type archive, and viewing a singular post of that type. |
8d81526
to
5229b5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for continuing to iterate on this.
I gave it a test
- Created Post
- Add Nav block
- Add 2 levels of submenu
- Published
- Checked classes
- Saw
current-menu-ancestor
applied to- lowest submenu item
- parent of lowest submenu item
- nothing on top level
I'm not sure that's intended behaviour but I could be wrong...
b3853fb
to
baef261
Compare
@getdave thank you for testing this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@getdave on your last screenshot I don't see any items with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aristath many thanks |
* Add a current-menu-ancestor class to menus * Use WP_HTML_Tag_Processor * bugfix
Flagging for Dev Notes 6.2. - @aristath do you think it would be a suitable candidate for a blurb to a Miscellaneous post? |
@bph yeah I think that's a good idea |
What?
Fixes #39663
When a submenu item is active, this will add a
current-menu-ancestor
to its parent menu-item(s), allowing theme-developers to style these items more efficiently.Why?
To make styling easier, and for consistency with the old navigation
How?
Adds a simple check in the
navigation-submenu
block. If the inner-blocks HTML containscurrent-menu-item
, then the parent gets acurrent-menu-ancestor
class.Testing Instructions
Add a menu with submenus. Visit one of the pages in a submenu, and check that the parent menu-item gets the
current-menu-ancestor
class. Check with nested submenus as well (2 level deep submenus)