Skip to content

Navigation: Show inserter when Navigation Submenu block is selected #71578

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.

Sign up for GitHub

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

Merged
getdave merged 3 commits into from
Oct 2, 2025
Merged

Navigation: Show inserter when Navigation Submenu block is selected #71578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16f1d2e
Navigation: Show inserter when Navigation Submenu block is selected
getdave Sep 10, 2025
dc371fb
Look for inserter within Submenu
getdave Sep 29, 2025
b4e1cf1
Update test/e2e/specs/editor/blocks/navigation.spec.js
getdave Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/block-library/src/navigation/edit/inner-blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default function NavigationInnerBlocks( {
isImmediateParentOfSelectedBlock,
selectedBlockHasChildren,
isSelected,
hasSelectedDescendant,
Copy link
Contributor

@scruffian scruffian Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to call this hasSelectedInnerBlock, so it matches the selector name?

Copy link
Contributor Author

@getdave getdave Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

@getdave getdave Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm...I'm thinking about the logic here. Does it actually make sense? I'm wondering if there are edge cases here in terms of what's selected at what level of depth 🤔

Copy link
Contributor Author

@getdave getdave Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this some more and I think hasSelectedDescendant is actually the better name here.

The variable name should describe what it represents rather than which function was used to get it.

hasSelectedDescendant tells you "this block has a selected descendant somewhere in its hierarchy". hasSelectedInnerBlock might be confusing because it could sound like "does this block have inner blocks?" rather than "does this block have a selected descendant?"

} = useSelect(
( select ) => {
const {
Expand All @@ -41,6 +42,7 @@ export default function NavigationInnerBlocks( {
false
),
selectedBlockHasChildren: !! getBlockCount( selectedBlockId ),
hasSelectedDescendant: hasSelectedInnerBlock( clientId, true ),

// This prop is already available but computing it here ensures it's
// fresh compared to isImmediateParentOfSelectedBlock.
Expand Down Expand Up @@ -96,6 +98,7 @@ export default function NavigationInnerBlocks( {
isSelected ||
( isImmediateParentOfSelectedBlock &&
! selectedBlockHasChildren ) ||
hasSelectedDescendant ||
// Show the appender while dragging to allow inserting element between item and the appender.
parentOrChildHasSelection
? InnerBlocks.ButtonBlockAppender
Expand Down
12 changes: 10 additions & 2 deletions test/e2e/specs/editor/blocks/navigation.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,15 @@ test.describe( 'Navigation block', () => {
// Move focus to the submenu navigation appender
await page.keyboard.press( 'End' );
await pageUtils.pressKeys( 'ArrowRight', { times: 2 } );
await navigation.useBlockInserter();

// Use the submenu block inserter
const navBlock = navigation.getNavBlock();
const submenuBlock = navBlock.getByRole( 'document', {
name: 'Block: Submenu',
} );
const submenuBlockInserter = submenuBlock.getByLabel( 'Add block' );
await submenuBlockInserter.click();

await navigation.addLinkClose();
/**
* TODO: This is not desired behavior. Ideally the
Expand Down Expand Up @@ -686,7 +694,7 @@ class Navigation {
}

getNavBlockInserter() {
return this.getNavBlock().getByLabel( 'Add block' );
return this.getNavBlock().getByLabel( 'Add block' ).first();
}

getLinkControlSearch() {
Expand Down
Loading