From ace97353e22183c395fa9abd903fe3a19fe73826 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Tue, 8 Oct 2024 17:45:15 +0800 Subject: [PATCH] Pattern block: Ensure consistent editing of overrides in Write Mode (#65408) * Move pattern block block editing state to store selector Fix broken `getEnabledBlockParents` selector unit tests Update test Fix flash of content when in zoom out mode * Combine into one selector * Add unit tests * Add zoom out pattern block editing mode test ---- Co-authored-by: talldan Co-authored-by: getdave Co-authored-by: youknowriad Co-authored-by: draganescu --- .../src/store/private-selectors.js | 18 +++ packages/block-editor/src/store/selectors.js | 38 ++++++ .../src/store/test/private-selectors.js | 3 +- .../block-editor/src/store/test/selectors.js | 112 +++++++++++++++++- packages/block-library/src/block/edit.js | 57 +-------- 5 files changed, 175 insertions(+), 53 deletions(-) diff --git a/packages/block-editor/src/store/private-selectors.js b/packages/block-editor/src/store/private-selectors.js index c3228980310656..eeb987fc00f616 100644 --- a/packages/block-editor/src/store/private-selectors.js +++ b/packages/block-editor/src/store/private-selectors.js @@ -684,3 +684,21 @@ export function getClosestAllowedInsertionPointForPattern( export function getInsertionPoint( state ) { return state.insertionPoint; } + +/** + * Retrieves the number of parent pattern blocks. + * + * @param {Object} state Global application state. + * @param {string} clientId The block client ID. + * + * @return {number} The number of parent pattern blocks. + */ +export function getParentPatternCount( state, clientId ) { + const parents = getBlockParents( state, clientId ); + return parents.reduce( ( count, parent ) => { + if ( getBlockName( state, parent ) === 'core/block' ) { + return count + 1; + } + return count; + }, 0 ); +} diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 6cf6aae296141f..812c48c87e7dca 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -40,6 +40,7 @@ import { getSectionRootClientId, isSectionBlock, getParentSectionBlock, + getParentPatternCount, } from './private-selectors'; /** @@ -2972,6 +2973,43 @@ export const getBlockEditingMode = createRegistrySelector( clientId = ''; } + // Handle pattern blocks (core/block) and the content of those blocks. + const parentPatternCount = getParentPatternCount( state, clientId ); + + // Make the outer pattern block content only mode. + if ( + getBlockName( state, clientId ) === 'core/block' && + parentPatternCount === 0 + ) { + return 'contentOnly'; + } + + if ( parentPatternCount > 0 ) { + // Disable nested patterns. + if ( parentPatternCount > 1 ) { + return 'disabled'; + } + + // Disable pattern content editing in zoom-out mode. + const _isZoomOut = + __unstableGetEditorMode( state ) === 'zoom-out'; + if ( _isZoomOut ) { + return 'disabled'; + } + + // If the block has a binding of any kind, allow content only editing. + const attributes = getBlockAttributes( state, clientId ); + if ( + Object.keys( attributes?.metadata?.bindings ?? {} ) + ?.length > 0 + ) { + return 'contentOnly'; + } + + // Otherwise, the block is part of the pattern source and should not be editable. + return 'disabled'; + } + // In zoom-out mode, override the behavior set by // __unstableSetBlockEditingMode to only allow editing the top-level // sections. diff --git a/packages/block-editor/src/store/test/private-selectors.js b/packages/block-editor/src/store/test/private-selectors.js index cbb75daa4baaa0..cca0714f3856d7 100644 --- a/packages/block-editor/src/store/test/private-selectors.js +++ b/packages/block-editor/src/store/test/private-selectors.js @@ -428,7 +428,6 @@ describe( 'private selectors', () => { 'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c', ], ] ), - order: new Map( [ [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337', @@ -443,6 +442,7 @@ describe( 'private selectors', () => { ], [ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ], ] ), + byClientId: new Map( [] ), }, blockEditingModes: new Map( [ [ '', 'disabled' ], @@ -495,6 +495,7 @@ describe( 'private selectors', () => { ], [ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ], ] ), + byClientId: new Map( [] ), }, blockEditingModes: new Map( [ [ '', 'disabled' ], diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index a08c2e0dde1508..67af4d1b631162 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -3079,7 +3079,7 @@ describe( 'selectors', () => { byClientId: new Map( Object.entries( { block1: { name: 'core/test-block-ancestor' }, - block2: { name: 'core/block' }, + block2: { name: 'core/group' }, block3: { name: 'core/test-block-parent' }, } ) ), @@ -4668,4 +4668,114 @@ describe( 'getBlockEditingMode', () => { ) ).toBe( 'disabled' ); } ); + + describe( 'pattern blocks', () => { + const patternBlockState = { + settings: {}, + blocks: { + byClientId: new Map( + Object.entries( { + 'pattern-a': { name: 'core/block' }, + 'pattern-b': { name: 'core/block' }, + 'heading-a': { name: 'core/heading' }, + 'paragraph-a': { name: 'core/paragraph' }, + 'paragraph-b': { name: 'core/paragraph' }, + } ) + ), + order: new Map( + Object.entries( { + '': [ 'pattern-a' ], + 'pattern-a': [ + 'heading-a', + 'paragraph-a', + 'pattern-b', + ], + 'pattern-b': [ 'paragraph-b' ], + } ) + ), + parents: new Map( + Object.entries( { + 'paragraph-b': 'pattern-b', + 'pattern-b': 'pattern-a', + 'paragraph-a': 'pattern-a', + 'heading-a': 'pattern-a', + 'pattern-a': '', + } ) + ), + blockListSettings: { + 'pattern-a': {}, + 'pattern-b': {}, + }, + attributes: new Map( + Object.entries( { + 'paragraph-a': { + metadata: { + bindings: { + __default: { + source: 'core/pattern-overrides', + }, + }, + }, + }, + 'paragraph-b': { + metadata: { + bindings: { + __default: { + source: 'core/pattern-overrides', + }, + }, + }, + }, + } ) + ), + }, + }; + + it( 'should return contentOnly for the outer pattern block', () => { + expect( + getBlockEditingMode( patternBlockState, 'pattern-a' ) + ).toBe( 'contentOnly' ); + } ); + + it( 'should return contentOnly for blocks with bindings in the outer pattern', () => { + expect( + getBlockEditingMode( patternBlockState, 'paragraph-a' ) + ).toBe( 'contentOnly' ); + } ); + + it( 'should return disabled for unbound blocks', () => { + expect( + getBlockEditingMode( patternBlockState, 'heading-a' ) + ).toBe( 'disabled' ); + } ); + + it( 'should return disabled for the nested pattern', () => { + expect( + getBlockEditingMode( patternBlockState, 'pattern-a' ) + ).toBe( 'contentOnly' ); + } ); + + it( 'should return disabled for blocks with bindings in the nested pattern', () => { + expect( + getBlockEditingMode( patternBlockState, 'paragraph-b' ) + ).toBe( 'disabled' ); + } ); + + it( 'should disable all inner blocks of the outer pattern in zoom out mode with the outer pattern in content only mode', () => { + const state = { + ...patternBlockState, + editorMode: 'zoom-out', + }; + expect( getBlockEditingMode( state, 'pattern-a' ) ).toBe( + 'contentOnly' + ); + [ 'paragraph-a', 'paragraph-b', 'heading-a', 'pattern-b' ].forEach( + ( block ) => { + expect( getBlockEditingMode( state, block ) ).toBe( + 'disabled' + ); + } + ); + } ); + } ); } ); diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 104b07157cba74..3d4d07e52b386a 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -7,7 +7,7 @@ import clsx from 'clsx'; * WordPress dependencies */ import { useSelect, useDispatch } from '@wordpress/data'; -import { useRef, useMemo, useEffect } from '@wordpress/element'; +import { useRef, useMemo } from '@wordpress/element'; import { useEntityRecord, store as coreStore, @@ -37,12 +37,10 @@ import { getBlockBindingsSource } from '@wordpress/blocks'; /** * Internal dependencies */ -import { name as patternBlockName } from './index'; import { unlock } from '../lock-unlock'; const { useLayoutClasses } = unlock( blockEditorPrivateApis ); -const { isOverridableBlock, hasOverridableBlocks } = - unlock( patternsPrivateApis ); +const { hasOverridableBlocks } = unlock( patternsPrivateApis ); const fullAlignments = [ 'full', 'wide', 'left', 'right' ]; @@ -75,22 +73,6 @@ const useInferredLayout = ( blocks, parentLayout ) => { }, [ blocks, parentLayout ] ); }; -function setBlockEditMode( setEditMode, blocks, mode ) { - blocks.forEach( ( block ) => { - const editMode = - mode || - ( isOverridableBlock( block ) ? 'contentOnly' : 'disabled' ); - setEditMode( block.clientId, editMode ); - - setBlockEditMode( - setEditMode, - block.innerBlocks, - // Disable editing for nested patterns. - block.name === patternBlockName ? 'disabled' : mode - ); - } ); -} - function RecursionWarning() { const blockProps = useBlockProps(); return ( @@ -171,7 +153,6 @@ function ReusableBlockEdit( { name, attributes: { ref, content }, __unstableParentLayout: parentLayout, - clientId: patternClientId, setAttributes, } ) { const { record, hasResolved } = useEntityRecord( @@ -184,49 +165,24 @@ function ReusableBlockEdit( { } ); const isMissing = hasResolved && ! record; - const { setBlockEditingMode, __unstableMarkLastChangeAsPersistent } = + const { __unstableMarkLastChangeAsPersistent } = useDispatch( blockEditorStore ); - const { - innerBlocks, - onNavigateToEntityRecord, - editingMode, - hasPatternOverridesSource, - } = useSelect( + const { onNavigateToEntityRecord, hasPatternOverridesSource } = useSelect( ( select ) => { - const { getBlocks, getSettings, getBlockEditingMode } = - select( blockEditorStore ); + const { getSettings } = select( blockEditorStore ); // For editing link to the site editor if the theme and user permissions support it. return { - innerBlocks: getBlocks( patternClientId ), onNavigateToEntityRecord: getSettings().onNavigateToEntityRecord, - editingMode: getBlockEditingMode( patternClientId ), hasPatternOverridesSource: !! getBlockBindingsSource( 'core/pattern-overrides' ), }; }, - [ patternClientId ] + [] ); - // Sync the editing mode of the pattern block with the inner blocks. - useEffect( () => { - setBlockEditMode( - setBlockEditingMode, - innerBlocks, - // Disable editing if the pattern itself is disabled. - editingMode === 'disabled' || ! hasPatternOverridesSource - ? 'disabled' - : undefined - ); - }, [ - editingMode, - innerBlocks, - setBlockEditingMode, - hasPatternOverridesSource, - ] ); - const canOverrideBlocks = useMemo( () => hasPatternOverridesSource && hasOverridableBlocks( blocks ), [ hasPatternOverridesSource, blocks ] @@ -244,7 +200,6 @@ function ReusableBlockEdit( { } ); const innerBlocksProps = useInnerBlocksProps( blockProps, { - templateLock: 'all', layout, value: blocks, onInput: NOOP,