Skip to content

Commit

Permalink
Simplfy handling of save of Nav block uncontrolled inner blocks (#45517)
Browse files Browse the repository at this point in the history
* Simplfy handling of save of inner blocks

* Remove unused dep from effect

Co-authored-by: Daniel Richards <[email protected]>

* Memoize effect dep to avoid retriggering

* Correctly memoize and sort all fallback menus

* Simplify fallbacks compute function

Co-authored-by: Daniel Richards <[email protected]>
  • Loading branch information
getdave and talldan authored Nov 9, 2022
1 parent 10d8b49 commit 41a0bd5
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 57 deletions.
53 changes: 25 additions & 28 deletions packages/block-library/src/navigation/edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { useState, useEffect, useRef, Platform } from '@wordpress/element';
import {
useState,
useEffect,
useRef,
Platform,
useMemo,
} from '@wordpress/element';
import { addQueryArgs } from '@wordpress/url';
import {
__experimentalOffCanvasEditor as OffCanvasEditor,
Expand Down Expand Up @@ -197,9 +203,6 @@ function Navigation( {
__unstableMarkNextChangeAsNotPersistent,
} = useDispatch( blockEditorStore );

const [ hasSavedUnsavedInnerBlocks, setHasSavedUnsavedInnerBlocks ] =
useState( false );

const [ isResponsiveMenuOpen, setResponsiveMenuVisibility ] =
useState( false );

Expand Down Expand Up @@ -233,8 +236,16 @@ function Navigation( {
classicMenuConversionStatus === CLASSIC_MENU_CONVERSION_PENDING;

// Only autofallback to published menus.
const fallbackNavigationMenus = navigationMenus?.filter(
( menu ) => menu.status === 'publish'
const fallbackNavigationMenus = useMemo(
() =>
navigationMenus
?.filter( ( menu ) => menu.status === 'publish' )
?.sort( ( menuA, menuB ) => {
const menuADate = new Date( menuA.date );
const menuBDate = new Date( menuB.date );
return menuADate.getTime() < menuBDate.getTime();
} ),
[ navigationMenus ]
);

// Attempt to retrieve and prioritize any existing navigation menu unless:
Expand All @@ -254,12 +265,6 @@ function Navigation( {
return;
}

fallbackNavigationMenus.sort( ( menuA, menuB ) => {
const menuADate = new Date( menuA.date );
const menuBDate = new Date( menuB.date );
return menuADate.getTime() < menuBDate.getTime();
} );

/**
* This fallback displays (both in editor and on front)
* a list of pages only if no menu (user assigned or
Expand All @@ -269,7 +274,12 @@ function Navigation( {
*/
__unstableMarkNextChangeAsNotPersistent();
setRef( fallbackNavigationMenus[ 0 ].id );
}, [ navigationMenus ] );
}, [
ref,
isCreatingNavigationMenu,
fallbackNavigationMenus,
hasUncontrolledInnerBlocks,
] );

useEffect( () => {
if (
Expand Down Expand Up @@ -680,7 +690,7 @@ function Navigation( {
/>
);

if ( hasUnsavedBlocks ) {
if ( hasUnsavedBlocks && ! isCreatingNavigationMenu ) {
return (
<TagName { ...blockProps }>
<InspectorControls>
Expand Down Expand Up @@ -744,24 +754,11 @@ function Navigation( {
overlayTextColor={ overlayTextColor }
>
<UnsavedInnerBlocks
createNavigationMenu={ createNavigationMenu }
blocks={ uncontrolledInnerBlocks }
clientId={ clientId }
templateLock={ templateLock }
navigationMenus={ navigationMenus }
hasSelection={ isSelected || isInnerBlockSelected }
hasSavedUnsavedInnerBlocks={
hasSavedUnsavedInnerBlocks
}
onSave={ ( post ) => {
// Set some state used as a guard to prevent the creation of multiple posts.
setHasSavedUnsavedInnerBlocks( true );
// Switch to using the wp_navigation entity.
setRef( post.id );

showNavigationMenuStatusNotice(
__( `New Navigation Menu created.` )
);
} }
/>
</ResponsiveWrapper>
</TagName>
Expand Down
34 changes: 5 additions & 29 deletions packages/block-library/src/navigation/edit/unsaved-inner-blocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { useInnerBlocksProps } from '@wordpress/block-editor';
import { Disabled, Spinner } from '@wordpress/components';
import { Disabled } from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import { useContext, useEffect, useRef, useMemo } from '@wordpress/element';
Expand All @@ -11,7 +11,6 @@ import { useContext, useEffect, useRef, useMemo } from '@wordpress/element';
* Internal dependencies
*/
import useNavigationMenu from '../use-navigation-menu';
import useCreateNavigationMenu from './use-create-navigation-menu';

const EMPTY_OBJECT = {};
const DRAFT_MENU_PARAMS = [
Expand All @@ -38,9 +37,8 @@ const ALLOWED_BLOCKS = [

export default function UnsavedInnerBlocks( {
blocks,
clientId,
hasSavedUnsavedInnerBlocks,
onSave,
createNavigationMenu,

hasSelection,
} ) {
const originalBlocks = useRef();
Expand Down Expand Up @@ -75,7 +73,6 @@ export default function UnsavedInnerBlocks( {
// The block will be disabled in a block preview, use this as a way of
// avoiding the side-effects of this component for block previews.
const isDisabled = useContext( Disabled.Context );
const savingLock = useRef( false );

const innerBlocksProps = useInnerBlocksProps(
{
Expand Down Expand Up @@ -121,9 +118,6 @@ export default function UnsavedInnerBlocks( {

const { hasResolvedNavigationMenus, navigationMenus } = useNavigationMenu();

const { create: createNavigationMenu } =
useCreateNavigationMenu( clientId );

// Automatically save the uncontrolled blocks.
useEffect( () => {
// The block will be disabled when used in a BlockPreview.
Expand All @@ -140,9 +134,7 @@ export default function UnsavedInnerBlocks( {
// which is an indication they want to start editing.
if (
isDisabled ||
hasSavedUnsavedInnerBlocks ||
isSaving ||
savingLock.current ||
! hasResolvedDraftNavigationMenus ||
! hasResolvedNavigationMenus ||
! hasSelection ||
Expand All @@ -151,11 +143,7 @@ export default function UnsavedInnerBlocks( {
return;
}

savingLock.current = true;
createNavigationMenu( null, blocks ).then( ( menu ) => {
onSave( menu );
savingLock.current = false;
} );
createNavigationMenu( null, blocks );
}, [
isDisabled,
isSaving,
Expand All @@ -170,17 +158,5 @@ export default function UnsavedInnerBlocks( {

const Wrapper = isSaving ? Disabled : 'div';

return (
<>
{ isSaving ? (
<Spinner
className={
'wp-block-navigation__uncontrolled-inner-blocks-loading-indicator'
}
/>
) : (
<Wrapper { ...innerBlocksProps } />
) }
</>
);
return <Wrapper { ...innerBlocksProps } />;
}

0 comments on commit 41a0bd5

Please sign in to comment.