Navigation: Add submenu link via BlockSettingsMenuControls SlotFill#76005
Open
Navigation: Add submenu link via BlockSettingsMenuControls SlotFill#76005
Conversation
github-actions
bot
added
[Package] Editor
|
Size Change: +690 B (+0.01%) Total Size: 6.88 MB
ℹ️ View Unchanged
|
github-actions
bot
removed
[Package] Editor
|
Flaky tests detected in e31d1b4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22772321428
|
Add a BlockSettingsMenuControls fill from the navigation block that renders the "Add submenu link" menu item in the standard BlockSettingsDropdown. This makes the option available in the main document list view for navigation-link and navigation-submenu blocks. - Create AddSubmenuFill component using BlockSettingsMenuControls fill - Forward list-view props (expand, expandedState, setInsertedBlock) through fillProps in BlockSettingsDropdown - Pass isContentOnly through fillProps so fills can self-gate - Remove the blanket isContentOnly guard on BlockSettingsMenuControls.Slot (individual controls already have their own guards) - Update template-part fill to check isContentOnly from fillProps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add isContentOnly checks to all existing BlockSettingsMenuControls fills so they don't appear in content-only editing mode. This maintains the previous behavior where the entire Slot was hidden in content-only mode, but now each fill opts out individually, allowing specific fills (like AddSubmenuFill) to opt in. Updated fills: - patterns: PatternConvertButton, PatternsManageButton - reusable-blocks: ReusableBlockConvertButton, ReusableBlocksManageButton - template-part: ReplaceButton - image: "Set as featured image" - editor: PluginBlockSettingsMenuItem (public plugin API) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of requiring every fill to check isContentOnly individually, the BlockSettingsMenuControls Fill wrapper now hides fills in content-only mode by default. Fills that need to render in content-only mode can opt in via the supportsContentOnly prop. This reverts the individual isContentOnly checks added to fills in the previous commit and replaces them with a single centralized check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These are internal implementation details that don't need to be in the public API documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of creating a wrappedChildren variable, always wrap the fill children in a function that does an early return when the fill shouldn't display in content-only mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The main document list view and the inspector list view have separate insertedBlock/setInsertedBlock state. NavigationLinkUI (the link popover) only exists as additionalBlockContent in the inspector's PrivateListView, so it never appears in the main list view. Fix by managing local insertedBlock state in AddSubmenuFill and rendering NavigationLinkUI directly. This way the link popover appears after clicking "Add submenu link" regardless of which list view context it was triggered from. Also fix the expand call to use the correct clientId after converting a navigation-link to a navigation-submenu. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The dropdown was returning null early when isContentOnly was true and the block had no remove/duplicate/insert capabilities. This prevented fills like AddSubmenuFill (with supportsContentOnly) from rendering in the dropdown. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit removed the isEmpty early return entirely, which caused tests to fail because the Options button now appeared for all content-only blocks (headings, paragraphs in patterns, etc.). Restore the check but use useSlotFills to detect registered fills. When fills exist (e.g. AddSubmenuFill for navigation blocks), the dropdown renders in content-only mode. When no fills are registered, the dropdown is hidden as before. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The NavigationLinkUI popover was rendering from the canvas React tree, causing it to appear on the canvas instead of near the list view. Fix the fillProps prop ordering so the local setInsertedBlock takes precedence, find the list view row DOM element via data-block attribute, and pass it as the Popover anchor so the link picker appears next to the correct list view row. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Anchor the link popover to the parent submenu's Options button instead of the list view row, and use bottom-start placement to match the block settings dropdown positioning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
scruffian
force-pushed
the
scruffian/submenu-slotfill
branch
from
a169633 to
91d0069
Compare
Position the link popover on the same line as the newly added navigation-link block, anchored to its Options button on the right. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
scruffian
commented
Mar 3, 2026
| <Fill { ...props } /> | ||
| <Fill { ...restProps }> | ||
| { ( fillProps ) => { | ||
| if ( fillProps.isContentOnly && ! supportsContentOnly ) { |
Contributor
Author
There was a problem hiding this comment.
I think this is better than expecting all fills to update to explicitly opt-out of content only. It also does give fills the option to opt in, which might be more power than we want to allow?
Contributor
There was a problem hiding this comment.
I think it is a good idea overall. I am worried about the level of power as well. Lots of things can get jammed into this menu. I'm not sure what the best path is here.
scruffian
commented
Mar 3, 2026
| 'core/navigation-submenu', | ||
| ]; | ||
|
|
||
| function AddSubmenuItem( { |
Contributor
Author
There was a problem hiding this comment.
There is quite a bit of duplication here with the LeafMoreMenu implementation, but I plan to remove that one in #75694
scruffian
requested review from
ajitbohra,
ellatrix and
fabiankaegy
as code owners
|
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. |
jeryj
reviewed
Mar 5, 2026
Contributor
jeryj
left a comment
jeryj
left a comment
There was a problem hiding this comment.
Left some comments, as I'm not quite sure how this working yet. I haven't interacted with how these menus work, so I've got a little context catching up to do.
Comment on lines
-223
to
-226
| // It is possible that some plugins register fills for this menu | ||
| // even if Core doesn't render anything in the block settings menu. | ||
| // in which case, we may want to render the menu anyway. | ||
| // That said for now, we can start more conservative. |
Contributor
There was a problem hiding this comment.
I think this comment is still relevant. I don't think we should entirely remove it.
packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js
Outdated
Show resolved
Hide resolved
| <Fill { ...props } /> | ||
| <Fill { ...restProps }> | ||
| { ( fillProps ) => { | ||
| if ( fillProps.isContentOnly && ! supportsContentOnly ) { |
Contributor
There was a problem hiding this comment.
I think it is a good idea overall. I am worried about the level of power as well. Lots of things can get jammed into this menu. I'm not sure what the best path is here.
| isContentOnly, | ||
| expand, | ||
| expandedState, | ||
| setInsertedBlock, |
Contributor
There was a problem hiding this comment.
The setInsertedBlock flow here is hard to follow. There's a setInsertedBlock being passed here and then a later one that's getting set via a local state?
Contributor
Author
There was a problem hiding this comment.
The setInsertedBlock for sharing the state of the inserted block - the AddSubmenuFill shares this state between the slot and the navgation link UI component so they can both share the context of which block has been added.
- Use slot name string directly in useSlotFills since the custom wrapper component does not carry __unstableName from createSlotFill - Restore comment context about plugins registering fills - Replace requestAnimationFrame + querySelector with a MutationObserver that reliably waits for the block row to render in the list view Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the local insertedBlock/popoverAnchor state, useEffect, and MutationObserver from AddSubmenuFill. The fill now passes fillProps through directly, so the List View's existing AdditionalBlockContent mechanism handles popover rendering — matching how leaf-more-menu.js already works. Also reverts the anchor/placement props from NavigationLinkUI and LinkUI since they are no longer needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 770302b.
The useSlotFills check counts all registered fills globally, not just those that would render content for the current block. This caused the Options dropdown to remain visible for all blocks in content-only mode, even when fills would render null. Restore the original isEmpty logic. The fill still works in the list view because navigation children have canRemove/canDuplicate available, so isEmpty is false and the dropdown renders there. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the NavigationLinkUI popover closes (via Escape or after setting a URL), focus now returns to the Options button of the parent block that triggered the dropdown. Uses a ref to track the parent clientId and a useEffect to restore focus when insertedBlock becomes null. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Popover's useFocusReturn hook and list view focus management can race with our focus call. Defer with setTimeout(0) to ensure focus restoration runs after all synchronous cleanup completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace MutationObserver with simple querySelector by anchoring the NavigationLinkUI popover to the parent block's Options button (which already exists in the DOM) instead of the inserted child's row (which requires waiting for it to render). Merges anchor-finding and focus-restoration into a single useEffect. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Expose the BlockSettingsDropdown toggle button DOM element via fillProps so fills can use it as a popover anchor without relying on hardcoded CSS selectors. Uses useMergeRefs to compose with the caller's existing toggleProps ref. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a navigation-link is converted to a navigation-submenu, the original block is replaced with a new clientId. The toggle element captured from fillProps belongs to the old row and gets unmounted. Now passes anchor context with both the toggleElement (for the non-conversion case) and the clientId (for the conversion case, used to look up the new block's Options button via querySelector). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
For focus control, I think technically the correct experience for escaping out of adding a submenu item would be:
This would keep the experience similar to the Rename List Item flow. In order to avoid spawning multiple popovers on the page, I wonder if we should either:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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?
Adds the "Add submenu link" menu item to the standard
BlockSettingsDropdownvia aBlockSettingsMenuControlsSlotFill, and introduces centralized content-only mode filtering for fills.Why?
Currently the "Add submenu link" option only appears in the navigation block's custom list view (
LeafMoreMenu). Users working in the main document list view in the site editor cannot add submenu links from the block settings dropdown. This PR makes that action available via the standard SlotFill mechanism.Additionally, when blocks are in content-only editing mode, fills that aren't relevant (like "Convert to pattern", "Create template part", etc.) need to be excluded from the dropdown. Rather than requiring every fill to individually check
isContentOnly, this PR adds a centralized default: fills are hidden in content-only mode unless they explicitly opt in via thesupportsContentOnlyprop.This is a necessary step before removing the custom LeafMoreMenu that the Navigation block uses, in #75694
How?
BlockSettingsDropdown: Forwardsexpand,expandedState,setInsertedBlock, andisContentOnlythrough fillProps so fills can interact with list view state and content-only mode.BlockSettingsMenuControls(Fill wrapper): Wraps the children function to returnnullwhenisContentOnlyis true by default. Fills that need to render in content-only mode passsupportsContentOnlyto opt in. This is a single centralized check that protects all existing fills (including third-party plugins) without requiring changes to each one.AddSubmenuFill(new file): A newBlockSettingsMenuControlsfill in the navigation block that renders "Add submenu link" forcore/navigation-linkandcore/navigation-submenublocks. UsessupportsContentOnlyto opt in to content-only mode rendering. Deduplicates by checking that the selected block is a descendant of the specific navigation block instance.Navigation block edit: Renders
<AddSubmenuFill>in the main entity editing path.Testing Instructions
Testing Instructions for Keyboard
Screenshots
Screen.Recording.2026-03-03.at.15.31.32.mov
🤖 Generated with Claude Code