From b9ece22afd3d69b807647729d06d06b0b17def44 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 12 Nov 2025 12:46:58 -0600 Subject: [PATCH 01/14] Add tests for selecting new block when another navigation link popover is open --- .../specs/editor/blocks/navigation.spec.js | 140 +++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-) diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 6489f5b37658ce..88a175c6a00683 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -296,13 +296,15 @@ test.describe( 'Navigation block', () => { } ); test.describe( 'Focus management', () => { + let catPage, dogPage; + test.beforeAll( async ( { requestUtils } ) => { // We need pages to be published so the Link Control can return pages - await requestUtils.createPage( { + catPage = await requestUtils.createPage( { title: 'Cat', status: 'publish', } ); - await requestUtils.createPage( { + dogPage = await requestUtils.createPage( { title: 'Dog', status: 'publish', } ); @@ -683,6 +685,140 @@ test.describe( 'Navigation block', () => { expect( focusIsInSidebar ).toBe( true ); } ); + + test( 'Selecting a new block from another link with a popover open should respect the new block selection', async ( { + editor, + page, + pageUtils, + navigation, + requestUtils, + } ) => { + let inspectorNavigationLabel, + catLinkText, + dogLinkText, + linkPopover, + unavailableLinkText; + // Test setup step + await test.step( 'Test setup', async () => { + const nonExistentPageId = 99999; + // Create a menu with three links: + // 1. Invalid synced link (deleted page) + // 2. Valid synced link (Cat page) + // 3. Valid synced link (Dog page) + // 4. Custom URL link (example.com) + const menu = await requestUtils.createNavigationMenu( { + title: 'Test Menu with Unavailable Entity, Synced Cat Page, and Custom URL', + content: ` + + +`, + } ); + + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: menu.id, + }, + } ); + + // Open the insepctor sidebar, as this is the easiest way to visually see block selection + await editor.openDocumentSettingsSidebar(); + + // CRITICAL: Wait for synced link entities to load BEFORE interacting with them + // Synced links load their URLs asynchronously. If we click them before the URLs + // are loaded, the popover opens in edit mode (url: null) instead of preview mode. + // Select the Cat link temporarily to check if its URL is loaded in the sidebar. + catLinkText = editor.canvas + .getByRole( 'textbox', { + name: 'Navigation link text', + } ) + .filter( { hasText: 'Cat' } ); + await catLinkText.click(); + + const linkInput = page + .getByRole( 'tabpanel', { name: 'Settings' } ) + .getByRole( 'textbox', { + name: 'Link', + } ); + // Wait for the Cat link's URL to load + await expect( linkInput ).not.toHaveValue( '' ); + await expect( linkInput ).toBeDisabled(); // Synced links have disabled Link field + } ); + + await test.step( 'Popover closing from unsynced link to a synced link should not steal focus back to the previously selected (Cat) link', async () => { + // Cat link is already selected from setup step, with entity loaded + // Verify sidebar shows Cat + inspectorNavigationLabel = page + .getByRole( 'tabpanel', { name: 'Settings' } ) + .getByRole( 'textbox', { + name: 'Text', + } ); + await expect( inspectorNavigationLabel ).toHaveValue( 'Cat' ); + + await pageUtils.pressKeys( 'primary+k' ); + linkPopover = navigation.getLinkPopover(); + await expect( linkPopover ).toBeVisible(); + + // Check that the popover has focus on the Cat link + await expect( + linkPopover.getByRole( 'link', { + name: 'Cat (Opens in a new tab)', + } ) + ).toBeFocused(); + + dogLinkText = editor.canvas + .getByRole( 'textbox', { + name: 'Navigation link text', + } ) + .filter( { hasText: 'Dog' } ); + await dogLinkText.click(); + + // Verify the popover is closed + await expect( linkPopover ).toBeHidden(); + // Check that the Label in the inspector sidebar is Dog + await expect( inspectorNavigationLabel ).toHaveValue( 'Dog' ); + } ); + + await test.step( 'Popover closing from synced (Dog) link to an unsynced link should not steal focus back to the previously selected (Dog) link', async () => { + await pageUtils.pressKeys( 'primary+k' ); + await expect( linkPopover ).toBeVisible(); + + // Check that the popover has focus on the Dog link + await expect( + linkPopover.getByRole( 'link', { + name: 'Dog (Opens in a new tab)', + } ) + ).toBeFocused(); + + unavailableLinkText = editor.canvas + .locator( 'a' ) + .filter( { hasText: 'Unavailable Page (Invalid)' } ); + await unavailableLinkText.click(); + + // Check that the Label in the inspector sidebar is Unavailable Page + await expect( inspectorNavigationLabel ).toHaveValue( + 'Unavailable Page' + ); + } ); + + await test.step( 'Selecting a new block from a invalid synced link with a popover open should respect the new block selection', async () => { + // Verify the popover is visible (we want the invalid link click to have opened the popover) + await expect( linkPopover ).toBeVisible(); + await expect( + linkPopover.getByRole( 'combobox', { + name: 'Search or type URL', + } ) + ).toBeFocused(); + // Check that the popover has focus in the editable link state + + await catLinkText.click(); + + // Verify the popover is closed + await expect( linkPopover ).toBeHidden(); + // Check that the Label in the inspector sidebar is Cat + await expect( inspectorNavigationLabel ).toHaveValue( 'Cat' ); + } ); + } ); } ); test( 'Adding new links to a navigation block with existing inner blocks triggers creation of a single Navigation Menu', async ( { From 79686f877e718200b22ab4ec86787b8f4ee2263a Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 12 Nov 2025 12:48:54 -0600 Subject: [PATCH 02/14] Fix previous block stealing focus when deselecting block when popover is open --- packages/block-library/src/navigation-link/edit.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 6efd869daa0cf7..fcaba384165c1c 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -214,6 +214,8 @@ export default function NavigationLinkEdit( { const ref = useRef(); const linkUIref = useRef(); const prevUrl = usePrevious( url ); + // A link is "new" only if it has no URL and no URL binding + // If it has a URL binding (synced link), it's not new even if the URL is empty (e.g., deleted page) const isNewLink = useRef( ! url && ! metadata?.bindings?.url ); const { @@ -314,7 +316,7 @@ export default function NavigationLinkEdit( { // If we leave focus on this block, then when we close the link without creating a link, focus will // be lost during the new block selection process. useEffect( () => { - if ( isNewLink.current && isSelected && ! url ) { + if ( isNewLink.current && isSelected ) { selectBlock( parentBlockClientId ); } }, [] ); // eslint-disable-line react-hooks/exhaustive-deps @@ -445,7 +447,7 @@ export default function NavigationLinkEdit( { ); if ( - ! url || + ( ! url && ! ( hasUrlBinding && isBoundEntityAvailable ) ) || isInvalid || isDraft || ( hasUrlBinding && ! isBoundEntityAvailable ) @@ -457,7 +459,7 @@ export default function NavigationLinkEdit( { const classes = clsx( 'wp-block-navigation-item__content', { 'wp-block-navigation-link__placeholder': - ! url || + ( ! url && ! ( hasUrlBinding && isBoundEntityAvailable ) ) || isInvalid || isDraft || ( hasUrlBinding && ! isBoundEntityAvailable ), @@ -592,6 +594,8 @@ export default function NavigationLinkEdit( { } else if ( isNewLink.current ) { // If we just created a new link, select it selectBlock( clientId ); + // Mark as no longer new so we don't re-select on subsequent popover closes + isNewLink.current = false; } } } anchor={ popoverAnchor } From 3c6df97c475d08e1c9c91548dee1baf1dd9154af Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 13 Nov 2025 11:44:44 -0600 Subject: [PATCH 03/14] Use undefined label instead of empty url checks --- packages/block-library/src/navigation-link/edit.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index fcaba384165c1c..42c643c921d988 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -214,9 +214,9 @@ export default function NavigationLinkEdit( { const ref = useRef(); const linkUIref = useRef(); const prevUrl = usePrevious( url ); - // A link is "new" only if it has no URL and no URL binding - // If it has a URL binding (synced link), it's not new even if the URL is empty (e.g., deleted page) - const isNewLink = useRef( ! url && ! metadata?.bindings?.url ); + // A link is "new" only if it has an undefined label + // After the link is created, even if no label is provided, it's set to an empty string. + const isNewLink = useRef( label === undefined ); const { isAtMaxNesting, From 21b4a049de159d3533f323da9767b4dadddc9028 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 13 Nov 2025 12:21:50 -0600 Subject: [PATCH 04/14] Add failing test to check for category link popover focus and respecting block selection --- .../specs/editor/blocks/navigation.spec.js | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 88a175c6a00683..4e5506c43cfcb6 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -818,6 +818,67 @@ test.describe( 'Navigation block', () => { // Check that the Label in the inspector sidebar is Cat await expect( inspectorNavigationLabel ).toHaveValue( 'Cat' ); } ); + + await test.step( 'Creating a new category link should respect new block selection', async () => { + // Use the block inserter to add a new category link + await editor.canvas + .getByRole( 'button', { name: 'Add block' } ) + .click(); + + // Verify the popover is visible (we want the invalid link click to have opened the popover) + await expect( linkPopover ).toBeVisible(); + await expect( + linkPopover.getByRole( 'combobox', { + name: 'Search or type URL', + } ) + ).toBeFocused(); + + await linkPopover + .getByRole( 'button', { name: 'Add block' } ) + .click(); + + const addBlockDialog = page.getByRole( 'dialog', { + name: 'Add block', + } ); + + await expect( addBlockDialog ).toBeVisible(); + + await expect( + addBlockDialog.getByRole( 'button', { name: 'Back' } ) + ).toBeFocused(); + + await addBlockDialog + .getByRole( 'option', { name: 'Custom Link' } ) + .click(); + + await expect( + linkPopover.getByRole( 'combobox', { + name: 'Search or type URL', + } ) + ).toBeFocused(); + + await page.keyboard.type( 'Uncategorized' ); + + await expect( + linkPopover.getByRole( 'option', { name: 'Uncategorized' } ) + ).toBeVisible(); + + await pageUtils.pressKeys( 'ArrowDown' ); + await page.keyboard.press( 'Enter' ); + + await expect( + linkPopover.getByRole( 'link', { + name: 'Uncategorized (opens in a new tab)', + } ) + ).toBeFocused(); + + await catLinkText.click(); + + // Verify the popover is closed + await expect( linkPopover ).toBeHidden(); + // Check that the Label in the inspector sidebar is Cat + await expect( inspectorNavigationLabel ).toHaveValue( 'Cat' ); + } ); } ); } ); From a31060b4572d416eb16038e3bc54fee8538289ea Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 13 Nov 2025 13:54:57 -0600 Subject: [PATCH 05/14] Update tests to check for category link popover deselection --- .../specs/editor/blocks/navigation.spec.js | 70 +++++++------------ 1 file changed, 26 insertions(+), 44 deletions(-) diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 4e5506c43cfcb6..e13c6be8d5c2bc 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -698,6 +698,7 @@ test.describe( 'Navigation block', () => { dogLinkText, linkPopover, unavailableLinkText; + // Test setup step await test.step( 'Test setup', async () => { const nonExistentPageId = 99999; @@ -732,7 +733,7 @@ test.describe( 'Navigation block', () => { .getByRole( 'textbox', { name: 'Navigation link text', } ) - .filter( { hasText: 'Cat' } ); + .filter( { hasText: /^Cat$/ } ); await catLinkText.click(); const linkInput = page @@ -758,13 +759,11 @@ test.describe( 'Navigation block', () => { await pageUtils.pressKeys( 'primary+k' ); linkPopover = navigation.getLinkPopover(); await expect( linkPopover ).toBeVisible(); + const catPopoverLink = navigation.getLinkControlLink( 'Cat' ); + await expect( catPopoverLink ).toBeVisible(); // Check that the popover has focus on the Cat link - await expect( - linkPopover.getByRole( 'link', { - name: 'Cat (Opens in a new tab)', - } ) - ).toBeFocused(); + await expect( catPopoverLink ).toBeFocused(); dogLinkText = editor.canvas .getByRole( 'textbox', { @@ -783,12 +782,11 @@ test.describe( 'Navigation block', () => { await pageUtils.pressKeys( 'primary+k' ); await expect( linkPopover ).toBeVisible(); - // Check that the popover has focus on the Dog link - await expect( - linkPopover.getByRole( 'link', { - name: 'Dog (Opens in a new tab)', - } ) - ).toBeFocused(); + const dogPopoverLink = navigation.getLinkControlLink( 'Dog' ); + await expect( dogPopoverLink ).toBeVisible(); + + // Check that the popover has focus on the Cat link + await expect( dogPopoverLink ).toBeFocused(); unavailableLinkText = editor.canvas .locator( 'a' ) @@ -851,26 +849,7 @@ test.describe( 'Navigation block', () => { .getByRole( 'option', { name: 'Custom Link' } ) .click(); - await expect( - linkPopover.getByRole( 'combobox', { - name: 'Search or type URL', - } ) - ).toBeFocused(); - - await page.keyboard.type( 'Uncategorized' ); - - await expect( - linkPopover.getByRole( 'option', { name: 'Uncategorized' } ) - ).toBeVisible(); - - await pageUtils.pressKeys( 'ArrowDown' ); - await page.keyboard.press( 'Enter' ); - - await expect( - linkPopover.getByRole( 'link', { - name: 'Uncategorized (opens in a new tab)', - } ) - ).toBeFocused(); + await navigation.useLinkControlSearch( 'Uncategorized' ); await catLinkText.click(); @@ -1703,9 +1682,21 @@ class Navigation { * @param {string} label Text of page you want added. Must be a part of the pages added in the beforeAll in this test suite. */ async addPage( label ) { - const linkControlSearch = this.page.getByRole( 'combobox', { - name: 'Search or type URL', - } ); + await this.useLinkControlSearch( label ); + + const linkControlLink = await this.getLinkControlLink( label ); + + await expect( linkControlLink ).toBeVisible(); + await expect( linkControlLink ).toBeFocused(); + + await this.page.keyboard.press( 'Escape' ); + await expect( this.getLinkControlSearch() ).toBeHidden(); + + await this.checkLabelFocus( label ); + } + + async useLinkControlSearch( label ) { + const linkControlSearch = this.getLinkControlSearch(); await expect( linkControlSearch ).toBeFocused(); @@ -1721,15 +1712,6 @@ class Navigation { await this.pageUtils.pressKeys( 'ArrowDown' ); await this.page.keyboard.press( 'Enter' ); - - const linkControlLink = await this.getLinkControlLink( label ); - await expect( linkControlLink ).toBeFocused(); - - await this.page.keyboard.press( 'Escape' ); - - await expect( linkControlSearch ).toBeHidden(); - - await this.checkLabelFocus( label ); } /** From 2b621e84b7083c4c8764e3eed018f4a9ac2ab2b5 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 13 Nov 2025 14:23:55 -0600 Subject: [PATCH 06/14] More strict check for moving focus to the new block --- .../block-library/src/navigation-link/edit.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 42c643c921d988..a12cfa2091ab2d 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -273,7 +273,8 @@ export default function NavigationLinkEdit( { }, [ clientId, maxNestingLevel ] ); - const { getBlocks } = useSelect( blockEditorStore ); + const { getBlocks, getSelectedBlockClientId } = + useSelect( blockEditorStore ); // URL binding logic const { @@ -592,9 +593,16 @@ export default function NavigationLinkEdit( { if ( ! url && ! hasUrlBinding ) { onReplace( [] ); } else if ( isNewLink.current ) { - // If we just created a new link, select it - selectBlock( clientId ); - // Mark as no longer new so we don't re-select on subsequent popover closes + // If we just created a new link, select it ONLY if another block wasn't already selected + // When the user clicks another block, that block gets selected BEFORE onClose is called + // So we check: if this block is still selected, user closed via Escape; otherwise they clicked another block + if ( + getSelectedBlockClientId() === + parentBlockClientId + ) { + selectBlock( clientId ); + } + // Mark as no longer new isNewLink.current = false; } } } From 26ae3ab34ed6115c79ada076db8b986304a686ac Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Fri, 14 Nov 2025 07:19:41 -0600 Subject: [PATCH 07/14] selectBlock in onChange instead of onClose --- .../block-library/src/navigation-link/edit.js | 65 ++++++++++++------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index a12cfa2091ab2d..85928d1ca7ab17 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -30,7 +30,7 @@ import { useState, useEffect, useRef, useCallback } from '@wordpress/element'; import { decodeEntities } from '@wordpress/html-entities'; import { link as linkIcon, addSubmenu } from '@wordpress/icons'; import { store as coreStore } from '@wordpress/core-data'; -import { useMergeRefs, usePrevious, useInstanceId } from '@wordpress/compose'; +import { useMergeRefs, useInstanceId } from '@wordpress/compose'; /** * Internal dependencies @@ -213,7 +213,6 @@ export default function NavigationLinkEdit( { const itemLabelPlaceholder = __( 'Add label…' ); const ref = useRef(); const linkUIref = useRef(); - const prevUrl = usePrevious( url ); // A link is "new" only if it has an undefined label // After the link is created, even if no label is provided, it's set to an empty string. const isNewLink = useRef( label === undefined ); @@ -336,20 +335,37 @@ export default function NavigationLinkEdit( { transformToSubmenu, ] ); - // If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label text. + // Handle link UI when a new link is created useEffect( () => { - // We only want to do this when the URL has gone from nothing to a new URL AND the label looks like a URL - if ( - ! prevUrl && - url && - isLinkOpen && - isURL( prependHTTP( label ) ) && - /^.+\.[a-z]+/.test( label ) - ) { + // We know if a link was just created if: + // 1. isNewLink.current is true + // 2. url has a value + if ( ! isNewLink.current || ! url ) { + return; + } + + // If the link UI is open, then focus has not resolved from the selectBlock in the onChange. + // We defer handling any UI updates until the focus has resolved and the link UI is closed. + if ( isLinkOpen ) { + return; + } + + // Ensure this only runs once + isNewLink.current = false; + + // We just created a link and the block is now selected. + // The LinkUI is closed due to the selectBlock from the onChange. + // If the label looks like a URL, focus and select the label text. + // Otherwise, reopen the link UI so the popover is visible in the preview state + if ( isURL( prependHTTP( label ) ) && /^.+\.[a-z]+/.test( label ) ) { // Focus and select the label text. selectLabelText(); + } else { + // The link was just created and is now selected which closed the popover. + // Re-open the link UI so the popover is visible in the preview state + setIsLinkOpen( true ); } - }, [ prevUrl, url, isLinkOpen, label ] ); + }, [ url, isLinkOpen, isNewLink, label ] ); /** * Focus the Link label text and select it. @@ -592,18 +608,6 @@ export default function NavigationLinkEdit( { // Don't remove if binding exists (even if entity is unavailable) so user can fix it. if ( ! url && ! hasUrlBinding ) { onReplace( [] ); - } else if ( isNewLink.current ) { - // If we just created a new link, select it ONLY if another block wasn't already selected - // When the user clicks another block, that block gets selected BEFORE onClose is called - // So we check: if this block is still selected, user closed via Escape; otherwise they clicked another block - if ( - getSelectedBlockClientId() === - parentBlockClientId - ) { - selectBlock( clientId ); - } - // Mark as no longer new - isNewLink.current = false; } } } anchor={ popoverAnchor } @@ -626,6 +630,19 @@ export default function NavigationLinkEdit( { } else { clearBinding(); } + + // If the link was just created, we want to select the block so the inspector controls + // are accurate. + if ( + isNewLink.current && + updatedAttributes.url && + clientId !== getSelectedBlockClientId() + ) { + // Note: selectBlock will close the link UI if it is open, so we handle that + // separately by re-opening the link UI in a useEffect. + // TODO: Prevent the link UI from being closed by selectBlock. + selectBlock( clientId ); + } } } /> ) } From 4aa39ff44fbe7b8b7dc255097180f344ed0eed11 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Fri, 14 Nov 2025 07:24:39 -0600 Subject: [PATCH 08/14] Add check that link ui is closed after adding a custom url --- test/e2e/specs/editor/blocks/navigation.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index e13c6be8d5c2bc..c1c10c916c94f8 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -383,6 +383,8 @@ test.describe( 'Navigation block', () => { await navigation.useBlockInserter(); await navigation.addCustomURL( 'https://example.com' ); await navigation.expectToHaveTextSelected( 'example.com' ); + // The link UI should be closed when creating a custom link + await expect( navigation.getLinkPopover() ).toBeHidden(); } ); await test.step( 'we can open and close the preview with the keyboard and escape buttons from a top-level nav link with a url-like label using both the shortcut and toolbar', async () => { From ad70be00a63cb514cd664ac2428d413c2ef42921 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Mon, 17 Nov 2025 11:41:28 -0600 Subject: [PATCH 09/14] Focus submenu appender on first submenu item creation and update tests --- .../block-library/src/navigation-link/edit.js | 76 ++++++++++++------- .../specs/editor/blocks/navigation.spec.js | 33 ++++++-- 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 85928d1ca7ab17..858ee10ececf27 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -216,6 +216,8 @@ export default function NavigationLinkEdit( { // A link is "new" only if it has an undefined label // After the link is created, even if no label is provided, it's set to an empty string. const isNewLink = useRef( label === undefined ); + // Track whether we should focus the submenu appender when closing the link UI + const shouldSelectSubmenuAppenderOnClose = useRef( false ); const { isAtMaxNesting, @@ -224,6 +226,7 @@ export default function NavigationLinkEdit( { hasChildren, validateLinkStatus, parentBlockClientId, + isSubmenu, } = useSelect( ( select ) => { const { @@ -268,12 +271,12 @@ export default function NavigationLinkEdit( { hasChildren: !! getBlockCount( clientId ), validateLinkStatus: enableLinkStatusValidation, parentBlockClientId: parentBlockId, + isSubmenu: parentBlockName === 'core/navigation-submenu', }; }, [ clientId, maxNestingLevel ] ); - const { getBlocks, getSelectedBlockClientId } = - useSelect( blockEditorStore ); + const { getBlocks } = useSelect( blockEditorStore ); // URL binding logic const { @@ -344,26 +347,37 @@ export default function NavigationLinkEdit( { return; } - // If the link UI is open, then focus has not resolved from the selectBlock in the onChange. - // We defer handling any UI updates until the focus has resolved and the link UI is closed. - if ( isLinkOpen ) { - return; - } - // Ensure this only runs once isNewLink.current = false; // We just created a link and the block is now selected. - // The LinkUI is closed due to the selectBlock from the onChange. // If the label looks like a URL, focus and select the label text. - // Otherwise, reopen the link UI so the popover is visible in the preview state - if ( isURL( prependHTTP( label ) ) && /^.+\.[a-z]+/.test( label ) ) { + if ( + isURL( prependHTTP( label ) ) && + /^.+\.[a-z]+/.test( label ) && + isLinkOpen + ) { // Focus and select the label text. selectLabelText(); } else { - // The link was just created and is now selected which closed the popover. - // Re-open the link UI so the popover is visible in the preview state - setIsLinkOpen( true ); + // If the link was just created, we want to select the block so the inspector controls + // are accurate. + selectBlock( clientId, null ); + + // Edge case: When the created link is the first child of a submenu, the focus will have + // originated from the add submenu toolbar button. In this case, we need to return focus + // to the submenu appender if the user closes the link ui using the keyboard. + // Check if this is the first and only child of a newly created submenu + if ( isSubmenu ) { + const parentBlocks = getBlocks( parentBlockClientId ); + // Only set the flag if this is the only child (meaning new submenu) + if ( + parentBlocks.length === 1 && + parentBlocks[ 0 ].clientId === clientId + ) { + shouldSelectSubmenuAppenderOnClose.current = true; + } + } } }, [ url, isLinkOpen, isNewLink, label ] ); @@ -608,6 +622,27 @@ export default function NavigationLinkEdit( { // Don't remove if binding exists (even if entity is unavailable) so user can fix it. if ( ! url && ! hasUrlBinding ) { onReplace( [] ); + return; + } + + // Edge case: If this is the first child of a new submenu, focus the submenu's appender + if ( + shouldSelectSubmenuAppenderOnClose.current + ) { + shouldSelectSubmenuAppenderOnClose.current = false; + + // The appender is the next sibling in the DOM after the current block + if ( + listItemRef.current?.nextElementSibling + ) { + const appenderButton = + listItemRef.current.nextElementSibling.querySelector( + '.block-editor-button-block-appender' + ); + if ( appenderButton ) { + appenderButton.focus(); + } + } } } } anchor={ popoverAnchor } @@ -630,19 +665,6 @@ export default function NavigationLinkEdit( { } else { clearBinding(); } - - // If the link was just created, we want to select the block so the inspector controls - // are accurate. - if ( - isNewLink.current && - updatedAttributes.url && - clientId !== getSelectedBlockClientId() - ) { - // Note: selectBlock will close the link UI if it is open, so we handle that - // separately by re-opening the link UI in a useEffect. - // TODO: Prevent the link UI from being closed by selectBlock. - selectBlock( clientId ); - } } } /> ) } diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index c1c10c916c94f8..c34f7fd00655b6 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -355,6 +355,7 @@ test.describe( 'Navigation block', () => { await pageUtils.pressKeys( 'ArrowDown' ); await navigation.useBlockInserter(); await navigation.addPage( 'Cat' ); + await pageUtils.pressKeys( 'ArrowLeft', { times: 2 } ); } ); await test.step( 'can use the shortcut to open the preview with the keyboard and escape keypress sends focus back to the navigation link block', async () => { @@ -424,7 +425,9 @@ test.describe( 'Navigation block', () => { // TODO: Use Enter after that bug is resolved await navigation.useLinkShortcut(); - await navigation.addPage( 'Dog' ); + await navigation.addSubmenuPage( 'Dog' ); + await pageUtils.pressKeys( 'ArrowUp' ); + await pageUtils.pressKeys( 'ArrowRight' ); } ); await test.step( 'can use the shortcut to open the preview with the keyboard and escape keypress sends focus back to the navigation link block in the submenu', async () => { @@ -537,8 +540,6 @@ test.describe( 'Navigation block', () => { await pageUtils.pressKeys( 'ArrowDown' ); await navigation.useBlockInserter(); await navigation.addPage( 'Cat' ); - await pageUtils.pressKeys( 'ArrowDown' ); - await pageUtils.pressKeys( 'ArrowRight', { times: 2 } ); await navigation.useBlockInserter(); await navigation.addCustomURL( 'https://example.com' ); await navigation.expectToHaveTextSelected( 'example.com' ); @@ -554,8 +555,6 @@ test.describe( 'Navigation block', () => { // TODO: Use Enter after that bug is resolved await navigation.useLinkShortcut(); await navigation.addPage( 'Dog' ); - await page.keyboard.press( 'End' ); - await pageUtils.pressKeys( 'ArrowRight', { times: 2 } ); await navigation.useBlockInserter(); await navigation.addCustomURL( 'https://wordpress.org' ); await navigation.expectToHaveTextSelected( 'wordpress.org' ); @@ -1026,6 +1025,7 @@ test.describe( 'Navigation block', () => { await pageUtils.pressKeys( 'ArrowDown' ); await navigation.useBlockInserter(); await navigation.addPage( 'Test Page 1' ); + await pageUtils.pressKeys( 'ArrowLeft', { times: 2 } ); } ); await test.step( 'Verify bound link displays correctly in Link UI popover', async () => { @@ -1630,6 +1630,12 @@ class Navigation { return this.getNavBlock().getByLabel( 'Add block' ).first(); } + getSubmenuBlockInserter() { + return this.editor.canvas + .getByRole( 'document', { name: 'Block: Submenu' } ) + .getByLabel( 'Add block' ); + } + getLinkControlSearch() { return this.page.getByRole( 'combobox', { name: 'Search or type URL', @@ -1681,9 +1687,10 @@ class Navigation { * Usage: * - Open the new link control however you'd like (block appender, command+k on Add link label...) * - * @param {string} label Text of page you want added. Must be a part of the pages added in the beforeAll in this test suite. + * @param {string} label Text of page you want added. Must be a part of the pages added in the beforeAll in this test suite. + * @param {boolean} submenu Whether the page is being added to a submenu. */ - async addPage( label ) { + async addPage( label, submenu = false ) { await this.useLinkControlSearch( label ); const linkControlLink = await this.getLinkControlLink( label ); @@ -1694,7 +1701,17 @@ class Navigation { await this.page.keyboard.press( 'Escape' ); await expect( this.getLinkControlSearch() ).toBeHidden(); - await this.checkLabelFocus( label ); + // Check appender has focus + if ( submenu ) { + // chec for the submenu appender + await expect( this.getSubmenuBlockInserter() ).toBeFocused(); + } else { + await expect( this.getNavBlockInserter() ).toBeFocused(); + } + } + + async addSubmenuPage( label ) { + await this.addPage( label, true ); } async useLinkControlSearch( label ) { From 7c68c2a2572e7e451897e929819bfc7bc767b55a Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 18 Nov 2025 11:27:12 -0600 Subject: [PATCH 10/14] Fix comments. --- packages/block-library/src/navigation-link/edit.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 858ee10ececf27..47df76ad9b69ae 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -367,10 +367,11 @@ export default function NavigationLinkEdit( { // Edge case: When the created link is the first child of a submenu, the focus will have // originated from the add submenu toolbar button. In this case, we need to return focus // to the submenu appender if the user closes the link ui using the keyboard. - // Check if this is the first and only child of a newly created submenu + // Check if this is the first and only child of a newly created submenu. if ( isSubmenu ) { const parentBlocks = getBlocks( parentBlockClientId ); - // Only set the flag if this is the only child (meaning new submenu) + // If this is the only child, then this is a new submenu. + // Set the flag to select the submenu appender when the link ui is closed. if ( parentBlocks.length === 1 && parentBlocks[ 0 ].clientId === clientId From 9a0dad87bd34a3c8cda5cf754a92b7a3fcd45b4e Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 18 Nov 2025 11:29:25 -0600 Subject: [PATCH 11/14] Combine lengthy checks into one const --- packages/block-library/src/navigation-link/edit.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 47df76ad9b69ae..6f626af6799dca 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -478,23 +478,20 @@ export default function NavigationLinkEdit( { } ); - if ( + const needsValidLink = ( ! url && ! ( hasUrlBinding && isBoundEntityAvailable ) ) || isInvalid || isDraft || - ( hasUrlBinding && ! isBoundEntityAvailable ) - ) { + ( hasUrlBinding && ! isBoundEntityAvailable ); + + if ( needsValidLink ) { blockProps.onClick = () => { setIsLinkOpen( true ); }; } const classes = clsx( 'wp-block-navigation-item__content', { - 'wp-block-navigation-link__placeholder': - ( ! url && ! ( hasUrlBinding && isBoundEntityAvailable ) ) || - isInvalid || - isDraft || - ( hasUrlBinding && ! isBoundEntityAvailable ), + 'wp-block-navigation-link__placeholder': needsValidLink, } ); const missingText = getMissingText( type ); From a7658740c06f97f28bf0637f62c56f39269d8da3 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 19 Nov 2025 09:00:37 -0600 Subject: [PATCH 12/14] Remove unused isEndingWithFocusRef --- .../block-editor/src/components/link-control/index.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 980447166cc3e2..01944fe6dfb9c8 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -187,7 +187,6 @@ function LinkControl( { const wrapperNode = useRef(); const textInputRef = useRef(); const searchInputRef = useRef(); - const isEndingEditWithFocusRef = useRef( false ); const settingsKeys = settings.map( ( { id } ) => id ); @@ -244,8 +243,6 @@ function LinkControl( { wrapperNode.current; nextFocusTarget.focus(); - - isEndingEditWithFocusRef.current = false; }, [ isEditingLink, isCreatingPage ] ); // The component mounting reference is maintained separately @@ -261,14 +258,9 @@ function LinkControl( { const hasLinkValue = value?.url?.trim()?.length > 0; /** - * Cancels editing state and marks that focus may need to be restored after - * the next render, if focus was within the wrapper when editing finished. + * Cancels editing state. */ const stopEditing = () => { - isEndingEditWithFocusRef.current = !! wrapperNode.current?.contains( - wrapperNode.current.ownerDocument.activeElement - ); - setIsEditingLink( false ); }; From ccf5ca98a7c0abda583e369f4645b0a361192146 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 19 Nov 2025 10:20:34 -0600 Subject: [PATCH 13/14] Fix category preview focus loss when creating a new category link --- .../src/components/link-control/index.js | 38 +++++++++++++++++-- .../specs/editor/blocks/navigation.spec.js | 13 +++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 01944fe6dfb9c8..6442d2569384dd 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -15,7 +15,7 @@ import { __experimentalInputControlSuffixWrapper as InputControlSuffixWrapper, } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; -import { useRef, useState, useEffect } from '@wordpress/element'; +import { useRef, useState, useEffect, useMemo } from '@wordpress/element'; import { useInstanceId } from '@wordpress/compose'; import { focus } from '@wordpress/dom'; import { ENTER } from '@wordpress/keycodes'; @@ -187,6 +187,15 @@ function LinkControl( { const wrapperNode = useRef(); const textInputRef = useRef(); const searchInputRef = useRef(); + // TODO: Remove entityUrlFallbackRef and previewValue in favor of value prop after taxonomy entity binding + // is stable and returns the correct URL instead of null while resolving when creating the entity. + // + // Preserve the URL from entity suggestions before binding overrides it + // This is due to entity binding not being available immediately after the suggestion is selected. + // The URL can return null, especially for taxonomy entities, while entity binding is being resolved. + // To avoid unnecessary rerenders and focus loss, we preserve the URL from the suggestion and use it + // as a fallback until the entity binding is available. + const entityUrlFallbackRef = useRef(); const settingsKeys = settings.map( ( { id } ) => id ); @@ -265,6 +274,11 @@ function LinkControl( { }; const handleSelectSuggestion = ( updatedValue ) => { + // Preserve the URL for taxonomy entities before binding overrides it + if ( updatedValue?.kind === 'taxonomy' && updatedValue?.url ) { + entityUrlFallbackRef.current = updatedValue.url; + } + // Suggestions may contains "settings" values (e.g. `opensInNewTab`) // which should not override any existing settings values set by the // user. This filters out any settings values from the suggestion. @@ -388,6 +402,24 @@ function LinkControl( { const isDisabled = ! valueHasChanges || currentInputIsEmpty; const showSettings = !! settings?.length && isEditingLink && hasLinkValue; + const previewValue = useMemo( () => { + // There is a chance that the value is not yet set from the entity binding, so we use the preserved URL. + if ( + value?.kind === 'taxonomy' && + ! value?.url && + entityUrlFallbackRef.current + ) { + // combine the value prop with the preserved URL from the suggestion + return { + ...value, + url: entityUrlFallbackRef.current, + }; + } + + // If we don't have a fallback URL, use the value prop. + return value; + }, [ value ] ); + return (
setIsEditingLink( true ) } hasRichPreviews={ hasRichPreviews } hasUnlinkControl={ shownUnlinkControl } diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index c34f7fd00655b6..5b9b301cf0c79d 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -852,6 +852,19 @@ test.describe( 'Navigation block', () => { await navigation.useLinkControlSearch( 'Uncategorized' ); + // expect the sidebar to show 'Uncategorized' as the label + await expect( inspectorNavigationLabel ).toHaveValue( + 'Uncategorized' + ); + + await expect( + navigation.getLinkControlLink( 'Uncategorized' ) + ).toBeVisible(); + + await expect( + navigation.getLinkControlLink( 'Uncategorized' ) + ).toBeFocused(); + await catLinkText.click(); // Verify the popover is closed From a7aea0d6aef10dea73429ad0a4af690d405ad28b Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 19 Nov 2025 11:11:30 -0600 Subject: [PATCH 14/14] Only select navigation block when created via our link ui on the canvas --- packages/block-library/src/navigation-link/edit.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 6f626af6799dca..f2f2985b35a171 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -340,10 +340,11 @@ export default function NavigationLinkEdit( { // Handle link UI when a new link is created useEffect( () => { - // We know if a link was just created if: + // We know if a link was just created from our link UI if // 1. isNewLink.current is true // 2. url has a value - if ( ! isNewLink.current || ! url ) { + // 3. isLinkOpen is true + if ( ! isNewLink.current || ! url || ! isLinkOpen ) { return; } @@ -352,11 +353,7 @@ export default function NavigationLinkEdit( { // We just created a link and the block is now selected. // If the label looks like a URL, focus and select the label text. - if ( - isURL( prependHTTP( label ) ) && - /^.+\.[a-z]+/.test( label ) && - isLinkOpen - ) { + if ( isURL( prependHTTP( label ) ) && /^.+\.[a-z]+/.test( label ) ) { // Focus and select the label text. selectLabelText(); } else {