From b0a7d1d199088354b8f9c995d3e35a991ec6c51f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 09:51:52 +0000 Subject: [PATCH 01/18] Refactor hook to single async function --- .../src/navigation/edit/index.js | 38 +++++- .../edit/navigation-menu-selector.js | 30 +---- .../src/navigation/edit/placeholder/index.js | 3 + .../use-convert-classic-menu-to-block-menu.js | 124 ++++++++++++++++++ 4 files changed, 163 insertions(+), 32 deletions(-) create mode 100644 packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 25cc3f5e58ac98..dd6484733c3297 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -55,6 +55,7 @@ import UnsavedInnerBlocks from './unsaved-inner-blocks'; import NavigationMenuDeleteControl from './navigation-menu-delete-control'; import useNavigationNotice from './use-navigation-notice'; import OverlayMenuIcon from './overlay-menu-icon'; +import useConvertClassicToBlockMenu from './use-convert-classic-menu-to-block-menu'; const EMPTY_ARRAY = []; @@ -231,6 +232,13 @@ function Navigation( { clientId ); + const { + convert, + state: classicMenuConversionState, + } = useConvertClassicToBlockMenu( clientId ); + + const isConvertingClassicMenu = classicMenuConversionState?.isFetching; + // The standard HTML5 tag for the block wrapper. const TagName = 'nav'; @@ -247,7 +255,12 @@ function Navigation( { // "loading" state: // - there is a ref attribute pointing to a Navigation Post // - the Navigation Post isn't available (hasn't resolved) yet. - const isLoading = !! ( ref && ! isEntityAvailable ); + // - we are not running the Classic Menu conversion process. + const isLoading = !! ( + ref && + ! isEntityAvailable && + ! isConvertingClassicMenu + ); const blockProps = useBlockProps( { ref: navRef, @@ -310,6 +323,15 @@ function Navigation( { ] = useState(); const [ detectedOverlayColor, setDetectedOverlayColor ] = useState(); + useEffect( () => { + if ( + ! classicMenuConversionState?.isFetching && + classicMenuConversionState.navMenu + ) { + setRef( classicMenuConversionState.navMenu?.id ); + } + }, [ classicMenuConversionState ] ); + // Spacer block needs orientation from context. This is a patch until // https://github.com/WordPress/gutenberg/issues/36197 is addressed. useEffect( () => { @@ -490,11 +512,16 @@ function Navigation( { isResolvingCanUserCreateNavigationMenu={ isResolvingCanUserCreateNavigationMenu } - onFinish={ ( post ) => { - if ( post ) { + onFinish={ ( post, requiresConversion = false ) => { + if ( requiresConversion ) { + // Setting ref and selecting block + // handled by effect post-conversion. + convert( post.id, post.name ); + } + if ( ! requiresConversion && post ) { setRef( post.id ); + selectBlock( clientId ); } - selectBlock( clientId ); } } /> @@ -513,6 +540,9 @@ function Navigation( { onSelect={ ( { id } ) => { setRef( id ); } } + onSelectClassic={ ( classicMenu ) => { + convert( classicMenu.id, classicMenu.name ); + } } onCreateNew={ startWithEmptyMenu } /* translators: %s: The name of a menu. */ actionLabel={ __( "Switch to '%s'" ) } 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 0993a8159b1f31..5fe021b31d0d66 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -17,13 +17,11 @@ import { useCallback, useMemo } from '@wordpress/element'; */ import useNavigationMenu from '../use-navigation-menu'; import useNavigationEntities from '../use-navigation-entities'; -import useConvertClassicMenu from '../use-convert-classic-menu'; -import useCreateNavigationMenu from './use-create-navigation-menu'; export default function NavigationMenuSelector( { currentMenuId, - clientId, onSelect, + onSelectClassic, onCreateNew, showManageActions = false, actionLabel, @@ -43,27 +41,6 @@ export default function NavigationMenuSelector( { canSwitchNavigationMenu, } = useNavigationMenu(); - const createNavigationMenu = useCreateNavigationMenu( clientId ); - - const onFinishMenuCreation = async ( - blocks, - navigationMenuTitle = null - ) => { - if ( ! canUserCreateNavigationMenu ) { - return; - } - - const navigationMenu = await createNavigationMenu( - navigationMenuTitle, - blocks - ); - onSelect( navigationMenu ); - }; - - const convertClassicMenuToBlocks = useConvertClassicMenu( - onFinishMenuCreation - ); - const handleSelect = useCallback( ( _onClose ) => ( selectedId ) => { _onClose(); @@ -132,10 +109,7 @@ export default function NavigationMenuSelector( { { onClose(); - convertClassicMenuToBlocks( - menu.id, - menu.name - ); + onSelectClassic( menu ); } } key={ menu.id } aria-label={ sprintf( diff --git a/packages/block-library/src/navigation/edit/placeholder/index.js b/packages/block-library/src/navigation/edit/placeholder/index.js index 9906e37244eed4..ab8b0c45025070 100644 --- a/packages/block-library/src/navigation/edit/placeholder/index.js +++ b/packages/block-library/src/navigation/edit/placeholder/index.js @@ -85,6 +85,9 @@ export default function NavigationPlaceholder( { currentMenuId={ currentMenuId } clientId={ clientId } onSelect={ onFinish } + onSelectClassic={ ( menu ) => { + onFinish( menu, true ); + } } toggleProps={ { variant: 'tertiary', iconPosition: 'right', diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js new file mode 100644 index 00000000000000..ec0a38429efe2e --- /dev/null +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -0,0 +1,124 @@ +/** + * WordPress dependencies + */ +import { useRegistry } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; +import { + useReducer, + useCallback, + useRef, + useLayoutEffect, +} from '@wordpress/element'; + +/** + * Internal dependencies + */ +import useCreateNavigationMenu from './use-create-navigation-menu'; +import menuItemsToBlocks from '../menu-items-to-blocks'; + +function reducer( state, action ) { + switch ( action.type ) { + case 'RESOLVED': + return { + ...state, + isFetching: false, + navMenu: action.navMenu, + }; + case 'ERROR': + return { + ...state, + isFetching: false, + navMenu: null, + }; + case 'LOADING': + return { + ...state, + isFetching: true, + }; + default: + throw new Error( `Unexpected action type ${ action.type }` ); + } +} + +function useSafeDispatch( dispatch ) { + const mounted = useRef( false ); + useLayoutEffect( () => { + mounted.current = true; + return () => ( mounted.current = false ); + }, [] ); + return useCallback( + ( ...args ) => ( mounted.current ? dispatch( ...args ) : void 0 ), + [ dispatch ] + ); +} + +function useConvertClassicToBlockMenu( clientId ) { + const createNavigationMenu = useCreateNavigationMenu( clientId ); + const registry = useRegistry(); + + const [ state, dispatch ] = useReducer( reducer, { + navMenu: null, + isFetching: false, + } ); + + const safeDispatch = useSafeDispatch( dispatch ); + + async function convertClassicMenuToBlockMenu( menuId, menuName ) { + const menuItemsParameters = { + menus: menuId, + per_page: -1, + context: 'view', + }; + + // 1. Fetch the classic Menu items. + const classicMenuItems = await registry + .resolveSelect( coreStore ) + .getMenuItems( menuItemsParameters ); + + // 2. Convert the classic items into blocks. + const { innerBlocks } = menuItemsToBlocks( classicMenuItems ); + + // 3. Create the `wp_navigation` Post with the blocks. + const navigationMenu = await createNavigationMenu( + menuName, + innerBlocks + ); + + return navigationMenu; + } + + const convert = useCallback( + ( menuId, menuName ) => { + if ( ! menuId || ! menuName ) { + safeDispatch( { + type: 'ERROR', + } ); + } + + safeDispatch( { + type: 'LOADING', + } ); + + convertClassicMenuToBlockMenu( menuId, menuName ) + .then( ( navMenu ) => { + safeDispatch( { + type: 'RESOLVED', + navMenu, + } ); + } ) + .catch( () => { + safeDispatch( { + type: 'ERROR', + } ); + } ); + }, + [ clientId ] + ); + + return { + convert, + state, + }; +} + +export default useConvertClassicToBlockMenu; From 124e2445d9abdb76971e6948ae9bb17b0923ae6f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 10:17:00 +0000 Subject: [PATCH 02/18] Adjust UI state vars to account for conversion process state --- .../block-library/src/navigation/edit/index.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index dd6484733c3297..d4c5af052c2b0b 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -244,23 +244,25 @@ function Navigation( { // "placeholder" shown if: // - we don't have a ref attribute pointing to a Navigation Post. + // - we are not running a menu conversion process. // - we don't have uncontrolled blocks. // - (legacy) we have a Navigation Area without a ref attribute pointing to a Navigation Post. const isPlaceholder = - ! ref && ( ! hasUncontrolledInnerBlocks || isWithinUnassignedArea ); + ! ref && + ! isConvertingClassicMenu && + ( ! hasUncontrolledInnerBlocks || isWithinUnassignedArea ); const isEntityAvailable = ! isNavigationMenuMissing && isNavigationMenuResolved; // "loading" state: + // - we are running the Classic Menu conversion process. + // OR // - there is a ref attribute pointing to a Navigation Post // - the Navigation Post isn't available (hasn't resolved) yet. - // - we are not running the Classic Menu conversion process. - const isLoading = !! ( - ref && - ! isEntityAvailable && - ! isConvertingClassicMenu - ); + const isLoading = + isConvertingClassicMenu || + !! ( ref && ! isEntityAvailable && ! isConvertingClassicMenu ); const blockProps = useBlockProps( { ref: navRef, From ffa8a7e5eefb2e86632cc3a080ebd27b59233023 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 10:27:07 +0000 Subject: [PATCH 03/18] Unify menu ref update process --- .../src/navigation/edit/index.js | 21 ++++++++++++------- .../src/navigation/edit/placeholder/index.js | 3 ++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index d4c5af052c2b0b..218735ba8a5609 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -325,12 +325,17 @@ function Navigation( { ] = useState(); const [ detectedOverlayColor, setDetectedOverlayColor ] = useState(); + function handleUpdateMenu( menuId ) { + setRef( menuId ); + selectBlock( clientId ); + } + useEffect( () => { if ( ! classicMenuConversionState?.isFetching && classicMenuConversionState.navMenu ) { - setRef( classicMenuConversionState.navMenu?.id ); + handleUpdateMenu( classicMenuConversionState.navMenu?.id ); } }, [ classicMenuConversionState ] ); @@ -514,15 +519,17 @@ function Navigation( { isResolvingCanUserCreateNavigationMenu={ isResolvingCanUserCreateNavigationMenu } - onFinish={ ( post, requiresConversion = false ) => { - if ( requiresConversion ) { + onFinish={ ( post, isClassicMenu = false ) => { + if ( ! post ) { + return; + } + + if ( isClassicMenu ) { // Setting ref and selecting block // handled by effect post-conversion. convert( post.id, post.name ); - } - if ( ! requiresConversion && post ) { - setRef( post.id ); - selectBlock( clientId ); + } else { + handleUpdateMenu( post.id ); } } } /> diff --git a/packages/block-library/src/navigation/edit/placeholder/index.js b/packages/block-library/src/navigation/edit/placeholder/index.js index ab8b0c45025070..eb9cdba30b55f2 100644 --- a/packages/block-library/src/navigation/edit/placeholder/index.js +++ b/packages/block-library/src/navigation/edit/placeholder/index.js @@ -86,7 +86,8 @@ export default function NavigationPlaceholder( { clientId={ clientId } onSelect={ onFinish } onSelectClassic={ ( menu ) => { - onFinish( menu, true ); + const isClassicMenu = true; + onFinish( menu, isClassicMenu ); } } toggleProps={ { variant: 'tertiary', From 34974aca65b13a3bbcd54b1681b61a21025edfdb Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 10:29:48 +0000 Subject: [PATCH 04/18] Remove superceeded hook --- .../navigation/use-convert-classic-menu.js | 58 ------------------- 1 file changed, 58 deletions(-) delete mode 100644 packages/block-library/src/navigation/use-convert-classic-menu.js diff --git a/packages/block-library/src/navigation/use-convert-classic-menu.js b/packages/block-library/src/navigation/use-convert-classic-menu.js deleted file mode 100644 index c857e9c97cb0db..00000000000000 --- a/packages/block-library/src/navigation/use-convert-classic-menu.js +++ /dev/null @@ -1,58 +0,0 @@ -/** - * WordPress dependencies - */ -import { useCallback, useState, useEffect } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import useNavigationEntities from './use-navigation-entities'; -import menuItemsToBlocks from './menu-items-to-blocks'; - -export default function useConvertClassicMenu( onFinish ) { - const [ selectedMenu, setSelectedMenu ] = useState(); - const [ - isAwaitingMenuItemResolution, - setIsAwaitingMenuItemResolution, - ] = useState( false ); - const [ menuName, setMenuName ] = useState( '' ); - - const { menuItems, hasResolvedMenuItems } = useNavigationEntities( - selectedMenu - ); - - const createFromMenu = useCallback( - ( name ) => { - const { innerBlocks: blocks } = menuItemsToBlocks( menuItems ); - onFinish( blocks, name ); - }, - [ menuItems, menuItemsToBlocks, onFinish ] - ); - - useEffect( () => { - // If the user selected a menu but we had to wait for menu items to - // finish resolving, then create the block once resolution finishes. - if ( isAwaitingMenuItemResolution && hasResolvedMenuItems ) { - createFromMenu( menuName ); - setIsAwaitingMenuItemResolution( false ); - } - }, [ isAwaitingMenuItemResolution, hasResolvedMenuItems, menuName ] ); - - return useCallback( - ( id, name ) => { - setSelectedMenu( id ); - - // If we have menu items, create the block right away. - if ( hasResolvedMenuItems ) { - createFromMenu( name ); - return; - } - - // Otherwise, create the block when resolution finishes. - setIsAwaitingMenuItemResolution( true ); - // Store the name to use later. - setMenuName( name ); - }, - [ hasResolvedMenuItems, createFromMenu ] - ); -} From c8a8e1cbd1a1a0ceae71f46688fbfdbc9666691c Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 11:24:30 +0000 Subject: [PATCH 05/18] Refactor conversion state to enum --- packages/block-library/src/navigation/edit/index.js | 3 ++- .../edit/use-convert-classic-menu-to-block-menu.js | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 218735ba8a5609..17eb76dac35fb7 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -237,7 +237,8 @@ function Navigation( { state: classicMenuConversionState, } = useConvertClassicToBlockMenu( clientId ); - const isConvertingClassicMenu = classicMenuConversionState?.isFetching; + const isConvertingClassicMenu = + classicMenuConversionState?.status === 'fetching'; // The standard HTML5 tag for the block wrapper. const TagName = 'nav'; diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index ec0a38429efe2e..5031b149e8a496 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -21,19 +21,21 @@ function reducer( state, action ) { case 'RESOLVED': return { ...state, - isFetching: false, + status: 'success', navMenu: action.navMenu, }; case 'ERROR': return { ...state, - isFetching: false, + status: 'error', + error: action.error, navMenu: null, }; case 'LOADING': return { ...state, - isFetching: true, + status: 'fetching', + error: null, }; default: throw new Error( `Unexpected action type ${ action.type }` ); @@ -58,7 +60,8 @@ function useConvertClassicToBlockMenu( clientId ) { const [ state, dispatch ] = useReducer( reducer, { navMenu: null, - isFetching: false, + status: 'idle', + error: null, } ); const safeDispatch = useSafeDispatch( dispatch ); From 8161482568ffaa0017a17d81711983c9e5debd6d Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 12:00:00 +0000 Subject: [PATCH 06/18] Add initial error handling for conversion process --- .../block-library/src/navigation/edit/index.js | 16 +++++++++++++++- .../use-convert-classic-menu-to-block-menu.js | 15 ++++++++++++++- .../src/navigation/edit/use-navigation-notice.js | 6 +++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 17eb76dac35fb7..e0395cbe119872 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -333,10 +333,17 @@ function Navigation( { useEffect( () => { if ( - ! classicMenuConversionState?.isFetching && + classicMenuConversionState?.status === 'success' && classicMenuConversionState.navMenu ) { handleUpdateMenu( classicMenuConversionState.navMenu?.id ); + hideClassicMenuConversionErrorNotice(); + } + + if ( classicMenuConversionState?.status === 'error' ) { + showClassicMenuConversionErrorNotice( + classicMenuConversionState.error + ); } }, [ classicMenuConversionState ] ); @@ -386,6 +393,13 @@ function Navigation( { } ); + const [ + showClassicMenuConversionErrorNotice, + hideClassicMenuConversionErrorNotice, + ] = useNavigationNotice( { + name: 'block-library/core/navigation/classic-menu-conversion/error', + } ); + useEffect( () => { if ( ! isSelected && ! isInnerBlockSelected ) { hideCantEditNotice(); diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 5031b149e8a496..12242189d4f818 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -78,6 +78,18 @@ function useConvertClassicToBlockMenu( clientId ) { .resolveSelect( coreStore ) .getMenuItems( menuItemsParameters ); + // Offline response results in `null` + if ( classicMenuItems === null ) { + throw new Error( + // TODO - i18n + `Unable to fetch classic menu "${ menuName }" from API.`, + { + menuId, + menuName, + } + ); + } + // 2. Convert the classic items into blocks. const { innerBlocks } = menuItemsToBlocks( classicMenuItems ); @@ -109,9 +121,10 @@ function useConvertClassicToBlockMenu( clientId ) { navMenu, } ); } ) - .catch( () => { + .catch( ( e ) => { safeDispatch( { type: 'ERROR', + error: e?.message, } ); } ); }, diff --git a/packages/block-library/src/navigation/edit/use-navigation-notice.js b/packages/block-library/src/navigation/edit/use-navigation-notice.js index 2466c79c2e7d51..ad0c6d33587c92 100644 --- a/packages/block-library/src/navigation/edit/use-navigation-notice.js +++ b/packages/block-library/src/navigation/edit/use-navigation-notice.js @@ -5,19 +5,19 @@ import { useRef } from '@wordpress/element'; import { useDispatch } from '@wordpress/data'; import { store as noticeStore } from '@wordpress/notices'; -function useNavigationNotice( { name, message } = {} ) { +function useNavigationNotice( { name, message = '' } = {} ) { const noticeRef = useRef(); const { createWarningNotice, removeNotice } = useDispatch( noticeStore ); - const showNotice = () => { + const showNotice = ( customMsg ) => { if ( noticeRef.current ) { return; } noticeRef.current = name; - createWarningNotice( message, { + createWarningNotice( customMsg || message, { id: noticeRef.current, type: 'snackbar', } ); From 651ed4c8971428a0baa1b1dee937d7773e8097c8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 13:55:32 +0000 Subject: [PATCH 07/18] i18n of classic menu fetch error --- .../edit/use-convert-classic-menu-to-block-menu.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 12242189d4f818..6a73884c3dba5e 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -9,6 +9,7 @@ import { useRef, useLayoutEffect, } from '@wordpress/element'; +import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies @@ -82,7 +83,11 @@ function useConvertClassicToBlockMenu( clientId ) { if ( classicMenuItems === null ) { throw new Error( // TODO - i18n - `Unable to fetch classic menu "${ menuName }" from API.`, + sprintf( + // translators: %s: the name of a menu (e.g. Header navigation). + __( `Unable to fetch classic menu "%s" from API.` ), + menuName + ), { menuId, menuName, @@ -107,6 +112,7 @@ function useConvertClassicToBlockMenu( clientId ) { if ( ! menuId || ! menuName ) { safeDispatch( { type: 'ERROR', + error: '', } ); } From 27d0a941f9e9cade4738f8f30210cb8d6d97ae34 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 14:15:43 +0000 Subject: [PATCH 08/18] Handle more creation errors --- .../use-convert-classic-menu-to-block-menu.js | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 6a73884c3dba5e..7d350861695c9e 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -68,24 +68,55 @@ function useConvertClassicToBlockMenu( clientId ) { const safeDispatch = useSafeDispatch( dispatch ); async function convertClassicMenuToBlockMenu( menuId, menuName ) { + let navigationMenu; + let classicMenuItems; + const menuItemsParameters = { menus: menuId, per_page: -1, context: 'view', }; + const fetchError = new Error( + sprintf( + // translators: %s: the name of a menu (e.g. Header navigation). + __( `Unable to fetch classic menu "%s" from API.` ), + menuName + ), + { + menuId, + menuName, + } + ); + // 1. Fetch the classic Menu items. - const classicMenuItems = await registry - .resolveSelect( coreStore ) - .getMenuItems( menuItemsParameters ); + try { + classicMenuItems = await registry + .resolveSelect( coreStore ) + .getMenuItems( menuItemsParameters ); + } catch ( e ) { + throw fetchError; + } - // Offline response results in `null` + // Handle offline response which resolves to `null`. if ( classicMenuItems === null ) { + throw fetchError; + } + + // 2. Convert the classic items into blocks. + const { innerBlocks } = menuItemsToBlocks( classicMenuItems ); + + // 3. Create the `wp_navigation` Post with the blocks. + try { + navigationMenu = await createNavigationMenu( + menuName, + innerBlocks + ); + } catch ( e ) { throw new Error( - // TODO - i18n sprintf( // translators: %s: the name of a menu (e.g. Header navigation). - __( `Unable to fetch classic menu "%s" from API.` ), + __( `Unable to create Navigation Menu "%s".` ), menuName ), { @@ -95,15 +126,6 @@ function useConvertClassicToBlockMenu( clientId ) { ); } - // 2. Convert the classic items into blocks. - const { innerBlocks } = menuItemsToBlocks( classicMenuItems ); - - // 3. Create the `wp_navigation` Post with the blocks. - const navigationMenu = await createNavigationMenu( - menuName, - innerBlocks - ); - return navigationMenu; } From 6fad27972385189d30a33e5287bee5befa53ba83 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 14:34:18 +0000 Subject: [PATCH 09/18] Manually hoist var --- .../block-library/src/navigation/edit/index.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index e0395cbe119872..2ef51499a9e986 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -326,6 +326,13 @@ function Navigation( { ] = useState(); const [ detectedOverlayColor, setDetectedOverlayColor ] = useState(); + const [ + showClassicMenuConversionErrorNotice, + hideClassicMenuConversionErrorNotice, + ] = useNavigationNotice( { + name: 'block-library/core/navigation/classic-menu-conversion/error', + } ); + function handleUpdateMenu( menuId ) { setRef( menuId ); selectBlock( clientId ); @@ -393,13 +400,6 @@ function Navigation( { } ); - const [ - showClassicMenuConversionErrorNotice, - hideClassicMenuConversionErrorNotice, - ] = useNavigationNotice( { - name: 'block-library/core/navigation/classic-menu-conversion/error', - } ); - useEffect( () => { if ( ! isSelected && ! isInnerBlockSelected ) { hideCantEditNotice(); From 04bdbc2227466eb14b6aec64a90717dcdd82d950 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 3 Mar 2022 16:08:21 +0000 Subject: [PATCH 10/18] Add accessible menu import announcements --- packages/block-library/src/navigation/edit/index.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 2ef51499a9e986..5b12ec50a7df2a 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -39,6 +39,7 @@ import { Spinner, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; +import { speak } from '@wordpress/a11y'; /** * Internal dependencies @@ -339,18 +340,24 @@ function Navigation( { } useEffect( () => { + if ( classicMenuConversionState?.status === 'fetching' ) { + speak( __( 'Classic menu importing.' ) ); + } + if ( classicMenuConversionState?.status === 'success' && classicMenuConversionState.navMenu ) { handleUpdateMenu( classicMenuConversionState.navMenu?.id ); hideClassicMenuConversionErrorNotice(); + speak( __( 'Classic menu imported successfully.' ) ); } if ( classicMenuConversionState?.status === 'error' ) { showClassicMenuConversionErrorNotice( classicMenuConversionState.error ); + speak( __( 'Classic menu import failed.' ) ); } }, [ classicMenuConversionState ] ); From 35cbf100a67771a945abc7bdefad21cf640182d1 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 05:50:10 +0000 Subject: [PATCH 11/18] Refactor to remove useReducer in favour of simple useState Addresses https://github.com/WordPress/gutenberg/pull/38858#discussion_r808956065 --- .../src/navigation/edit/index.js | 27 +++--- .../use-convert-classic-menu-to-block-menu.js | 86 +++++-------------- 2 files changed, 35 insertions(+), 78 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 5b12ec50a7df2a..d54090fcb0b46c 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -235,11 +235,12 @@ function Navigation( { const { convert, - state: classicMenuConversionState, + status: classicMenuConversionStatus, + error: classicMenuConversionError, + value: classicMenuConversionResult, } = useConvertClassicToBlockMenu( clientId ); - const isConvertingClassicMenu = - classicMenuConversionState?.status === 'fetching'; + const isConvertingClassicMenu = classicMenuConversionStatus === 'pending'; // The standard HTML5 tag for the block wrapper. const TagName = 'nav'; @@ -340,26 +341,28 @@ function Navigation( { } useEffect( () => { - if ( classicMenuConversionState?.status === 'fetching' ) { + if ( classicMenuConversionStatus === 'pending' ) { speak( __( 'Classic menu importing.' ) ); } if ( - classicMenuConversionState?.status === 'success' && - classicMenuConversionState.navMenu + classicMenuConversionStatus === 'success' && + classicMenuConversionResult ) { - handleUpdateMenu( classicMenuConversionState.navMenu?.id ); + handleUpdateMenu( classicMenuConversionResult?.id ); hideClassicMenuConversionErrorNotice(); speak( __( 'Classic menu imported successfully.' ) ); } - if ( classicMenuConversionState?.status === 'error' ) { - showClassicMenuConversionErrorNotice( - classicMenuConversionState.error - ); + if ( classicMenuConversionStatus === 'error' ) { + showClassicMenuConversionErrorNotice( classicMenuConversionError ); speak( __( 'Classic menu import failed.' ) ); } - }, [ classicMenuConversionState ] ); + }, [ + classicMenuConversionStatus, + classicMenuConversionResult, + classicMenuConversionError, + ] ); // Spacer block needs orientation from context. This is a patch until // https://github.com/WordPress/gutenberg/issues/36197 is addressed. diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 7d350861695c9e..8a6c166d68feba 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -3,12 +3,7 @@ */ import { useRegistry } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; -import { - useReducer, - useCallback, - useRef, - useLayoutEffect, -} from '@wordpress/element'; +import { useState, useCallback } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; /** @@ -17,55 +12,17 @@ import { __, sprintf } from '@wordpress/i18n'; import useCreateNavigationMenu from './use-create-navigation-menu'; import menuItemsToBlocks from '../menu-items-to-blocks'; -function reducer( state, action ) { - switch ( action.type ) { - case 'RESOLVED': - return { - ...state, - status: 'success', - navMenu: action.navMenu, - }; - case 'ERROR': - return { - ...state, - status: 'error', - error: action.error, - navMenu: null, - }; - case 'LOADING': - return { - ...state, - status: 'fetching', - error: null, - }; - default: - throw new Error( `Unexpected action type ${ action.type }` ); - } -} - -function useSafeDispatch( dispatch ) { - const mounted = useRef( false ); - useLayoutEffect( () => { - mounted.current = true; - return () => ( mounted.current = false ); - }, [] ); - return useCallback( - ( ...args ) => ( mounted.current ? dispatch( ...args ) : void 0 ), - [ dispatch ] - ); -} +const CLASSIC_MENU_CONVERSION_SUCCESS = CLASSIC_MENU_CONVERSION_SUCCESS; +const CLASSIC_MENU_CONVERSION_ERROR = CLASSIC_MENU_CONVERSION_ERROR; +const CLASSIC_MENU_CONVERSION_PENDING = CLASSIC_MENU_CONVERSION_PENDING; function useConvertClassicToBlockMenu( clientId ) { const createNavigationMenu = useCreateNavigationMenu( clientId ); const registry = useRegistry(); - const [ state, dispatch ] = useReducer( reducer, { - navMenu: null, - status: 'idle', - error: null, - } ); - - const safeDispatch = useSafeDispatch( dispatch ); + const [ status, setStatus ] = useState( 'idle' ); + const [ value, setValue ] = useState( null ); + const [ error, setError ] = useState( null ); async function convertClassicMenuToBlockMenu( menuId, menuName ) { let navigationMenu; @@ -132,28 +89,23 @@ function useConvertClassicToBlockMenu( clientId ) { const convert = useCallback( ( menuId, menuName ) => { if ( ! menuId || ! menuName ) { - safeDispatch( { - type: 'ERROR', - error: '', - } ); + setError( 'Unable to convert menu. Missing menu details.' ); + setStatus( CLASSIC_MENU_CONVERSION_ERROR ); + return; } - safeDispatch( { - type: 'LOADING', - } ); + setStatus( CLASSIC_MENU_CONVERSION_PENDING ); + setValue( null ); + setError( null ); convertClassicMenuToBlockMenu( menuId, menuName ) .then( ( navMenu ) => { - safeDispatch( { - type: 'RESOLVED', - navMenu, - } ); + setValue( navMenu ); + setStatus( CLASSIC_MENU_CONVERSION_SUCCESS ); } ) .catch( ( e ) => { - safeDispatch( { - type: 'ERROR', - error: e?.message, - } ); + setError( e?.message ); + setStatus( CLASSIC_MENU_CONVERSION_ERROR ); } ); }, [ clientId ] @@ -161,7 +113,9 @@ function useConvertClassicToBlockMenu( clientId ) { return { convert, - state, + status, + value, + error, }; } From 864a469dc647d7c26c91d5934a570fe7041037f3 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 05:57:20 +0000 Subject: [PATCH 12/18] Fix bug where constants were accidentally assigned to constants --- .../edit/use-convert-classic-menu-to-block-menu.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 8a6c166d68feba..74ce4687d30198 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -12,9 +12,9 @@ import { __, sprintf } from '@wordpress/i18n'; import useCreateNavigationMenu from './use-create-navigation-menu'; import menuItemsToBlocks from '../menu-items-to-blocks'; -const CLASSIC_MENU_CONVERSION_SUCCESS = CLASSIC_MENU_CONVERSION_SUCCESS; -const CLASSIC_MENU_CONVERSION_ERROR = CLASSIC_MENU_CONVERSION_ERROR; -const CLASSIC_MENU_CONVERSION_PENDING = CLASSIC_MENU_CONVERSION_PENDING; +const CLASSIC_MENU_CONVERSION_SUCCESS = 'success'; +const CLASSIC_MENU_CONVERSION_ERROR = 'error'; +const CLASSIC_MENU_CONVERSION_PENDING = 'pending'; function useConvertClassicToBlockMenu( clientId ) { const createNavigationMenu = useCreateNavigationMenu( clientId ); From 987c38f801d0da3115a816df5b25a6f683c832fa Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 06:12:34 +0000 Subject: [PATCH 13/18] Simplify menu selector props MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detect the classic menu by relying on it’s `auto_add` prop which we know will not be defined on a Navigation Post. --- .../src/navigation/edit/index.js | 35 +++++++++++-------- .../edit/navigation-menu-selector.js | 17 ++++++--- .../src/navigation/edit/placeholder/index.js | 4 --- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index d54090fcb0b46c..61d89218bd1615 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -544,17 +544,17 @@ function Navigation( { isResolvingCanUserCreateNavigationMenu={ isResolvingCanUserCreateNavigationMenu } - onFinish={ ( post, isClassicMenu = false ) => { - if ( ! post ) { - return; - } - + onFinish={ ( navPostOrClassicMenu ) => { + const isClassicMenu = navPostOrClassicMenu.hasOwnProperty( + 'auto_add' + ); if ( isClassicMenu ) { - // Setting ref and selecting block - // handled by effect post-conversion. - convert( post.id, post.name ); + convert( + navPostOrClassicMenu.id, + navPostOrClassicMenu.name + ); } else { - handleUpdateMenu( post.id ); + handleUpdateMenu( navPostOrClassicMenu?.id ); } } } /> @@ -571,11 +571,18 @@ function Navigation( { { - setRef( id ); - } } - onSelectClassic={ ( classicMenu ) => { - convert( classicMenu.id, classicMenu.name ); + onSelect={ ( navPostOrClassicMenu ) => { + const isClassicMenu = navPostOrClassicMenu.hasOwnProperty( + 'auto_add' + ); + if ( isClassicMenu ) { + convert( + navPostOrClassicMenu.id, + navPostOrClassicMenu.name + ); + } else { + setRef( navPostOrClassicMenu?.id ); + } } } onCreateNew={ startWithEmptyMenu } /* translators: %s: The name of a menu. */ 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 5fe021b31d0d66..2a0f6fa60206df 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -21,7 +21,6 @@ import useNavigationEntities from '../use-navigation-entities'; export default function NavigationMenuSelector( { currentMenuId, onSelect, - onSelectClassic, onCreateNew, showManageActions = false, actionLabel, @@ -51,6 +50,14 @@ export default function NavigationMenuSelector( { [ navigationMenus ] ); + const handleSelectClassic = useCallback( + ( _onClose, menu ) => () => { + _onClose(); + onSelect( menu ); + }, + [] + ); + const menuChoices = useMemo( () => { return ( navigationMenus?.map( ( { id, title } ) => { @@ -107,10 +114,10 @@ export default function NavigationMenuSelector( { const label = decodeEntities( menu.name ); return ( { - onClose(); - onSelectClassic( menu ); - } } + onClick={ handleSelectClassic( + onClose, + menu + ) } key={ menu.id } aria-label={ sprintf( createActionLabel, diff --git a/packages/block-library/src/navigation/edit/placeholder/index.js b/packages/block-library/src/navigation/edit/placeholder/index.js index eb9cdba30b55f2..9906e37244eed4 100644 --- a/packages/block-library/src/navigation/edit/placeholder/index.js +++ b/packages/block-library/src/navigation/edit/placeholder/index.js @@ -85,10 +85,6 @@ export default function NavigationPlaceholder( { currentMenuId={ currentMenuId } clientId={ clientId } onSelect={ onFinish } - onSelectClassic={ ( menu ) => { - const isClassicMenu = true; - onFinish( menu, isClassicMenu ); - } } toggleProps={ { variant: 'tertiary', iconPosition: 'right', From 31ccf0261409f5dfd5a947c69ddffdf23fa650d0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 12:21:51 +0000 Subject: [PATCH 14/18] Improve error handling --- .../use-convert-classic-menu-to-block-menu.js | 69 +++++++++++-------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 74ce4687d30198..8269541517be7d 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -15,12 +15,13 @@ import menuItemsToBlocks from '../menu-items-to-blocks'; const CLASSIC_MENU_CONVERSION_SUCCESS = 'success'; const CLASSIC_MENU_CONVERSION_ERROR = 'error'; const CLASSIC_MENU_CONVERSION_PENDING = 'pending'; +const CLASSIC_MENU_CONVERSION_IDLE = 'idle'; function useConvertClassicToBlockMenu( clientId ) { const createNavigationMenu = useCreateNavigationMenu( clientId ); const registry = useRegistry(); - const [ status, setStatus ] = useState( 'idle' ); + const [ status, setStatus ] = useState( CLASSIC_MENU_CONVERSION_IDLE ); const [ value, setValue ] = useState( null ); const [ error, setError ] = useState( null ); @@ -28,36 +29,37 @@ function useConvertClassicToBlockMenu( clientId ) { let navigationMenu; let classicMenuItems; - const menuItemsParameters = { - menus: menuId, - per_page: -1, - context: 'view', - }; - - const fetchError = new Error( - sprintf( - // translators: %s: the name of a menu (e.g. Header navigation). - __( `Unable to fetch classic menu "%s" from API.` ), - menuName - ), - { - menuId, - menuName, - } - ); - // 1. Fetch the classic Menu items. try { classicMenuItems = await registry .resolveSelect( coreStore ) - .getMenuItems( menuItemsParameters ); - } catch ( e ) { - throw fetchError; + .getMenuItems( { + menus: menuId, + per_page: -1, + context: 'view', + } ); + } catch ( err ) { + throw new Error( + sprintf( + // translators: %s: the name of a menu (e.g. Header navigation). + __( `Unable to fetch classic menu "%s" from API.` ), + menuName + ), + { + cause: err, + } + ); } // Handle offline response which resolves to `null`. if ( classicMenuItems === null ) { - throw fetchError; + throw new Error( + sprintf( + // translators: %s: the name of a menu (e.g. Header navigation). + __( `Unable to fetch classic menu "%s" from API.` ), + menuName + ) + ); } // 2. Convert the classic items into blocks. @@ -69,7 +71,7 @@ function useConvertClassicToBlockMenu( clientId ) { menuName, innerBlocks ); - } catch ( e ) { + } catch ( err ) { throw new Error( sprintf( // translators: %s: the name of a menu (e.g. Header navigation). @@ -77,8 +79,7 @@ function useConvertClassicToBlockMenu( clientId ) { menuName ), { - menuId, - menuName, + cause: err, } ); } @@ -103,9 +104,21 @@ function useConvertClassicToBlockMenu( clientId ) { setValue( navMenu ); setStatus( CLASSIC_MENU_CONVERSION_SUCCESS ); } ) - .catch( ( e ) => { - setError( e?.message ); + .catch( ( err ) => { + setError( err?.message ); setStatus( CLASSIC_MENU_CONVERSION_ERROR ); + + // Rethrow error for debugging. + throw new Error( + sprintf( + // translators: %s: the name of a menu (e.g. Header navigation). + __( `Unable to create Navigation Menu "%s".` ), + menuName + ), + { + cause: err, + } + ); } ); }, [ clientId ] From 5ae5df777973ad882b635fc88693fe664e9b37af Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 12:27:30 +0000 Subject: [PATCH 15/18] Use constants over comparisons for statuses --- .../block-library/src/navigation/edit/index.js | 15 ++++++++++----- .../use-convert-classic-menu-to-block-menu.js | 8 ++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 61d89218bd1615..005f70cefed7e7 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -56,7 +56,11 @@ import UnsavedInnerBlocks from './unsaved-inner-blocks'; import NavigationMenuDeleteControl from './navigation-menu-delete-control'; import useNavigationNotice from './use-navigation-notice'; import OverlayMenuIcon from './overlay-menu-icon'; -import useConvertClassicToBlockMenu from './use-convert-classic-menu-to-block-menu'; +import useConvertClassicToBlockMenu, { + CLASSIC_MENU_CONVERSION_ERROR, + CLASSIC_MENU_CONVERSION_PENDING, + CLASSIC_MENU_CONVERSION_SUCCESS, +} from './use-convert-classic-menu-to-block-menu'; const EMPTY_ARRAY = []; @@ -240,7 +244,8 @@ function Navigation( { value: classicMenuConversionResult, } = useConvertClassicToBlockMenu( clientId ); - const isConvertingClassicMenu = classicMenuConversionStatus === 'pending'; + const isConvertingClassicMenu = + classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_PENDING; // The standard HTML5 tag for the block wrapper. const TagName = 'nav'; @@ -341,12 +346,12 @@ function Navigation( { } useEffect( () => { - if ( classicMenuConversionStatus === 'pending' ) { + if ( classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_PENDING ) { speak( __( 'Classic menu importing.' ) ); } if ( - classicMenuConversionStatus === 'success' && + classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_SUCCESS && classicMenuConversionResult ) { handleUpdateMenu( classicMenuConversionResult?.id ); @@ -354,7 +359,7 @@ function Navigation( { speak( __( 'Classic menu imported successfully.' ) ); } - if ( classicMenuConversionStatus === 'error' ) { + if ( classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_ERROR ) { showClassicMenuConversionErrorNotice( classicMenuConversionError ); speak( __( 'Classic menu import failed.' ) ); } diff --git a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js index 8269541517be7d..475d232b4735f6 100644 --- a/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js +++ b/packages/block-library/src/navigation/edit/use-convert-classic-menu-to-block-menu.js @@ -12,10 +12,10 @@ import { __, sprintf } from '@wordpress/i18n'; import useCreateNavigationMenu from './use-create-navigation-menu'; import menuItemsToBlocks from '../menu-items-to-blocks'; -const CLASSIC_MENU_CONVERSION_SUCCESS = 'success'; -const CLASSIC_MENU_CONVERSION_ERROR = 'error'; -const CLASSIC_MENU_CONVERSION_PENDING = 'pending'; -const CLASSIC_MENU_CONVERSION_IDLE = 'idle'; +export const CLASSIC_MENU_CONVERSION_SUCCESS = 'success'; +export const CLASSIC_MENU_CONVERSION_ERROR = 'error'; +export const CLASSIC_MENU_CONVERSION_PENDING = 'pending'; +export const CLASSIC_MENU_CONVERSION_IDLE = 'idle'; function useConvertClassicToBlockMenu( clientId ) { const createNavigationMenu = useCreateNavigationMenu( clientId ); From bf69385bff7a1b30ea9388e0f1558a11e2ad27db Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 14:23:59 +0000 Subject: [PATCH 16/18] Extract common function for handling selecting a Navigation --- .../src/navigation/edit/index.js | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 005f70cefed7e7..05205ab29fe621 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -447,6 +447,22 @@ function Navigation( { ref, ] ); + const handleSelectNavigation = ( navPostOrClassicMenu ) => { + if ( ! navPostOrClassicMenu ) { + return; + } + + const isClassicMenu = navPostOrClassicMenu?.hasOwnProperty( + 'auto_add' + ); + + if ( isClassicMenu ) { + convert( navPostOrClassicMenu.id, navPostOrClassicMenu.name ); + } else { + handleUpdateMenu( navPostOrClassicMenu?.id ); + } + }; + const startWithEmptyMenu = useCallback( () => { registry.batch( () => { if ( navigationArea ) { @@ -549,19 +565,7 @@ function Navigation( { isResolvingCanUserCreateNavigationMenu={ isResolvingCanUserCreateNavigationMenu } - onFinish={ ( navPostOrClassicMenu ) => { - const isClassicMenu = navPostOrClassicMenu.hasOwnProperty( - 'auto_add' - ); - if ( isClassicMenu ) { - convert( - navPostOrClassicMenu.id, - navPostOrClassicMenu.name - ); - } else { - handleUpdateMenu( navPostOrClassicMenu?.id ); - } - } } + onFinish={ handleSelectNavigation } /> ); @@ -576,19 +580,7 @@ function Navigation( { { - const isClassicMenu = navPostOrClassicMenu.hasOwnProperty( - 'auto_add' - ); - if ( isClassicMenu ) { - convert( - navPostOrClassicMenu.id, - navPostOrClassicMenu.name - ); - } else { - setRef( navPostOrClassicMenu?.id ); - } - } } + onSelect={ handleSelectNavigation } onCreateNew={ startWithEmptyMenu } /* translators: %s: The name of a menu. */ actionLabel={ __( "Switch to '%s'" ) } From ef9b3d7a98d50d5bb696be5946ac3f4794d37f99 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 4 Mar 2022 14:26:31 +0000 Subject: [PATCH 17/18] Optimise handler --- .../src/navigation/edit/index.js | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 05205ab29fe621..d73d5e959cffb2 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -447,21 +447,24 @@ function Navigation( { ref, ] ); - const handleSelectNavigation = ( navPostOrClassicMenu ) => { - if ( ! navPostOrClassicMenu ) { - return; - } + const handleSelectNavigation = useCallback( + ( navPostOrClassicMenu ) => { + if ( ! navPostOrClassicMenu ) { + return; + } - const isClassicMenu = navPostOrClassicMenu?.hasOwnProperty( - 'auto_add' - ); + const isClassicMenu = navPostOrClassicMenu?.hasOwnProperty( + 'auto_add' + ); - if ( isClassicMenu ) { - convert( navPostOrClassicMenu.id, navPostOrClassicMenu.name ); - } else { - handleUpdateMenu( navPostOrClassicMenu?.id ); - } - }; + if ( isClassicMenu ) { + convert( navPostOrClassicMenu.id, navPostOrClassicMenu.name ); + } else { + handleUpdateMenu( navPostOrClassicMenu?.id ); + } + }, + [ convert, handleUpdateMenu ] + ); const startWithEmptyMenu = useCallback( () => { registry.batch( () => { From ca5eeb5f0db608b6bfac23b17f09c184320133b6 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 7 Mar 2022 08:59:56 +0000 Subject: [PATCH 18/18] Remove unwanted conditional operator --- packages/block-library/src/navigation/edit/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index d73d5e959cffb2..7b15d775f281a3 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -453,14 +453,14 @@ function Navigation( { return; } - const isClassicMenu = navPostOrClassicMenu?.hasOwnProperty( + const isClassicMenu = navPostOrClassicMenu.hasOwnProperty( 'auto_add' ); if ( isClassicMenu ) { convert( navPostOrClassicMenu.id, navPostOrClassicMenu.name ); } else { - handleUpdateMenu( navPostOrClassicMenu?.id ); + handleUpdateMenu( navPostOrClassicMenu.id ); } }, [ convert, handleUpdateMenu ]