diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index a6c66a7a9d5341..59ab9f30262448 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -191,7 +191,6 @@ function Navigation( { isNavigationMenuResolved, isNavigationMenuMissing, navigationMenus, - navigationMenu, canUserUpdateNavigationMenu, hasResolvedCanUserUpdateNavigationMenu, canUserDeleteNavigationMenu, @@ -240,8 +239,6 @@ function Navigation( { const navRef = useRef(); - const isDraftNavigationMenu = navigationMenu?.status === 'draft'; - const { convert: convertClassicMenu, status: classicMenuConversionStatus, @@ -331,11 +328,6 @@ function Navigation( { ] = useState(); const [ detectedOverlayColor, setDetectedOverlayColor ] = useState(); - const handleUpdateMenu = ( menuId ) => { - setRef( menuId ); - selectBlock( clientId ); - }; - useEffect( () => { hideClassicMenuConversionNotice(); if ( classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_PENDING ) { @@ -426,27 +418,6 @@ function Navigation( { ref, ] ); - const navigationSelectorRef = useRef(); - const [ shouldFocusNavigationSelector, setShouldFocusNavigationSelector ] = - useState( false ); - - // Focus support after menu selection. - useEffect( () => { - if ( - isDraftNavigationMenu || - ! isEntityAvailable || - ! shouldFocusNavigationSelector - ) { - return; - } - navigationSelectorRef?.current?.focus(); - setShouldFocusNavigationSelector( false ); - }, [ - isDraftNavigationMenu, - isEntityAvailable, - shouldFocusNavigationSelector, - ] ); - const isResponsive = 'never' !== overlayMenu; const overlayMenuPreviewClasses = classnames( @@ -598,21 +569,16 @@ function Navigation( { { - handleUpdateMenu( menuId ); - setShouldFocusNavigationSelector( true ); - } } + onSelectNavigationMenu={ setRef } onSelectClassicMenu={ async ( classicMenu ) => { const navMenu = await convertClassicMenu( classicMenu.id, classicMenu.name ); if ( navMenu ) { - handleUpdateMenu( navMenu.id ); - setShouldFocusNavigationSelector( true ); + setRef( navMenu.id ); } } } onCreateNew={ () => createNavigationMenu( '', [] ) } @@ -658,28 +624,22 @@ function Navigation( { } // Show a warning if the selected menu is no longer available. - // TODO - the user should be able to select a new one? if ( ref && isNavigationMenuMissing ) { return ( { - handleUpdateMenu( menuId ); - setShouldFocusNavigationSelector( true ); - } } + onSelectNavigationMenu={ setRef } onSelectClassicMenu={ async ( classicMenu ) => { const navMenu = await convertClassicMenu( classicMenu.id, classicMenu.name ); if ( navMenu ) { - handleUpdateMenu( navMenu.id ); - setShouldFocusNavigationSelector( true ); + setRef( navMenu.id ); } } } onCreateNew={ () => createNavigationMenu( '', [] ) } @@ -740,8 +700,8 @@ function Navigation( { isResolvingCanUserCreateNavigationMenu } onSelectNavigationMenu={ ( menuId ) => { - handleUpdateMenu( menuId ); - setShouldFocusNavigationSelector( true ); + setRef( menuId ); + selectBlock( clientId ); } } onSelectClassicMenu={ async ( classicMenu ) => { const navMenu = await convertClassicMenu( @@ -749,8 +709,8 @@ function Navigation( { classicMenu.name ); if ( navMenu ) { - handleUpdateMenu( navMenu.id ); - setShouldFocusNavigationSelector( true ); + setRef( navMenu.id ); + selectBlock( clientId ); } } } onCreateEmpty={ () => createNavigationMenu( '', [] ) } @@ -763,37 +723,26 @@ function Navigation( { - { ! isDraftNavigationMenu && isEntityAvailable && ( - - { - handleUpdateMenu( menuId ); - setShouldFocusNavigationSelector( true ); - } } - onSelectClassicMenu={ async ( classicMenu ) => { - const navMenu = await convertClassicMenu( - classicMenu.id, - classicMenu.name - ); - if ( navMenu ) { - handleUpdateMenu( navMenu.id ); - setShouldFocusNavigationSelector( - true - ); - } - } } - onCreateNew={ () => - createNavigationMenu( '', [] ) + + { + const navMenu = await convertClassicMenu( + classicMenu.id, + classicMenu.name + ); + if ( navMenu ) { + setRef( navMenu.id ); } - /* translators: %s: The name of a menu. */ - actionLabel={ __( "Switch to '%s'" ) } - showManageActions - /> - - ) } + } } + onCreateNew={ () => createNavigationMenu( '', [] ) } + /* translators: %s: The name of a menu. */ + actionLabel={ __( "Switch to '%s'" ) } + showManageActions + /> + { stylingInspectorControls } { isEntityAvailable && ( diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index b83280ec129ec3..cda822edb8b4ca 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -10,7 +10,7 @@ import { import { __, sprintf } from '@wordpress/i18n'; import { decodeEntities } from '@wordpress/html-entities'; import { addQueryArgs } from '@wordpress/url'; -import { forwardRef, useMemo } from '@wordpress/element'; +import { useMemo } from '@wordpress/element'; /** * Internal dependencies @@ -18,18 +18,15 @@ import { forwardRef, useMemo } from '@wordpress/element'; import useNavigationMenu from '../use-navigation-menu'; import useNavigationEntities from '../use-navigation-entities'; -function NavigationMenuSelector( - { - currentMenuId, - onSelectNavigationMenu, - onSelectClassicMenu, - onCreateNew, - showManageActions = false, - actionLabel, - toggleProps = {}, - }, - forwardedRef -) { +export default function NavigationMenuSelector( { + currentMenuId, + onSelectNavigationMenu, + onSelectClassicMenu, + onCreateNew, + showManageActions = false, + actionLabel, + toggleProps = {}, +} ) { /* translators: %s: The name of a menu. */ const createActionLabel = __( "Create from '%s'" ); @@ -78,7 +75,6 @@ function NavigationMenuSelector( return ( { - onClose(); - onSelectNavigationMenu( menuId ); - } } + onSelect={ onSelectNavigationMenu } choices={ menuChoices } /> @@ -142,5 +135,3 @@ function NavigationMenuSelector( ); } - -export default forwardRef( NavigationMenuSelector ); diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index f95b8064959579..c2fbebbad7e876 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -1681,13 +1681,10 @@ Expected mock function not to be called but it was called with: ["POST", "http:/ ); } ); - // Skip reason: running it in interactive works but selecting and - // checking for focus consistently fails in the test. - // eslint-disable-next-line jest/no-disabled-tests - it.skip( 'should always focus select menu button after item selection', async () => { + it( 'keeps focus the menu item after navigation menu selection', async () => { // Create some navigation menus to work with. await createNavigationMenu( { - title: 'First navigation', + title: 'First Example Navigation', content: '', } ); @@ -1703,23 +1700,26 @@ Expected mock function not to be called but it was called with: ["POST", "http:/ // Insert new block and wait for the insert to complete. await insertBlock( 'Navigation' ); await waitForBlock( 'Navigation' ); - - const navigationSelector = await page.waitForXPath( - "//button[text()='Select Menu']" + // First select a menu from the block placeholder + const selectMenuDropdown = await page.waitForSelector( + '[aria-label="Select Menu"]' ); - await navigationSelector.click(); - - const theOption = await page.waitForXPath( - "//button[@aria-checked='false'][contains(., 'First navigation')]" + await selectMenuDropdown.click(); + const firstNavigationOption = await page.waitForXPath( + '//button[.="First Example Navigation"]' ); - theOption.click(); + await firstNavigationOption.click(); - // Once the options are closed, does select menu button receive focus? - const selectMenuDropdown2 = await page.$( - '[aria-label="Select Menu"]' + // Next switch menu using the toolbar. + await clickBlockToolbarButton( 'Select Menu' ); + + const secondNavigationOption = await page.waitForXPath( + '//button[.="Second Example Navigation"]' ); + await secondNavigationOption.click(); - await expect( selectMenuDropdown2 ).toHaveFocus(); + // The menu item should still have focus. + await expect( secondNavigationOption ).toHaveFocus(); } ); } ); } );