WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.53k stars 4.21k forks source link

Correctly apply current-menu-ancestor class in Nav block #67169

Open getdave opened 1 day ago

getdave commented 1 day ago

What?

Applies the current-menu-ancestor class to the <li> not the <a>

Closes https://github.com/WordPress/gutenberg/issues/50069

Why?

Consistency with current-menu-item which is applied to the <li>.

How?

Applies the current-menu-ancestor class to the <li> not the <a>

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

getdave commented 1 day ago

I'm guessing this class isn't applied in the Editor because there is no concept of this there right?

github-actions[bot] commented 1 day ago

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: mrwweb <mrwweb@git.wordpress.org>
Co-authored-by: jordesign <jordesign@git.wordpress.org>
Co-authored-by: webmandesign <webmandesign@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

MaggieCabrera commented 1 day ago

I'm guessing this class isn't applied in the Editor because there is no concept of this there right?

that sounds about right, it would be hard to map

MaggieCabrera commented 16 hours ago

I'm ok tentatively approving this if we get some more feedback from themers. If this only affect block themes, I've done a search on a dump I have of block themes in the themes repo and I don't see many themes that would be affected by this change