-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Navigation: Allow multiple navigations with the same ref #47453
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.
Code LGTM - there's some linting. I totally support this for now. If we ever figure we need navigation blocks inside navigation blocks we can then check for ref equality as well.
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.
Nice work. Please can we reference the bug report Issue in this PR description so we're clear what this PR is fixing?
* @param array $parsed_blocks the parsed blocks to be normalized. | ||
* @return bool true if the navigation block contains a nested navigation block. | ||
*/ | ||
function block_core_navigation_has_nested_core_navigation( $parsed_blocks ) { |
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.
function block_core_navigation_has_nested_core_navigation( $parsed_blocks ) { | |
function block_core_navigation_block_contains_core_navigation( $parsed_blocks ) { |
Nit: I think we can just name this to explain that it's testing a set of blocks for the presence of a core navigation block. The argument is "a collection of blocks" not "a core navigation block".
It's not "does this core navigation block contain core navigation blocks".
Flaky tests detected in 575db6b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4014912047
|
What?
Our last fix for preventing recursive navigation blocks (#46387) prevents users from having the same navigation twice in one page. This allows that use case while still preventing nested navigation blocks.
Fixes #47085 (comment)
Why?
Users should be allowed to create multiple navigation menus on the same page, with the same ref.
How?
Use a recursive loop to check whether a block or any inner blocks contain a navigation block.
Testing Instructions
<!-- /wp:navigation -->
to a wp_navigation CPT, but you'll need to use mysql. (See Navigation Block: Prevent circular references in navigation block rendering #46387 for instructions).Testing Instructions for Keyboard
As above.
Screenshots or screencast
Two identical navigations on the same page: