diff --git a/packages/block-editor/src/components/provider/use-block-sync.js b/packages/block-editor/src/components/provider/use-block-sync.js index dbcb8374987128..22443ebbc27f30 100644 --- a/packages/block-editor/src/components/provider/use-block-sync.js +++ b/packages/block-editor/src/components/provider/use-block-sync.js @@ -7,7 +7,7 @@ import { last, noop } from 'lodash'; * WordPress dependencies */ import { useEffect, useRef } from '@wordpress/element'; -import { useRegistry } from '@wordpress/data'; +import { useRegistry, useSelect } from '@wordpress/data'; import { cloneBlock } from '@wordpress/blocks'; /** @@ -83,6 +83,15 @@ export default function useBlockSync( { __unstableMarkNextChangeAsNotPersistent, } = registry.dispatch( blockEditorStore ); const { getBlockName, getBlocks } = registry.select( blockEditorStore ); + const isControlled = useSelect( + ( select ) => { + return ( + ! clientId || + select( blockEditorStore ).areInnerBlocksControlled( clientId ) + ); + }, + [ clientId ] + ); const pendingChanges = useRef( { incoming: null, outgoing: [] } ); const subscribed = useRef( false ); @@ -97,15 +106,21 @@ export default function useBlockSync( { // and so it would already be persisted. __unstableMarkNextChangeAsNotPersistent(); if ( clientId ) { - setHasControlledInnerBlocks( clientId, true ); - __unstableMarkNextChangeAsNotPersistent(); - const storeBlocks = controlledBlocks.map( ( block ) => - cloneBlock( block ) - ); - if ( subscribed.current ) { - pendingChanges.current.incoming = storeBlocks; - } - replaceInnerBlocks( clientId, storeBlocks ); + // It is important to batch here because otherwise, + // as soon as `setHasControlledInnerBlocks` is called + // the effect to restore might be triggered + // before the actual blocks get set properly in state. + registry.batch( () => { + setHasControlledInnerBlocks( clientId, true ); + const storeBlocks = controlledBlocks.map( ( block ) => + cloneBlock( block ) + ); + if ( subscribed.current ) { + pendingChanges.current.incoming = storeBlocks; + } + __unstableMarkNextChangeAsNotPersistent(); + replaceInnerBlocks( clientId, storeBlocks ); + } ); } else { if ( subscribed.current ) { pendingChanges.current.incoming = controlledBlocks; @@ -157,6 +172,15 @@ export default function useBlockSync( { } }, [ controlledBlocks, clientId ] ); + useEffect( () => { + // When the block becomes uncontrolled, it means its inner state has been reset + // we need to take the blocks again from the external value property. + if ( ! isControlled ) { + pendingChanges.current.outgoing = []; + setControlledBlocks(); + } + }, [ isControlled ] ); + useEffect( () => { const { getSelectionStart, @@ -164,6 +188,7 @@ export default function useBlockSync( { getSelectedBlocksInitialCaretPosition, isLastBlockChangePersistent, __unstableIsLastBlockChangeIgnored, + areInnerBlocksControlled, } = registry.select( blockEditorStore ); let blocks = getBlocks( clientId ); @@ -182,8 +207,17 @@ export default function useBlockSync( { if ( clientId !== null && getBlockName( clientId ) === null ) return; - const newIsPersistent = isLastBlockChangePersistent(); + // When RESET_BLOCKS on parent blocks get called, the controlled blocks + // can reset to uncontrolled, in these situations, it means we need to populate + // the blocks again from the external blocks (the value property here) + // and we should stop triggering onChange + const isStillControlled = + ! clientId || areInnerBlocksControlled( clientId ); + if ( ! isStillControlled ) { + return; + } + const newIsPersistent = isLastBlockChangePersistent(); const newBlocks = getBlocks( clientId ); const areBlocksDifferent = newBlocks !== blocks; blocks = newBlocks; diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index e5c333aacd223d..7aa91bd85b1f02 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -737,6 +737,27 @@ const withSaveReusableBlock = ( reducer ) => ( state, action ) => { return reducer( state, action ); }; +/** + * Higher-order reducer which removes blocks from state when switching parent block controlled state. + * + * @param {Function} reducer Original reducer function. + * + * @return {Function} Enhanced reducer function. + */ +const withResetControlledBlocks = ( reducer ) => ( state, action ) => { + if ( action.type === 'SET_HAS_CONTROLLED_INNER_BLOCKS' ) { + // when switching a block from controlled to uncontrolled or inverse, + // we need to remove its content first. + const tempState = reducer( state, { + type: 'REPLACE_INNER_BLOCKS', + rootClientId: action.clientId, + blocks: [], + } ); + return reducer( tempState, action ); + } + + return reducer( state, action ); +}; /** * Reducer returning the blocks state. @@ -754,7 +775,8 @@ export const blocks = flow( withReplaceInnerBlocks, // needs to be after withInnerBlocksRemoveCascade withBlockReset, withPersistentBlockChange, - withIgnoredBlockChange + withIgnoredBlockChange, + withResetControlledBlocks )( { byClientId( state = {}, action ) { switch ( action.type ) { diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 0aec777af05542..66f3e98533ee88 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -2050,6 +2050,41 @@ describe( 'state', () => { expect( state.isIgnoredChange ).toBe( true ); } ); } ); + + describe( 'controlledInnerBlocks', () => { + it( 'should remove the content of the block if it switches from controlled to uncontrolled or opposite', () => { + const original = blocks( undefined, { + type: 'RESET_BLOCKS', + blocks: [ + { + clientId: 'chicken', + name: 'core/test-block', + attributes: {}, + innerBlocks: [ + { + clientId: 'child', + name: 'core/test-block', + attributes: {}, + innerBlocks: [], + }, + ], + }, + ], + } ); + + const state = blocks( original, { + type: 'SET_HAS_CONTROLLED_INNER_BLOCKS', + clientId: 'chicken', + hasControlledInnerBlocks: true, + } ); + + expect( state.controlledInnerBlocks.chicken ).toBe( true ); + // The previous content of the block should be removed + expect( state.byClientId.child ).toBeUndefined(); + expect( state.tree.child ).toBeUndefined(); + expect( state.tree.chicken.innerBlocks ).toEqual( [] ); + } ); + } ); } ); } ); diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 441fee4dbff23e..eb7ed5bc2ab631 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -28,7 +28,7 @@ import { } from '@wordpress/block-editor'; import { EntityProvider, useEntityProp } from '@wordpress/core-data'; -import { useDispatch, useSelect } from '@wordpress/data'; +import { useDispatch, useSelect, useRegistry } from '@wordpress/data'; import { PanelBody, ToggleControl, @@ -132,6 +132,7 @@ function Navigation( { const ref = navigationArea ? navigationAreaMenu : attributes.ref; + const registry = useRegistry(); const setRef = useCallback( ( postId ) => { setAttributes( { ref: postId } ); @@ -149,7 +150,6 @@ function Navigation( { const { hasUncontrolledInnerBlocks, uncontrolledInnerBlocks, - controlledInnerBlocks, isInnerBlockSelected, hasSubmenus, } = useSelect( @@ -178,7 +178,6 @@ function Navigation( { ), hasUncontrolledInnerBlocks: _hasUncontrolledInnerBlocks, uncontrolledInnerBlocks: _uncontrolledInnerBlocks, - controlledInnerBlocks: _controlledInnerBlocks, isInnerBlockSelected: hasSelectedInnerBlock( clientId, true ), }; }, @@ -322,18 +321,6 @@ function Navigation( { setIsPlaceholderShown( ! isEntityAvailable ); }, [ isEntityAvailable ] ); - // If the ref no longer exists the reset the inner blocks to provide a - // clean slate. - useEffect( () => { - if ( - ! hasUncontrolledInnerBlocks && - controlledInnerBlocks?.length > 0 && - ref === undefined - ) { - replaceInnerBlocks( clientId, [] ); - } - }, [ clientId, ref, hasUncontrolledInnerBlocks, controlledInnerBlocks ] ); - const [ showCantEditNotice, hideCantEditNotice ] = useNavigationNotice( { name: 'block-library/core/navigation/permissions/update', message: __( @@ -383,15 +370,19 @@ function Navigation( { ] ); const startWithEmptyMenu = useCallback( () => { - if ( navigationArea ) { - setAreaMenu( 0 ); - } - setAttributes( { - ref: undefined, + registry.batch( () => { + if ( navigationArea ) { + setAreaMenu( 0 ); + } + setAttributes( { + ref: undefined, + } ); + if ( ! ref ) { + replaceInnerBlocks( clientId, [] ); + } + setIsPlaceholderShown( true ); } ); - - setIsPlaceholderShown( true ); - }, [ clientId ] ); + }, [ clientId, ref ] ); // If the block has inner blocks, but no menu id, this was an older // navigation block added before the block used a wp_navigation entity. @@ -423,11 +414,6 @@ function Navigation( { onSave={ ( post ) => { // Set some state used as a guard to prevent the creation of multiple posts. setHasSavedUnsavedInnerBlocks( true ); - // replaceInnerBlocks is required to ensure the block editor store is sync'd - // to be aware that there are now no inner blocks (as blocks moved to entity). - // This should probably happen automatically with useBlockSync - // but there appears to be a bug. - replaceInnerBlocks( clientId, [] ); // Switch to using the wp_navigation entity. setRef( post.id ); } } @@ -613,15 +599,7 @@ function Navigation( { { hasResolvedCanUserDeleteNavigationEntity && canUserDeleteNavigationEntity && ( { - if ( navigationArea ) { - setAreaMenu( 0 ); - } - setAttributes( { - ref: undefined, - } ); - setIsPlaceholderShown( true ); - } } + onDelete={ startWithEmptyMenu } /> ) } diff --git a/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js b/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js index 42a57a0ec81c72..030a7dbbfdd37e 100644 --- a/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js +++ b/packages/block-library/src/navigation/edit/unsaved-inner-blocks.js @@ -18,7 +18,6 @@ import { useContext, useEffect, useRef } from '@wordpress/element'; import useNavigationMenu from '../use-navigation-menu'; import useCreateNavigationMenu from './use-create-navigation-menu'; -const NOOP = () => {}; const EMPTY_OBJECT = {}; const DRAFT_MENU_PARAMS = [ 'postType', @@ -41,13 +40,6 @@ export default function UnsavedInnerBlocks( { const innerBlocksProps = useInnerBlocksProps( blockProps, { renderAppender: hasSelection ? undefined : false, - - // Make the inner blocks 'controlled'. This allows the block to always - // work with controlled inner blocks, smoothing out the switch to using - // an entity. - value: blocks, - onChange: NOOP, - onInput: NOOP, } ); const {