Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
59b8295
refactor: eliminate prop drilling for directInsert in PrivateListView
getdave Aug 11, 2025
f6ba618
Use block list settings api
getdave Aug 11, 2025
2730503
Use original const def
getdave Aug 11, 2025
ad16bd3
Access setting directly in appender
getdave Aug 11, 2025
3385f0d
Revert branch component to match trunk
getdave Aug 11, 2025
4358d61
Revert another file to trunk
getdave Aug 11, 2025
6e47e86
Pass entire block to callback
getdave Aug 11, 2025
3312dfc
Revert changes to menu controls
getdave Aug 11, 2025
eeef4e4
Properly handle cleanup if no link selected
getdave Aug 11, 2025
359a453
Allow for setting blocks inserterd by Add block Quick Inserter
getdave Aug 11, 2025
0f9d541
fix: clean up original nav link block when selecting different block …
getdave Aug 14, 2025
4e88985
fix: prevent focus stealing when selecting blocks in Navigation list …
getdave Aug 14, 2025
8b58014
refactor: rename setInsertedBlock prop to onBlockInsert for better cl…
getdave Aug 14, 2025
6c586aa
fix: add iframe-aware cleanup for LinkUI in Navigation block List View
getdave Aug 14, 2025
12c1f84
fix: add iframe-aware cleanup for LinkUI in Navigation block List View
getdave Aug 14, 2025
ccc9ad8
Fix test to expect directly inserted page link
getdave Aug 15, 2025
f4a47ea
Fix another test to expect direct inserted Page block
getdave Aug 15, 2025
c5b206c
tweak comments
getdave Aug 15, 2025
05c9a62
Remove hacky fix to onClose failure
getdave Aug 15, 2025
5205281
Revert alteration to hook
getdave Aug 15, 2025
aa509f4
Optimise useSelect calls
getdave Sep 3, 2025
b9cb6eb
Simplify cleanup function to avoid passing functions around
getdave Sep 3, 2025
12ceb1e
Fix mess up from rebase
getdave Sep 3, 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
4 changes: 1 addition & 3 deletions packages/block-editor/src/components/inserter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,9 +386,7 @@ export default compose( [
);

if ( onSelectOrClose ) {
onSelectOrClose( {
clientId: blockToInsert?.clientId,
} );
onSelectOrClose( blockToInsert );
Copy link
Contributor Author

@getdave getdave Aug 11, 2025

Choose a reason for hiding this comment

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

This will be backwards compat as the new object will still contain clientId.

}

const message = sprintf(
Expand Down
19 changes: 13 additions & 6 deletions packages/block-editor/src/components/list-view/appender.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@ export const Appender = forwardRef(
const { insertedBlock, setInsertedBlock } = useListViewContext();

const instanceId = useInstanceId( Appender );
const hideInserter = useSelect(
const { directInsert, hideInserter } = useSelect(
( select ) => {
const { getTemplateLock, isZoomOut } = unlock(
select( blockEditorStore )
);
const { getBlockListSettings, getTemplateLock, isZoomOut } =
unlock( select( blockEditorStore ) );

return !! getTemplateLock( clientId ) || isZoomOut();
const settings = getBlockListSettings( clientId );
const directInsertValue = settings?.directInsert || false;
const hideInserterValue =
!! getTemplateLock( clientId ) || isZoomOut();

return {
directInsert: directInsertValue,
hideInserter: hideInserterValue,
};
},
[ clientId ]
);
Expand Down Expand Up @@ -79,7 +86,7 @@ export const Appender = forwardRef(
position="bottom right"
isAppender
selectBlockOnInsert={ false }
shouldDirectInsert={ false }
shouldDirectInsert={ directInsert }
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we don't have any instances where this is false anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should mirror the block list settings.

__experimentalIsQuick
{ ...props }
toggleProps={ { 'aria-describedby': descriptionId } }
Expand Down
6 changes: 4 additions & 2 deletions packages/block-library/src/navigation-link/link-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function getSuggestionsQuery( type, kind ) {
}
}

function LinkUIBlockInserter( { clientId, onBack } ) {
function LinkUIBlockInserter( { clientId, onBack, onBlockInsert } ) {
const { rootBlockClientId } = useSelect(
( select ) => {
const { getBlockRootClientId } = select( blockEditorStore );
Expand Down Expand Up @@ -135,7 +135,8 @@ function LinkUIBlockInserter( { clientId, onBack } ) {
clientId={ clientId }
isAppender={ false }
prioritizePatterns={ false }
selectBlockOnInsert
selectBlockOnInsert={ ! onBlockInsert }
onSelect={ onBlockInsert ? onBlockInsert : undefined }
hasSearch={ false }
/>
</div>
Expand Down Expand Up @@ -250,6 +251,7 @@ function UnforwardedLinkUI( props, ref ) {
setFocusAddBlockButton( true );
setFocusAddPageButton( false );
} }
onBlockInsert={ props?.onBlockInsert }
/>
) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ export default function LeafMoreMenu( props ) {
<AddSubmenuItem
block={ block }
onClose={ onClose }
expanded
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The expanded prop was being passed to AddSubmenuItem but never actually used in the component. The function only uses expandedState and expand props for its logic. This was a cleanup to remove dead code that was likely leftover from an earlier implementation.

expandedState={ props.expandedState }
expand={ props.expand }
setInsertedBlock={ props.setInsertedBlock }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const BLOCKS_WITH_LINK_UI_SUPPORT = [
const { PrivateListView } = unlock( blockEditorPrivateApis );

function AdditionalBlockContent( { block, insertedBlock, setInsertedBlock } ) {
const { updateBlockAttributes } = useDispatch( blockEditorStore );
const { updateBlockAttributes, removeBlock } =
useDispatch( blockEditorStore );

const supportsLinkControls = BLOCKS_WITH_LINK_UI_SUPPORT?.includes(
insertedBlock?.name
Expand All @@ -47,6 +48,27 @@ function AdditionalBlockContent( { block, insertedBlock, setInsertedBlock } ) {
return null;
}

/**
* Cleanup function for auto-inserted Navigation Link blocks.
*
* Removes the block if it has no URL and clears the inserted block state.
* This ensures consistent cleanup behavior across different contexts.
*/
const cleanupInsertedBlock = () => {
// Prevent automatic block selection when removing blocks in list view context
// This avoids focus stealing that would close the list view and switch to canvas
const shouldAutoSelectBlock = false;

// Follows the exact same pattern as Navigation Link block's onClose handler
// If there is no URL then remove the auto-inserted block to avoid empty blocks
if ( ! insertedBlock?.attributes?.url && insertedBlock?.clientId ) {
// Remove the block entirely to avoid poor UX
// This matches the Navigation Link block's behavior
removeBlock( insertedBlock.clientId, shouldAutoSelectBlock );
}
setInsertedBlock( null );
};

const setInsertedBlockAttributes =
( _insertedBlockClientId ) => ( _updatedAttributes ) => {
if ( ! _insertedBlockClientId ) {
Expand All @@ -55,12 +77,28 @@ function AdditionalBlockContent( { block, insertedBlock, setInsertedBlock } ) {
updateBlockAttributes( _insertedBlockClientId, _updatedAttributes );
};

// Wrapper function to clean up original block when a new block is selected
const handleSetInsertedBlock = ( newBlock ) => {
// Prevent automatic block selection when removing blocks in list view context
// This avoids focus stealing that would close the list view and switch to canvas
const shouldAutoSelectBlock = false;

// If we have an existing inserted block and a new block is being set,
// remove the original block to avoid duplicates
if ( insertedBlock?.clientId && newBlock ) {
removeBlock( insertedBlock.clientId, shouldAutoSelectBlock );
}
setInsertedBlock( newBlock );
};

return (
<LinkUI
clientId={ insertedBlock?.clientId }
link={ insertedBlock?.attributes }
onBlockInsert={ handleSetInsertedBlock }
onClose={ () => {
setInsertedBlock( null );
// Use cleanup function
cleanupInsertedBlock();
} }
onChange={ ( updatedValue ) => {
updateAttributes(
Expand All @@ -70,9 +108,6 @@ function AdditionalBlockContent( { block, insertedBlock, setInsertedBlock } ) {
);
setInsertedBlock( null );
} }
onCancel={ () => {
setInsertedBlock( null );
} }
/>
);
}
Expand Down
59 changes: 10 additions & 49 deletions test/e2e/specs/editor/blocks/navigation-list-view.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,31 +171,8 @@ test.describe( 'Navigation block - List view editing', () => {

await appender.click();

// Expect to see the block inserter.
await expect(
page.getByRole( 'searchbox', {
name: 'Search',
} )
).toBeFocused();

const blockResults = page.getByRole( 'listbox', {
name: 'Blocks',
} );

await expect( blockResults ).toBeVisible();

const blockResultOptions = blockResults.getByRole( 'option' );

// Expect to see the Page Link and Custom Link blocks as the nth(0) and nth(1) results.
// This is important for usability as the Page Link block is the most likely to be used.
await expect( blockResultOptions.nth( 0 ) ).toHaveText( 'Page Link' );
await expect( blockResultOptions.nth( 1 ) ).toHaveText( 'Custom Link' );

// Select the Page Link option.
const customLinkResult = blockResultOptions.nth( 1 );
await customLinkResult.click();

// Expect to see the Link creation UI be focused.
// Expect a Navigation Link block to be inserted
// and immediately trigger its Link UI.
const linkUIInput = linkControl.getSearchInput();

// Coverage for bug whereby Link UI input would be incorrectly prepopulated.
Expand All @@ -215,17 +192,17 @@ test.describe( 'Navigation block - List view editing', () => {
const thirdResult = await linkControl.getNthSearchResult( 2 );

const firstResultType =
await linkControl.getSearchResultType( firstResult );
await linkControl.getSearchResultText( firstResult );

const secondResultType =
await linkControl.getSearchResultType( secondResult );
await linkControl.getSearchResultText( secondResult );

const thirdResultType =
await linkControl.getSearchResultType( thirdResult );
await linkControl.getSearchResultText( thirdResult );

expect( firstResultType ).toBe( 'Page' );
expect( secondResultType ).toBe( 'Page' );
expect( thirdResultType ).toBe( 'Page' );
expect( firstResultType ).toContain( 'Page' );
expect( secondResultType ).toContain( 'Page' );
expect( thirdResultType ).toContain( 'Page' );

// Grab the text from the first result so we can check (later on) that it was inserted.
const firstResultText =
Expand Down Expand Up @@ -503,16 +480,8 @@ test.describe( 'Navigation block - List view editing', () => {
} )
.click();

const blockResults = page.getByRole( 'listbox', {
name: 'Blocks',
} );

await expect( blockResults ).toBeVisible();

const blockResultOptions = blockResults.getByRole( 'option' );

// Select the Page Link option.
await blockResultOptions.nth( 0 ).click();
// Expect the Link UI to be focused.
await expect( linkControl.getSearchInput() ).toBeFocused();

// Immediately dismiss the Link UI thereby not populating the `url` attribute
// of the block.
Expand Down Expand Up @@ -631,12 +600,4 @@ class LinkControl {
.locator( '.components-menu-item__item' ) // this is the only way to get the label text without the URL.
.innerText();
}

async getSearchResultType( result ) {
await expect( result ).toBeVisible();

return result
.locator( '.components-menu-item__shortcut' ) // this is the only way to get the type text.
.innerText();
}
}
Loading