Nav submenu: add support to style current submenu in theme.json#76367
Nav submenu: add support to style current submenu in theme.json#76367MaggieCabrera wants to merge 3 commits intotrunkfrom
Conversation
|
Flaky tests detected in 8fd42d5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22949982588
|
c11d8b5 to
40b2cd6
Compare
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
1 similar comment
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| https://github.com/WordPress/wordpress-develop/pull/11174 | ||
|
|
||
| * https://github.com/WordPress/gutenberg/pull/75736 | ||
| * https://github.com/WordPress/gutenberg/pull/76367 |
There was a problem hiding this comment.
I'm updating the same core backport instead of making a new one, since it's built on top of it directly and it hasn't been merged yet
|
I'm a bit hesitant about this one. I think we should be moving in the direction of making navigation link and navigation submenu blocks simply variations of the same block. This will make that harder to do and moves us away from that direction. If this is a feature that we think themes will use then i might be a good compromise, however I'm struggling to think of a use case for it... |
getdave
left a comment
There was a problem hiding this comment.
My main question is can you target the top level menu item represented by a submenu if it is current? I'm conscious it's a different block that core/navigation-link but you'd still want to be able to style the parent.
Styling menus within the submenu dropdown seems like a real edge case. Not sure if this is required but I'd defer to your theme expertise on that.
I've never personally needed to. That said, I don't think this PR is a must, but it certainly offers more flexibility at very little cost. I would love to hear what @WordPress/block-themers have to say about this. |
Yes, the navigation-link selector also covers navigation-submenu links so it works in trunk already. |
What?
Follow up to #75736
Following the discussion on the other PR, we are adding support for
currentstate to the submenu block tooWhy?
For consistency and to allow users more nunaced control over how they style their navigation blocks.
How?
We are adding support the same we did for the nav link, by adding it to the allowed blocks constant, adding a selector on block.json and modifying the schema andadding tests.
Testing Instructions
Add this to your theme.json, before this PR only the nav-link applied, now we can target submenus specifically.
Screenshots or screencast