Render custom overlay template parts in Navigation block (behind experiment) #73967
+303
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getdave
added
[Block] Navigation
|
Size Change: +307 B (+0.01%) Total Size: 2.57 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 843923b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20167675151
|
getdave
changed the title
jeryj
reviewed
Dec 12, 2025
Contributor
jeryj
left a comment
jeryj
left a comment
There was a problem hiding this comment.
I'm concerned about experimental overlay markup leaking out to the main canvas for people without the experiment on. Could the overlay specific stuff be more batched? Or have a default function that prefixes the functions and individual changes to make sure they don't get applied if the experiment is off?
| * @param array $attributes The block attributes. | ||
| * @return WP_Block_List Returns the inner blocks for the overlay template part. | ||
| */ | ||
| private static function get_overlay_blocks_from_template_part( $overlay_template_part_id, $attributes ) { |
Contributor
There was a problem hiding this comment.
I haven't worked with template parts much. This function is getting the blocks for a template by ID with sensible fallbacks, right? I'm surprised this isn't a util already.
| $responsive_container_classes = array( | ||
| 'wp-block-navigation__responsive-container', | ||
| $is_hidden_by_default ? 'hidden-by-default' : '', | ||
| $has_custom_overlay ? 'has-custom-overlay' : '', |
Contributor
There was a problem hiding this comment.
'has-custom-overlay' feels a little odd of a name since the overlay html is not within this code. Should we name it by intention, like 'disable-default-overlay'?
| // When menu is closed (not open), show desktop container and hide overlay container. | ||
| &:not(.is-menu-open) { | ||
| .wp-block-navigation__responsive-container-content { | ||
| .wp-block-navigation__desktop-container { |
Contributor
There was a problem hiding this comment.
This menu might display on mobile devices too (no overlay, always show menu), so we should avoid "desktop" as the name. I'm trying to think of better names but struggling to find one I really like:
- default
- full
- text
- inline
- canvas
- page
- None? Just wp-block-navigation__container and have the overlay be the odd one out?
Contributor
|
I'm able to get the
|
Contributor
Author
|
Thanks for review. I'm going to take another look at this to see if we can isolate the experiment work further to avoid leakages. |
- Add get_overlay_blocks_from_template_part() helper to retrieve blocks from overlay template parts - Add get_template_part_blocks_html() to render template part blocks without navigation container wrapper - Modify get_responsive_container_markup() to render both desktop and overlay blocks in separate containers - Add CSS rules to show/hide desktop and overlay containers based on breakpoint and menu state - Ensure overlay container is full width - Maintain backward compatibility when no overlay is selected
- Add has-custom-overlay class to responsive container when overlay template part is selected - Exclude default padding from responsive-container when custom overlay is present - Exclude default padding-top from responsive-container-content when custom overlay is present - Exclude default margin-top from responsive-dialog when custom overlay is present - Exclude default background and text colors when custom overlay is present - Simplify CSS structure by removing duplicate selectors
…lock - Add has_navigation_overlay_close_block() helper to check for core/navigation-overlay-close block - Skip checking inner blocks of navigation blocks for optimization - Conditionally render default close button only when overlay doesn't have its own close block - Hide default close button when core/navigation-overlay-close block is present in overlay template part
- Add block_core_navigation_add_directives_to_overlay_close() function following the same pattern as block_core_navigation_add_directives_to_submenu() - Use WP_HTML_Tag_Processor to find and add directives to the overlay close button - Add data-wp-on--click directive to enable closing the overlay menu - Apply directives only when overlay close block is present and navigation is interactive
- Add wp-block-navigation__desktop-container to gap: inherit rule - Ensures navigation block continues to inherit block gap CSS after adding desktop container wrapper - Overlay container excluded from gap inheritance as it controls its own spacing - Fix indentation issues in padding rules
- Add disable_overlay_menu_for_nested_navigation_blocks() function to recursively find and modify navigation blocks - Set overlayMenu attribute to 'never' for any navigation blocks found in overlay template parts - Prevents nested overlays (inception problem) when navigation blocks are included in overlay content - Applied to both theme file and database post code paths
- Add top and right spacing to wp-block-navigation__responsive-container-close when parent has has-custom-overlay class - Use clamp value matching root padding pattern to position button away from overlay edges - Only applies when default close button is rendered within a custom overlay template part
- Add is_overlay_experiment_enabled() helper to check experiment status - Make desktop container wrapper conditional on experiment being enabled - Maintain backward compatibility: without experiment, desktop navigation markup remains unchanged - Prevents CSS/layout issues for users without the experiment enabled - Add comprehensive comment explaining why this is gated
jeryj
force-pushed
the
feature/navigation-overlay-rendering
branch
from
843923b to
e5a5b71
Compare
Contributor
|
@getdave I rebased this with trunk to bring in the other recent overlay template changes |
Contributor
|
Why do we still output the HTML for the navigation block default overlay when the navigation has a custom overlay? I don't understand how the current route is more protective than if the experiment is on, and there's an overlay, not outputting unused html? |
* sets default negative values for custom overlay experiment in one location * consolates code to make it harder to accidentally bleed experiment code * changes desktop class to inline so it's not screen size specific
The wrapper could cause issues with how people target their css. Sibling + selectors could break if we add a new container. To avoid this, we use the existing markup and output the navigation custom overlay at the end of the block.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
This PR implements rendering of custom overlay template parts in the Navigation block, addressing a significant part of #73081. When a custom overlay template part is selected, the block now renders both the standard desktop navigation blocks and the overlay template part blocks, with proper show/hide behavior based on breakpoint and menu state.
Why
This enables users to create custom overlay designs for mobile navigation while maintaining the standard desktop navigation. The overlay template part can contain any blocks, allowing for fully customized mobile menu experiences.
How
get_overlay_blocks_from_template_part()helper to retrieve blocks from overlay template partsget_responsive_container_markup()to render both desktop and overlay blocks in separate containerscore/navigation-overlay-closeblock as alternative close buttonMarkup Changes and Backwards Compatibility
Important: The desktop container wrapper (
wp-block-navigation__desktop-container) is gated behind thegutenberg-customizable-navigation-overlaysexperiment to ensure backward compatibility.<div class="wp-block-navigation__desktop-container">to enable proper show/hide behavior with overlay containersThe experiment gate prevents the markup change from affecting trunk until the feature is ready for general availability. All other functionality (overlay rendering, close button handling, etc.) only activates when an overlay is selected, which the UI prevents unless the experiment is enabled.
Default/Fallback Scenarios
core/navigation-overlay-closeblockTesting Instructions
Overlays containing Nav blocks with overlay set to "Always":
Overlays with/without Nav Overlay Close block:
core/navigation-overlay-closeblock: Create an overlay template part that includes acore/navigation-overlay-closeblock. Verify that:core/navigation-overlay-closeblock: Create an overlay template part without the close block. Verify that:With and without custom overlay:
Accessibility and SEO Considerations
Accessibility: Overlay content (including any navigation blocks within it) is present in the DOM even when visually hidden. While we use
aria-hidden="true"by default, this may not be sufficient for all assistive technologies.SEO: Search engines may index overlay content that is intended to be mobile-only, potentially causing duplicate content issues if the overlay contains navigation blocks.
Potential Mitigations
To address these concerns, we could consider:
aria-hiddenmanagement: Use Interactivity API to togglearia-hiddenbased on breakpoint and menu state (currently only set statically)hiddenattribute: Addhiddenattribute when overlay is not visible (in addition to CSS display:none)inertattribute: Useinertattribute to make overlay content non-interactive when hiddendata-noindexor similar attributes to overlay container when hiddenThese mitigations should be evaluated and potentially implemented in a follow-up PR based on testing and feedback.