Add a current-menu-ancestor class to navigation items

[aristath]

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 contains current-menu-item, then the parent gets a current-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)

[draganescu]

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.

[aristath]

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... 🤔
I agree, it would be nice to add a test for that. Unfortunately, I don't know how to do that 😅

[getdave]

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.

Unfortunately, I don't know how to do that 😅

The class is defined in

gutenberg/packages/block-library/src/navigation-link/index.php

Lines 155 to 161 in 895ca1f

	$wrapper_attributes = get_block_wrapper_attributes(
	array(
	'class' => $css_classes . ' wp-block-navigation-item' . ( $has_submenu ? ' has-child' : '' ) .
	( $is_active ? ' current-menu-item' : '' ),
	'style' => $style_attribute,
	)
	);

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.

[Marc-pi]

i've just cross linked duplicated issues, so you can close all fo them when merged ;-) less is more

[coreyworrell]

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.

[getdave]

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...

[aristath]

@getdave thank you for testing this!
I pushed a fix, it should be working fine now 👍

[getdave]

You are welcome.

It seems to be now we have no classes?

I'm testing this by

• creating a menu with several levels of nesting
• publishing
• doing to front of site
• checking for classes on parents of nested items

[aristath]

It seems to be now we have no classes?

That's weird... Seems to be working fine on my end 🤔

checking for classes on parents of nested items

Is the child the active page? When it is, I can see the classes fine on my localhost

[aristath]

@getdave on your last screenshot I don't see any items with the .current-menu-item, so it makes sense that the ancestors don't get the .current-menu-ancestor class. Unless I missed something on that screenshot? 🤔

[getdave]

Tested (again) following the instructions more carefully this time. I can see that the ancestor class is applied to the anchor for ancestors of the current page.

Thank you for persisting with this 🙇

[Marc-pi]

@aristath many thanks
i think you can also close those i cross referenced above, here is a screenshot

[bph]

Flagging for Dev Notes 6.2. - @aristath do you think it would be a suitable candidate for a blurb to a Miscellaneous post?

[scruffian]

@bph yeah I think that's a good idea