Navigation/Page List: Share block_core_shared_get_submenu_visibility#75685
Navigation/Page List: Share block_core_shared_get_submenu_visibility#75685
Conversation
|
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. |
| $shared_file = dirname( __DIR__, 2 ) . '/packages/block-library/src/navigation/shared/get-submenu-visibility.php'; | ||
| if ( ! function_exists( 'block_core_shared_get_submenu_visibility' ) && file_exists( $shared_file ) ) { | ||
| require_once $shared_file; | ||
| } |
There was a problem hiding this comment.
This fixed the test failure, but I'm not sure if it's a good fix or the best way to handle it. The error was: Error: Call to undefined function block_core_shared_get_submenu_visibility()
Co-authored-by: Sarah Norris <1645628+mikachan@users.noreply.github.com>
Source files are only ever loaded from the build output (where flattenIndexFiles changes the directory structure), so the file_exists conditionals checking source vs build paths are unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of modifying the build system to handle cross-file function references, call the shared function with the gutenberg_ prefix directly in consumer files, matching the existing pattern used by block_core_shared_navigation_item_should_render. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop the function_exists/file_exists guards — if the file is missing the test should fail loudly, not silently skip loading it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4b83482 to
5e5e193
Compare
The phpcs ruleset forbids calling functions matching /^[Gg]utenberg.*$/ without the IS_GUTENBERG_PLUGIN guard. Add the guard with a fallback to the unprefixed core function name at all four call sites. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeryj
left a comment
There was a problem hiding this comment.
It's working correctly, and does consolidate the logic. Left some thoughts. I feel semi-strongly about them, but could be convinced :) Nothing major.
| if ( defined( 'IS_GUTENBERG_PLUGIN' ) && IS_GUTENBERG_PLUGIN ) { | ||
| $computed_visibility = gutenberg_block_core_shared_get_submenu_visibility( $block->context ); | ||
| } else { | ||
| $computed_visibility = block_core_shared_get_submenu_visibility( $block->context ); | ||
| } |
There was a problem hiding this comment.
I think this would be easier to read if we leave $computed_visibility = block_core_navigation_submenu_get_submenu_visibility( $block->context ); and then put
if ( defined( 'IS_GUTENBERG_PLUGIN' ) && IS_GUTENBERG_PLUGIN ) {
$computed_visibility = gutenberg_block_core_shared_get_submenu_visibility( $block->context );
} else {
$computed_visibility = block_core_shared_get_submenu_visibility( $block->context );
}
Within it. That hides some of the complexity when reading through this part for logic. I'd also like a comment here explaining why we need this, and how gutenberg_block_core_shared_get_submenu_visibility is really block_core_shared_get_submenu_visibility, but just when the Gutenberg plugin is on. It's confusing so reminders would be really helpful :)
There was a problem hiding this comment.
This won't work because block_core_navigation_submenu_get_submenu_visibility isn't defined in core yet, so we'll get a fatal error (Uncaught Error: Call to undefined function block_core_navigation_submenu_get_submenu_visibility()). That solution will only work once the function is backported to core.
… calls Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ction Instead of wrapping every call to block_core_shared_get_submenu_visibility in IS_GUTENBERG_PLUGIN checks, add it to the build system's prefixFunctions list so cross-file calls are automatically renamed. Also prevent double- prefixing when a function is both in prefixFunctions and defined in a file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ared function" This reverts commit f624ee0.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks @jeryj I have updated the comments to make it clear why we need the IS_GUTENBERG_PLUGIN check. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Size Change: 0 B Total Size: 7.66 MB ℹ️ View Unchanged
|
What?
Share
block_core_shared_get_submenu_visibilityfunction between the Page List, Navigation Submenu and Navigation blocks.Why?
Since this code is repeated in 3 places, we should share it.
How?
Move the code into a shared file.
Testing Instructions