From 78f960185df3dc1c3d26f084f9375b4431444fab Mon Sep 17 00:00:00 2001 From: Ella Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 01/44] Pattern overrides: use block binding editing API --- packages/block-library/src/block/edit.js | 276 ++---------------- .../editor/src/bindings/pattern-overrides.js | 45 ++- 2 files changed, 70 insertions(+), 251 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index a274843ff37ed..8e8db547a8691 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -6,9 +6,13 @@ import clsx from 'clsx'; /** * WordPress dependencies */ -import { useRegistry, useSelect, useDispatch } from '@wordpress/data'; +import { useSelect, useDispatch } from '@wordpress/data'; import { useRef, useMemo, useEffect } from '@wordpress/element'; -import { useEntityRecord, store as coreStore } from '@wordpress/core-data'; +import { + useEntityRecord, + store as coreStore, + useEntityBlockEditor, +} from '@wordpress/core-data'; import { Placeholder, Spinner, @@ -20,7 +24,6 @@ import { useInnerBlocksProps, RecursionProvider, useHasRecursion, - InnerBlocks, useBlockProps, Warning, privateApis as blockEditorPrivateApis, @@ -28,8 +31,7 @@ import { BlockControls, } from '@wordpress/block-editor'; import { privateApis as patternsPrivateApis } from '@wordpress/patterns'; -import { parse, cloneBlock, store as blocksStore } from '@wordpress/blocks'; -import { RichTextData } from '@wordpress/rich-text'; +import { store as blocksStore } from '@wordpress/blocks'; /** * Internal dependencies @@ -42,25 +44,6 @@ const { isOverridableBlock } = unlock( patternsPrivateApis ); const fullAlignments = [ 'full', 'wide', 'left', 'right' ]; -function getLegacyIdMap( blocks, content, nameCount = {} ) { - let idToClientIdMap = {}; - for ( const block of blocks ) { - if ( block?.innerBlocks?.length ) { - idToClientIdMap = { - ...idToClientIdMap, - ...getLegacyIdMap( block.innerBlocks, content, nameCount ), - }; - } - - const id = block.attributes.metadata?.id; - const clientId = block.clientId; - if ( id && content?.[ id ] ) { - idToClientIdMap[ clientId ] = id; - } - } - return idToClientIdMap; -} - const useInferredLayout = ( blocks, parentLayout ) => { const initialInferredAlignmentRef = useRef(); @@ -99,112 +82,6 @@ function hasOverridableBlocks( blocks ) { } ); } -function getOverridableAttributes( block ) { - return Object.entries( block.attributes.metadata.bindings ) - .filter( - ( [ , binding ] ) => binding.source === 'core/pattern-overrides' - ) - .map( ( [ attributeKey ] ) => attributeKey ); -} - -function applyInitialContentValuesToInnerBlocks( - blocks, - content = {}, - defaultValues, - legacyIdMap -) { - return blocks.map( ( block ) => { - const innerBlocks = applyInitialContentValuesToInnerBlocks( - block.innerBlocks, - content, - defaultValues, - legacyIdMap - ); - const metadataName = - legacyIdMap?.[ block.clientId ] ?? block.attributes.metadata?.name; - - if ( ! metadataName || ! isOverridableBlock( block ) ) { - return { ...block, innerBlocks }; - } - - const attributes = getOverridableAttributes( block ); - const newAttributes = { ...block.attributes }; - for ( const attributeKey of attributes ) { - defaultValues[ metadataName ] ??= {}; - defaultValues[ metadataName ][ attributeKey ] = - block.attributes[ attributeKey ]; - - const contentValues = content[ metadataName ]; - if ( contentValues?.[ attributeKey ] !== undefined ) { - newAttributes[ attributeKey ] = contentValues[ attributeKey ]; - } - } - return { - ...block, - attributes: newAttributes, - innerBlocks, - }; - } ); -} - -function isAttributeEqual( attribute1, attribute2 ) { - if ( - attribute1 instanceof RichTextData && - attribute2 instanceof RichTextData - ) { - return attribute1.toString() === attribute2.toString(); - } - return attribute1 === attribute2; -} - -function getContentValuesFromInnerBlocks( blocks, defaultValues, legacyIdMap ) { - /** @type {Record}>} */ - const content = {}; - for ( const block of blocks ) { - if ( block.name === patternBlockName ) { - continue; - } - if ( block.innerBlocks.length ) { - Object.assign( - content, - getContentValuesFromInnerBlocks( - block.innerBlocks, - defaultValues, - legacyIdMap - ) - ); - } - const metadataName = - legacyIdMap?.[ block.clientId ] ?? block.attributes.metadata?.name; - if ( ! metadataName || ! isOverridableBlock( block ) ) { - continue; - } - - const attributes = getOverridableAttributes( block ); - - for ( const attributeKey of attributes ) { - if ( - ! isAttributeEqual( - block.attributes[ attributeKey ], - defaultValues?.[ metadataName ]?.[ attributeKey ] - ) - ) { - content[ metadataName ] ??= {}; - // TODO: We need a way to represent `undefined` in the serialized overrides. - // Also see: https://github.com/WordPress/gutenberg/pull/57249#discussion_r1452987871 - content[ metadataName ][ attributeKey ] = - block.attributes[ attributeKey ] === undefined - ? // TODO: We use an empty string to represent undefined for now until - // we support a richer format for overrides and the block binding API. - // Currently only the `linkTarget` attribute of `core/button` is affected. - '' - : block.attributes[ attributeKey ]; - } - } - } - return Object.keys( content ).length > 0 ? content : undefined; -} - function setBlockEditMode( setEditMode, blocks, mode ) { blocks.forEach( ( block ) => { const editMode = @@ -257,51 +134,35 @@ function ReusableBlockEdit( { clientId: patternClientId, setAttributes, } ) { - const registry = useRegistry(); - const { record, editedRecord, hasResolved } = useEntityRecord( + const { record, hasResolved } = useEntityRecord( 'postType', 'wp_block', ref ); + const [ blocks, onInput, onChange ] = useEntityBlockEditor( + 'postType', + 'wp_block', + { id: ref } + ); const isMissing = hasResolved && ! record; - // The value of the `content` attribute, stored in a `ref` to avoid triggering the effect - // that runs `applyInitialContentValuesToInnerBlocks` unnecessarily. - const contentRef = useRef( content ); - - // The default content values from the original pattern for overridable attributes. - // Set by the `applyInitialContentValuesToInnerBlocks` function. - const defaultContent = useRef( {} ); - - const { - replaceInnerBlocks, - __unstableMarkNextChangeAsNotPersistent, - setBlockEditingMode, - } = useDispatch( blockEditorStore ); - const { syncDerivedUpdates } = unlock( useDispatch( blockEditorStore ) ); + const { setBlockEditingMode } = useDispatch( blockEditorStore ); const { - innerBlocks, userCanEdit, - getBlockEditingMode, onNavigateToEntityRecord, editingMode, hasPatternOverridesSource, } = useSelect( ( select ) => { const { canUser } = select( coreStore ); - const { - getBlocks, - getSettings, - getBlockEditingMode: _getBlockEditingMode, - } = select( blockEditorStore ); + const { getSettings, getBlockEditingMode: _getBlockEditingMode } = + select( blockEditorStore ); const { getBlockBindingsSource } = unlock( select( blocksStore ) ); - const blocks = getBlocks( patternClientId ); const canEdit = canUser( 'update', 'blocks', ref ); // For editing link to the site editor if the theme and user permissions support it. return { - innerBlocks: blocks, userCanEdit: canEdit, getBlockEditingMode: _getBlockEditingMode, onNavigateToEntityRecord: @@ -319,7 +180,7 @@ function ReusableBlockEdit( { useEffect( () => { setBlockEditMode( setBlockEditingMode, - innerBlocks, + blocks, // Disable editing if the pattern itself is disabled. editingMode === 'disabled' || ! hasPatternOverridesSource ? 'disabled' @@ -327,70 +188,17 @@ function ReusableBlockEdit( { ); }, [ editingMode, - innerBlocks, + blocks, setBlockEditingMode, hasPatternOverridesSource, ] ); const canOverrideBlocks = useMemo( - () => hasPatternOverridesSource && hasOverridableBlocks( innerBlocks ), - [ hasPatternOverridesSource, innerBlocks ] + () => hasPatternOverridesSource && hasOverridableBlocks( blocks ), + [ hasPatternOverridesSource, blocks ] ); - const initialBlocks = useMemo( - () => - // Clone the blocks to generate new client IDs. - editedRecord.blocks?.map( ( block ) => cloneBlock( block ) ) ?? - ( editedRecord.content && typeof editedRecord.content !== 'function' - ? parse( editedRecord.content ) - : [] ), - [ editedRecord.blocks, editedRecord.content ] - ); - - const legacyIdMap = useRef( {} ); - - // Apply the initial overrides from the pattern block to the inner blocks. - useEffect( () => { - // Build a map of clientIds to the old nano id system to provide back compat. - legacyIdMap.current = getLegacyIdMap( - initialBlocks, - contentRef.current - ); - defaultContent.current = {}; - const originalEditingMode = getBlockEditingMode( patternClientId ); - // Replace the contents of the blocks with the overrides. - registry.batch( () => { - setBlockEditingMode( patternClientId, 'default' ); - syncDerivedUpdates( () => { - const blocks = hasPatternOverridesSource - ? applyInitialContentValuesToInnerBlocks( - initialBlocks, - contentRef.current, - defaultContent.current, - legacyIdMap.current - ) - : initialBlocks; - - replaceInnerBlocks( patternClientId, blocks ); - } ); - setBlockEditingMode( patternClientId, originalEditingMode ); - } ); - }, [ - hasPatternOverridesSource, - __unstableMarkNextChangeAsNotPersistent, - patternClientId, - initialBlocks, - replaceInnerBlocks, - registry, - getBlockEditingMode, - setBlockEditingMode, - syncDerivedUpdates, - ] ); - - const { alignment, layout } = useInferredLayout( - innerBlocks, - parentLayout - ); + const { alignment, layout } = useInferredLayout( blocks, parentLayout ); const layoutClasses = useLayoutClasses( { layout }, name ); const blockProps = useBlockProps( { @@ -404,44 +212,12 @@ function ReusableBlockEdit( { const innerBlocksProps = useInnerBlocksProps( blockProps, { templateLock: 'contentOnly', layout, - renderAppender: innerBlocks?.length - ? undefined - : InnerBlocks.ButtonBlockAppender, + value: blocks, + onInput, + onChange, + renderAppender: blocks?.length ? undefined : blocks.ButtonBlockAppender, } ); - // Sync the `content` attribute from the updated blocks to the pattern block. - // `syncDerivedUpdates` is used here to avoid creating an additional undo level. - useEffect( () => { - if ( ! hasPatternOverridesSource ) { - return; - } - const { getBlocks } = registry.select( blockEditorStore ); - let prevBlocks = getBlocks( patternClientId ); - return registry.subscribe( () => { - const blocks = getBlocks( patternClientId ); - if ( blocks !== prevBlocks ) { - prevBlocks = blocks; - syncDerivedUpdates( () => { - const updatedContent = getContentValuesFromInnerBlocks( - blocks, - defaultContent.current, - legacyIdMap.current - ); - setAttributes( { - content: updatedContent, - } ); - contentRef.current = updatedContent; - } ); - } - }, blockEditorStore ); - }, [ - hasPatternOverridesSource, - syncDerivedUpdates, - patternClientId, - registry, - setAttributes, - ] ); - const handleEditOriginal = () => { onNavigateToEntityRecord( { postId: ref, @@ -451,7 +227,7 @@ function ReusableBlockEdit( { const resetContent = () => { if ( content ) { - replaceInnerBlocks( patternClientId, initialBlocks ); + setAttributes( { content: undefined } ); } }; diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 5f7f475a649a3..972b2ad08cc99 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -2,10 +2,53 @@ * WordPress dependencies */ import { _x } from '@wordpress/i18n'; +import { store as blockEditorStore } from '@wordpress/block-editor'; + +const CONTENT = 'content'; export default { name: 'core/pattern-overrides', label: _x( 'Pattern Overrides', 'block bindings source' ), - useSource: null, + getValue( { registry, clientId, attributeName } ) { + const { getBlockAttributes, getBlockParents, getBlockName } = + registry.select( blockEditorStore ); + const currentBlockAttributes = getBlockAttributes( clientId ); + const parents = getBlockParents( clientId, true ); + const patternClientId = parents.find( + ( id ) => getBlockName( id ) === 'core/block' + ); + + return getBlockAttributes( patternClientId )?.[ CONTENT ]?.[ + currentBlockAttributes?.metadata?.name + ]?.[ attributeName ]; + }, + setValue( { registry, clientId, attributeName, value } ) { + const { getBlockAttributes, getBlockParents, getBlockName } = + registry.select( blockEditorStore ); + const currentBlockAttributes = getBlockAttributes( clientId ); + const parents = getBlockParents( clientId, true ); + const patternClientId = parents.find( + ( id ) => getBlockName( id ) === 'core/block' + ); + const blockName = currentBlockAttributes?.metadata?.name; + + if ( ! patternClientId ) { + return; + } + + const currentBindingValue = + getBlockAttributes( patternClientId )?.[ CONTENT ]; + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...currentBindingValue, + [ blockName ]: { + ...currentBindingValue?.[ blockName ], + [ attributeName ]: value, + }, + }, + } ); + }, lockAttributesEditing: false, }; From 17b31b8ccba83a3fe25e4fa7efdb689430e42850 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 02/44] Update only the needed attributes with bindings --- .../src/hooks/use-bindings-attributes.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 5ca526b5749b7..1c658e2b52783 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -115,25 +115,28 @@ export const withBlockBindingSupport = createHigherOrderComponent( const keptAttributes = { ...nextAttributes }; - for ( const [ - attributeName, - boundAttribute, - ] of Object.entries( bindings ) ) { - const source = sources[ boundAttribute.source ]; + for ( const [ attributeName, newValue ] of Object.entries( + nextAttributes + ) ) { if ( - ! source?.setValue || + ! bindings[ attributeName ] || ! canBindAttribute( name, attributeName ) ) { continue; } + const source = + sources[ bindings[ attributeName ].source ]; + if ( ! source?.setValue ) { + continue; + } source.setValue( { registry, context, clientId, attributeName, - args: boundAttribute.args, - value: nextAttributes[ attributeName ], + args: bindings[ attributeName ].args, + value: newValue, } ); delete keptAttributes[ attributeName ]; } From 63c6b6d9b9c98e13209027c786789b4880a9aec7 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 03/44] Keep blocks as normal when editing pattern --- .../editor/src/bindings/pattern-overrides.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 972b2ad08cc99..c3b615192ee81 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -18,6 +18,16 @@ export default { ( id ) => getBlockName( id ) === 'core/block' ); + const overridableValue = + getBlockAttributes( patternClientId )?.[ CONTENT ]?.[ + currentBlockAttributes?.metadata?.name + ]?.[ attributeName ]; + + // If there is no pattern client ID, or it is not overwritten, return the default value. + if ( ! patternClientId || ! overridableValue ) { + return currentBlockAttributes[ attributeName ]; + } + return getBlockAttributes( patternClientId )?.[ CONTENT ]?.[ currentBlockAttributes?.metadata?.name ]?.[ attributeName ]; @@ -32,7 +42,13 @@ export default { ); const blockName = currentBlockAttributes?.metadata?.name; + // If there is no pattern client ID, set attributes as normal. if ( ! patternClientId ) { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( clientId, { + [ attributeName ]: value, + } ); return; } From 46063dbe49bbc9fc88ad6f29f753f7407417c86b Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 04/44] Use inner blocks for set edit mode --- packages/block-library/src/block/edit.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 8e8db547a8691..152d7915bf802 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -149,6 +149,7 @@ function ReusableBlockEdit( { const { setBlockEditingMode } = useDispatch( blockEditorStore ); const { + innerBlocks, userCanEdit, onNavigateToEntityRecord, editingMode, @@ -156,13 +157,17 @@ function ReusableBlockEdit( { } = useSelect( ( select ) => { const { canUser } = select( coreStore ); - const { getSettings, getBlockEditingMode: _getBlockEditingMode } = - select( blockEditorStore ); + const { + getBlocks, + getSettings, + getBlockEditingMode: _getBlockEditingMode, + } = select( blockEditorStore ); const { getBlockBindingsSource } = unlock( select( blocksStore ) ); const canEdit = canUser( 'update', 'blocks', ref ); // For editing link to the site editor if the theme and user permissions support it. return { + innerBlocks: getBlocks( patternClientId ), userCanEdit: canEdit, getBlockEditingMode: _getBlockEditingMode, onNavigateToEntityRecord: @@ -180,7 +185,7 @@ function ReusableBlockEdit( { useEffect( () => { setBlockEditMode( setBlockEditingMode, - blocks, + innerBlocks, // Disable editing if the pattern itself is disabled. editingMode === 'disabled' || ! hasPatternOverridesSource ? 'disabled' @@ -188,7 +193,7 @@ function ReusableBlockEdit( { ); }, [ editingMode, - blocks, + innerBlocks, setBlockEditingMode, hasPatternOverridesSource, ] ); From f1f78ae0c1fad0ada9dbca4795f6c3a8b74a39e5 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 05/44] Change indiviual reset logic --- .../src/components/reset-overrides-control.js | 78 ++++++------------- 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 1d3ae013addd3..d2026dc770995 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -6,80 +6,52 @@ import { BlockControls, } from '@wordpress/block-editor'; import { ToolbarButton, ToolbarGroup } from '@wordpress/components'; -import { useSelect, useRegistry } from '@wordpress/data'; -import { store as coreStore } from '@wordpress/core-data'; -import { parse } from '@wordpress/blocks'; +import { useDispatch, useSelect } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; -function recursivelyFindBlockWithName( blocks, name ) { - for ( const block of blocks ) { - if ( block.attributes.metadata?.name === name ) { - return block; - } - - const found = recursivelyFindBlockWithName( block.innerBlocks, name ); - if ( found ) { - return found; - } - } -} +const CONTENT = 'content'; export default function ResetOverridesControl( props ) { - const registry = useRegistry(); const name = props.attributes.metadata?.name; - const patternWithOverrides = useSelect( + const { updateBlockAttributes } = useDispatch( blockEditorStore ); + + const { isOverwritten, resetOverrides } = useSelect( ( select ) => { if ( ! name ) { return undefined; } - const { getBlockParentsByBlockName, getBlocksByClientId } = + const { getBlockAttributes, getBlockParents, getBlockName } = select( blockEditorStore ); - const patternBlock = getBlocksByClientId( - getBlockParentsByBlockName( props.clientId, 'core/block' ) - )[ 0 ]; - - if ( ! patternBlock?.attributes.content?.[ name ] ) { - return undefined; - } + const parents = getBlockParents( props.clientId, true ); + const patternClientId = parents.find( + ( id ) => getBlockName( id ) === 'core/block' + ); - return patternBlock; + const patternAttributes = getBlockAttributes( patternClientId ); + const existingOverrides = patternAttributes?.[ CONTENT ]; + + return { + isOverwritten: !! existingOverrides?.[ name ], + resetOverrides: async () => { + updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...existingOverrides, + [ name ]: undefined, + }, + } ); + }, + }; }, [ props.clientId, name ] ); - const resetOverrides = async () => { - const editedRecord = await registry - .resolveSelect( coreStore ) - .getEditedEntityRecord( - 'postType', - 'wp_block', - patternWithOverrides.attributes.ref - ); - const blocks = editedRecord.blocks ?? parse( editedRecord.content ); - const block = recursivelyFindBlockWithName( blocks, name ); - - const newAttributes = Object.assign( - // Reset every existing attribute to undefined. - Object.fromEntries( - Object.keys( props.attributes ).map( ( key ) => [ - key, - undefined, - ] ) - ), - // Then assign the original attributes. - block.attributes - ); - - props.setAttributes( newAttributes ); - }; - return ( { __( 'Reset' ) } From 8121723e60bccd76816752ad217331a2790f5638 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 06/44] Move variable after early return --- packages/editor/src/bindings/pattern-overrides.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index c3b615192ee81..42b63b52be2aa 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -35,13 +35,10 @@ export default { setValue( { registry, clientId, attributeName, value } ) { const { getBlockAttributes, getBlockParents, getBlockName } = registry.select( blockEditorStore ); - const currentBlockAttributes = getBlockAttributes( clientId ); const parents = getBlockParents( clientId, true ); const patternClientId = parents.find( ( id ) => getBlockName( id ) === 'core/block' ); - const blockName = currentBlockAttributes?.metadata?.name; - // If there is no pattern client ID, set attributes as normal. if ( ! patternClientId ) { registry @@ -52,6 +49,8 @@ export default { return; } + const currentBlockAttributes = getBlockAttributes( clientId ); + const blockName = currentBlockAttributes?.metadata?.name; const currentBindingValue = getBlockAttributes( patternClientId )?.[ CONTENT ]; registry From ec4daec9870952b488139f8932f29573a1717aef Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:16 +0200 Subject: [PATCH 07/44] Add inline comment for bindings setAttributes --- packages/block-editor/src/hooks/use-bindings-attributes.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 1c658e2b52783..aabb04343ffa8 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -115,6 +115,7 @@ export const withBlockBindingSupport = createHigherOrderComponent( const keptAttributes = { ...nextAttributes }; + // Loop only over the updated attributes to avoid modifying the bound ones that haven't changed. for ( const [ attributeName, newValue ] of Object.entries( nextAttributes ) ) { From 35aa0abb80cc28fc5091ace8f074346e3ce3db2d Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 08/44] Change variable to isOverriden --- packages/patterns/src/components/reset-overrides-control.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index d2026dc770995..0a3fea672b699 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -15,7 +15,7 @@ export default function ResetOverridesControl( props ) { const name = props.attributes.metadata?.name; const { updateBlockAttributes } = useDispatch( blockEditorStore ); - const { isOverwritten, resetOverrides } = useSelect( + const { isOverriden, resetOverrides } = useSelect( ( select ) => { if ( ! name ) { return undefined; @@ -51,7 +51,7 @@ export default function ResetOverridesControl( props ) { { __( 'Reset' ) } From a71ba38c761942bbab986a2b8378fef5cc81febe Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 09/44] Change missing isOverriden --- packages/patterns/src/components/reset-overrides-control.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 0a3fea672b699..38c4727e92601 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -32,7 +32,7 @@ export default function ResetOverridesControl( props ) { const existingOverrides = patternAttributes?.[ CONTENT ]; return { - isOverwritten: !! existingOverrides?.[ name ], + isOverriden: !! existingOverrides?.[ name ], resetOverrides: async () => { updateBlockAttributes( patternClientId, { [ CONTENT ]: { From 29afc1f1505b30f6532cd2a0e6462e8eef34a884 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 10/44] Fix detaching of synced patterns --- packages/patterns/src/store/actions.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/patterns/src/store/actions.js b/packages/patterns/src/store/actions.js index 7b9090ae4de66..31360f9148d92 100644 --- a/packages/patterns/src/store/actions.js +++ b/packages/patterns/src/store/actions.js @@ -115,11 +115,15 @@ export const convertSyncedPatternToStatic = } ); } + const patternInnerBlocks = registry + .select( blockEditorStore ) + .getBlocks( patternBlock.clientId ); + registry .dispatch( blockEditorStore ) .replaceBlocks( patternBlock.clientId, - cloneBlocksAndRemoveBindings( patternBlock.innerBlocks ) + cloneBlocksAndRemoveBindings( patternInnerBlocks ) ); }; From 31bf94cbea943e103bc62c22dee98afbc8d687ed Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 11/44] Sync values of bound attributes --- .../editor/src/bindings/pattern-overrides.js | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 42b63b52be2aa..c9727626463d9 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -33,23 +33,33 @@ export default { ]?.[ attributeName ]; }, setValue( { registry, clientId, attributeName, value } ) { - const { getBlockAttributes, getBlockParents, getBlockName } = + const { getBlockAttributes, getBlockParents, getBlockName, getBlocks } = registry.select( blockEditorStore ); + const currentBlockAttributes = getBlockAttributes( clientId ); const parents = getBlockParents( clientId, true ); const patternClientId = parents.find( ( id ) => getBlockName( id ) === 'core/block' ); - // If there is no pattern client ID, set attributes as normal. + // If there is no pattern client ID, sync blocks with the same name and same attributes. if ( ! patternClientId ) { - registry - .dispatch( blockEditorStore ) - .updateBlockAttributes( clientId, { - [ attributeName ]: value, - } ); + const blockName = currentBlockAttributes?.metadata?.name; + const syncBlocksWithSameName = ( blocks ) => { + for ( const block of blocks ) { + if ( block.attributes?.metadata?.name === blockName ) { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( block.clientId, { + [ attributeName ]: value, + } ); + } + syncBlocksWithSameName( block.innerBlocks ); + } + }; + + syncBlocksWithSameName( getBlocks() ); return; } - const currentBlockAttributes = getBlockAttributes( clientId ); const blockName = currentBlockAttributes?.metadata?.name; const currentBindingValue = getBlockAttributes( patternClientId )?.[ CONTENT ]; From 6c60eb3990c772bcfe3f246981060829dda1b338 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 12/44] Adapt synced pattern tests --- .../e2e/specs/editor/various/patterns.spec.js | 186 +++++++++++------- 1 file changed, 113 insertions(+), 73 deletions(-) diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js index fc1754330b3f5..8e598938bd0b7 100644 --- a/test/e2e/specs/editor/various/patterns.spec.js +++ b/test/e2e/specs/editor/various/patterns.spec.js @@ -3,6 +3,14 @@ */ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); +async function getPatternRecord( page, patternRef ) { + return await page.evaluate( async ( ref ) => { + return window.wp.data + .select( 'core' ) + .getEditedEntityRecord( 'postType', 'wp_block', ref ); + }, patternRef ); +} + test.describe( 'Unsynced pattern', () => { test.beforeAll( async ( { requestUtils } ) => { await requestUtils.deleteAllBlocks(); @@ -149,28 +157,22 @@ test.describe( 'Synced pattern', () => { .getByRole( 'button', { name: 'Create' } ) .click(); - await expect - .poll( - editor.getBlocks, - 'The block content should be wrapped by a pattern block wrapper' - ) - .toEqual( [ - { - name: 'core/block', - attributes: { ref: expect.any( Number ) }, - innerBlocks: [ - { - attributes: { - content: 'A useful paragraph to reuse', - dropCap: false, - }, - innerBlocks: [], - name: 'core/paragraph', - }, - ], - }, - ] ); - const after = await editor.getBlocks(); + // Wait until the pattern is inserted. + await editor.canvas + .getByRole( 'document', { + name: 'Block: Pattern', + } ) + .waitFor(); + + const [ syncedPattern ] = await editor.getBlocks(); + const patternRecord = await getPatternRecord( + page, + syncedPattern?.attributes?.ref + ); + + expect( patternRecord?.content ).toBe( + '\n

A useful paragraph to reuse

\n' + ); const patternBlock = editor.canvas.getByRole( 'document', { name: 'Block: Pattern', @@ -191,7 +193,30 @@ test.describe( 'Synced pattern', () => { .click(); await page.getByRole( 'option', { name: 'My synced pattern' } ).click(); - await expect.poll( editor.getBlocks ).toEqual( [ ...after, ...after ] ); + const [ firstSyncedPattern, secondSyncedPattern ] = + await editor.getBlocks(); + const firstSyncedPatternRecord = await getPatternRecord( + page, + firstSyncedPattern?.attributes?.ref + ); + const secondSyncedPatternRecord = await getPatternRecord( + page, + secondSyncedPattern?.attributes?.ref + ); + // Check first pattern is "My synced pattern". + expect( firstSyncedPatternRecord?.title ).toEqual( + 'My synced pattern' + ); + expect( firstSyncedPatternRecord?.content ).toEqual( + '\n

A useful paragraph to reuse

\n' + ); + // Check second pattern is "My synced pattern". + expect( secondSyncedPatternRecord?.title ).toEqual( + 'My synced pattern' + ); + expect( secondSyncedPatternRecord?.content ).toEqual( + '\n

A useful paragraph to reuse

\n' + ); } ); // Check for regressions of https://github.com/WordPress/gutenberg/issues/33072. @@ -254,22 +279,15 @@ test.describe( 'Synced pattern', () => { // Go back to the post. await editorTopBar.getByRole( 'button', { name: 'Back' } ).click(); - await expect.poll( editor.getBlocks ).toEqual( [ - { - name: 'core/block', - attributes: { ref: id }, - innerBlocks: [ - { - name: 'core/paragraph', - attributes: { - content: 'Einen Guten Tag!', - dropCap: false, - }, - innerBlocks: [], - }, - ], - }, - ] ); + const [ syncedPattern ] = await editor.getBlocks(); + const patternRecord = await getPatternRecord( + page, + syncedPattern?.attributes?.ref + ); + + expect( patternRecord?.content ).toBe( + '\n

Einen Guten Tag!

\n' + ); } ); // Check for regressions of https://github.com/WordPress/gutenberg/issues/26421. @@ -325,13 +343,16 @@ test.describe( 'Synced pattern', () => { attributes: { content: 'After Edit' }, }; - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: id }, - innerBlocks: [ expectedParagraphBlock ], - }, - ] ); + // It seems edited values are stored in the `blocks` array. + const [ syncedPattern ] = await editor.getBlocks(); + const [ modifiedBlock ] = ( + await getPatternRecord( page, syncedPattern?.attributes?.ref ) + )?.blocks; + + expect( modifiedBlock?.name ).toBe( expectedParagraphBlock.name ); + expect( modifiedBlock?.attributes?.content ).toBe( + expectedParagraphBlock.attributes.content + ); await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) @@ -345,6 +366,7 @@ test.describe( 'Synced pattern', () => { test( 'can be created, inserted, and converted to a regular block', async ( { editor, + page, requestUtils, } ) => { const { id } = await requestUtils.createBlock( { @@ -359,19 +381,24 @@ test.describe( 'Synced pattern', () => { attributes: { ref: id }, } ); + // // Wait until the pattern is created and inserted. + await editor.canvas.locator( 'text=Hello there!' ).waitFor(); + + const [ syncedPattern ] = await editor.getBlocks(); + const patternRecord = await getPatternRecord( + page, + syncedPattern?.attributes?.ref + ); + + expect( patternRecord?.content ).toBe( + '\n

Hello there!

\n' + ); + const expectedParagraphBlock = { name: 'core/paragraph', attributes: { content: 'Hello there!' }, }; - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: id }, - innerBlocks: [ expectedParagraphBlock ], - }, - ] ); - await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) ); @@ -413,22 +440,20 @@ test.describe( 'Synced pattern', () => { await page.keyboard.type( '/Awesome block' ); await page.getByRole( 'option', { name: 'Awesome block' } ).click(); - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: expect.any( Number ) }, - innerBlocks: [ - { - name: 'core/paragraph', - attributes: { content: 'Awesome Paragraph' }, - }, - ], - }, - ] ); + const [ syncedPattern ] = await editor.getBlocks(); + const patternRecord = await getPatternRecord( + page, + syncedPattern?.attributes?.ref + ); + + expect( patternRecord?.content ).toBe( + '\n

Awesome Paragraph

\n' + ); } ); test( 'can be created from multiselection and converted back to regular blocks', async ( { editor, + page, pageUtils, } ) => { await editor.insertBlock( { @@ -467,13 +492,28 @@ test.describe( 'Synced pattern', () => { }, ]; - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: expect.any( Number ) }, - innerBlocks: expectedParagraphBlocks, - }, - ] ); + // Wait until the pattern is created. + await editor.canvas + .getByRole( 'document', { + name: 'Block: Pattern', + } ) + .waitFor(); + + const [ syncedPattern ] = await editor.getBlocks(); + const patternRecord = await getPatternRecord( + page, + syncedPattern?.attributes?.ref + ); + + expect( patternRecord?.content ).toBe( + ` +

Hello there!

+ + + +

Second paragraph

+` + ); await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) From 49b062a25263a2afeb5127ec6aab80863cf4d8c0 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 13/44] Use overriden values when detaching pattern --- packages/patterns/src/store/actions.js | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/patterns/src/store/actions.js b/packages/patterns/src/store/actions.js index 31360f9148d92..e288904f25968 100644 --- a/packages/patterns/src/store/actions.js +++ b/packages/patterns/src/store/actions.js @@ -2,7 +2,7 @@ * WordPress dependencies */ -import { cloneBlock } from '@wordpress/blocks'; +import { getBlockType, cloneBlock } from '@wordpress/blocks'; import { store as coreStore } from '@wordpress/core-data'; import { store as blockEditorStore } from '@wordpress/block-editor'; @@ -93,6 +93,7 @@ export const convertSyncedPatternToStatic = const patternBlock = registry .select( blockEditorStore ) .getBlock( clientId ); + const existingOverrides = patternBlock.attributes?.content; function cloneBlocksAndRemoveBindings( blocks ) { return blocks.map( ( block ) => { @@ -101,6 +102,24 @@ export const convertSyncedPatternToStatic = metadata = { ...metadata }; delete metadata.id; delete metadata.bindings; + // Use overriden values of the pattern block if they exist. + if ( existingOverrides[ metadata.name ] ) { + // Iterate over each overriden attribute. + for ( const [ attributeName, value ] of Object.entries( + existingOverrides[ metadata.name ] + ) ) { + // Skip if the attribute does not exist in the block type. + if ( + ! getBlockType( block.name )?.attributes[ + attributeName + ] + ) { + continue; + } + // Update the block attribute with the override value. + block.attributes[ attributeName ] = value; + } + } } return cloneBlock( block, From 766a762cf2d7f1154d83f8f906246a3a0e2daa5a Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:17 +0200 Subject: [PATCH 14/44] Adapt pattern-overrides tests --- .../editor/various/pattern-overrides.spec.js | 60 ++++++++----------- 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 4dbe65436aa95..99fed5fcb3c18 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -148,6 +148,8 @@ test.describe( 'Pattern Overrides', () => { const paragraphs = patternBlocks.first().getByRole( 'document', { name: 'Block: Paragraph', } ); + // Ensure the first pattern is selected. + await patternBlocks.first().selectText(); await expect( paragraphs.first() ).not.toHaveAttribute( 'inert', 'true' @@ -164,6 +166,8 @@ test.describe( 'Pattern Overrides', () => { await paragraphs.first().selectText(); await page.keyboard.type( 'I would word it this way' ); + // Ensure the second pattern is selected. + await patternBlocks.last().selectText(); await patternBlocks .last() .getByRole( 'document', { @@ -417,7 +421,7 @@ test.describe( 'Pattern Overrides', () => { const postId = await editor.publishPost(); - // Check it renders correctly. + // Check the pattern has the correct attributes. await expect.poll( editor.getBlocks ).toMatchObject( [ { name: 'core/block', @@ -429,41 +433,22 @@ test.describe( 'Pattern Overrides', () => { }, }, }, - innerBlocks: [ - { - name: 'core/heading', - attributes: { content: 'Outer heading (edited)' }, - }, - { - name: 'core/block', - attributes: { - ref: innerPattern.id, - content: { - [ paragraphName ]: { - content: 'Inner paragraph (edited)', - }, - }, - }, - innerBlocks: [ - { - name: 'core/paragraph', - attributes: { - content: 'Inner paragraph (edited)', - }, - }, - ], - }, - ], + innerBlocks: [], }, ] ); - - await expect( - editor.canvas.getByRole( 'document', { - name: 'Block: Paragraph', - includeHidden: true, - } ), - 'The inner paragraph should not be editable' - ).toHaveAttribute( 'inert', 'true' ); + // Check it renders correctly. + const headingBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Heading', + includeHidden: true, + } ); + const paragraphBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Paragraph', + includeHidden: true, + } ); + await expect( headingBlock ).toHaveText( 'Outer heading (edited)' ); + await expect( headingBlock ).not.toHaveAttribute( 'inert', 'true' ); + await expect( paragraphBlock ).toHaveText( 'Inner paragraph (edited)' ); + await expect( paragraphBlock ).toHaveAttribute( 'inert', 'true' ); // Edit the outer pattern. await editor.selectBlocks( @@ -478,11 +463,16 @@ test.describe( 'Pattern Overrides', () => { .click(); // The inner paragraph should be editable in the pattern focus mode. + await editor.selectBlocks( + editor.canvas + .getByRole( 'document', { name: 'Block: Pattern' } ) + .first() + ); await expect( editor.canvas.getByRole( 'document', { name: 'Block: Paragraph', } ), - 'The inner paragraph should not be editable' + 'The inner paragraph should be editable' ).not.toHaveAttribute( 'inert', 'true' ); // Visit the post on the frontend. From 49286e137244ea7d6b57bf03cd01cb0cb264e347 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 15/44] Add workaround for pattern overrides --- .../src/hooks/use-bindings-attributes.js | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index aabb04343ffa8..76cd607029bf6 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -25,7 +25,7 @@ import { unlock } from '../lock-unlock'; const BLOCK_BINDINGS_ALLOWED_BLOCKS = { 'core/paragraph': [ 'content' ], 'core/heading': [ 'content' ], - 'core/image': [ 'url', 'title', 'alt' ], + 'core/image': [ 'id', 'url', 'title', 'alt' ], 'core/button': [ 'url', 'text', 'linkTarget' ], }; @@ -115,9 +115,25 @@ export const withBlockBindingSupport = createHigherOrderComponent( const keptAttributes = { ...nextAttributes }; + // Don't update non-bound attributes if block is connected to pattern overrides. + // Ad-hoc fix for pattern overrides until we figure out what users really want. + // This could be needed not only for pattern overrides but all sources. + if ( + Object.values( bindings ).some( + ( binding ) => + binding.source === 'core/pattern-overrides' + ) + ) { + for ( const attributeName in nextAttributes ) { + if ( ! bindings[ attributeName ] ) { + delete keptAttributes[ attributeName ]; + } + } + } + // Loop only over the updated attributes to avoid modifying the bound ones that haven't changed. for ( const [ attributeName, newValue ] of Object.entries( - nextAttributes + keptAttributes ) ) { if ( ! bindings[ attributeName ] || From ef29ab3caec7f81ec0d16f135f71b591b63710d3 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 16/44] Don't update non-bound attributes when using patterns --- .../src/hooks/use-bindings-attributes.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 76cd607029bf6..f0a4445368ec5 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -11,6 +11,7 @@ import { addFilter } from '@wordpress/hooks'; * Internal dependencies */ import { unlock } from '../lock-unlock'; +import { store as blockEditorStore } from '../store'; /** @typedef {import('@wordpress/compose').WPHigherOrderComponent} WPHigherOrderComponent */ /** @typedef {import('@wordpress/blocks').WPBlockSettings} WPBlockSettings */ @@ -124,9 +125,18 @@ export const withBlockBindingSupport = createHigherOrderComponent( binding.source === 'core/pattern-overrides' ) ) { - for ( const attributeName in nextAttributes ) { - if ( ! bindings[ attributeName ] ) { - delete keptAttributes[ attributeName ]; + // Don't update non-bound attribute only when using the pattern and not when editing the original one. + const { getBlockParents, getBlockName } = + registry.select( blockEditorStore ); + const parents = getBlockParents( clientId, true ); + const patternClientId = parents.find( + ( id ) => getBlockName( id ) === 'core/block' + ); + if ( patternClientId ) { + for ( const attributeName in nextAttributes ) { + if ( ! bindings[ attributeName ] ) { + delete keptAttributes[ attributeName ]; + } } } } From ba8b61866b2ff01495e644018806a918ef4faebc Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 17/44] Add test for blocks with a shared name --- .../editor/various/pattern-overrides.spec.js | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 99fed5fcb3c18..cafa3ce07b287 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -638,4 +638,148 @@ test.describe( 'Pattern Overrides', () => { } ) ).toBeHidden(); } ); + + test( 'create a pattern with synced blocks', async ( { + page, + admin, + editor, + } ) => { + let patternId; + const sharedName = 'Shared Name'; + + await test.step( 'create a pattern with synced blocks', async () => { + await admin.visitSiteEditor( { path: '/patterns' } ); + + await page + .getByRole( 'region', { name: 'Navigation' } ) + .getByRole( 'button', { name: 'Create pattern' } ) + .click(); + + await page + .getByRole( 'menu', { name: 'Create pattern' } ) + .getByRole( 'menuitem', { name: 'Create pattern' } ) + .click(); + + const createPatternDialog = page.getByRole( 'dialog', { + name: 'Create pattern', + } ); + await createPatternDialog + .getByRole( 'textbox', { name: 'Name' } ) + .fill( 'Pattern with synced values' ); + await createPatternDialog + .getByRole( 'checkbox', { name: 'Synced' } ) + .setChecked( true ); + await createPatternDialog + .getByRole( 'button', { name: 'Create' } ) + .click(); + + await editor.canvas.locator( 'body' ).click(); + + // Insert heading with shared name. + await editor.insertBlock( { + name: 'core/heading', + attributes: { + content: 'default content', + metadata: { + name: sharedName, + bindings: { + content: { + source: 'core/pattern-overrides', + }, + }, + }, + }, + } ); + + // Insert first paragraph with shared name. + await editor.insertBlock( { + name: 'core/paragraph', + attributes: { + content: 'default content', + metadata: { + name: sharedName, + bindings: { + content: { + source: 'core/pattern-overrides', + }, + }, + }, + }, + } ); + + // Insert second paragraph with shared name. + await editor.insertBlock( { + name: 'core/paragraph', + attributes: { + content: 'default content', + metadata: { + name: sharedName, + bindings: { + content: { + source: 'core/pattern-overrides', + }, + }, + }, + }, + } ); + + const headingBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Heading', + } ); + const firstParagraph = editor.canvas + .getByRole( 'document', { + name: 'Block: Paragraph', + } ) + .first(); + const secondParagraph = editor.canvas + .getByRole( 'document', { + name: 'Block: Paragraph', + } ) + .last(); + + await firstParagraph.fill( 'updated content' ); + await expect( headingBlock ).toHaveText( 'updated content' ); + await expect( firstParagraph ).toHaveText( 'updated content' ); + await expect( secondParagraph ).toHaveText( 'updated content' ); + + await page + .getByRole( 'region', { name: 'Editor top bar' } ) + .getByRole( 'button', { name: 'Save' } ) + .click(); + + await expect( + page.getByRole( 'button', { name: 'Dismiss this notice' } ) + ).toBeVisible(); + + patternId = new URL( page.url() ).searchParams.get( 'postId' ); + } ); + + await test.step( 'create a post and insert the pattern with synced values', async () => { + await admin.createNewPost(); + + await editor.insertBlock( { + name: 'core/block', + attributes: { ref: patternId }, + } ); + + const headingBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Heading', + } ); + const firstParagraph = editor.canvas + .getByRole( 'document', { + name: 'Block: Paragraph', + } ) + .first(); + const secondParagraph = editor.canvas + .getByRole( 'document', { + name: 'Block: Paragraph', + } ) + .last(); + + await firstParagraph.fill( 'overriden content' ); + await expect( headingBlock ).toHaveText( 'overriden content' ); + await expect( firstParagraph ).toHaveText( 'overriden content' ); + await expect( secondParagraph ).toHaveText( 'overriden content' ); + } ); + } ); } ); From e695fb430b61ebd8f6c46ccfaf63945907378272 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 18/44] Support button rel in bindings editor logic --- packages/block-editor/src/hooks/use-bindings-attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index f0a4445368ec5..5d3d7d18b4c11 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -27,7 +27,7 @@ const BLOCK_BINDINGS_ALLOWED_BLOCKS = { 'core/paragraph': [ 'content' ], 'core/heading': [ 'content' ], 'core/image': [ 'id', 'url', 'title', 'alt' ], - 'core/button': [ 'url', 'text', 'linkTarget' ], + 'core/button': [ 'url', 'text', 'linkTarget', 'rel' ], }; /** From d9d9fd63e9043a616359231ec70a66c8f8268f60 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 19/44] Solve undo/redo issues --- packages/block-library/src/block/edit.js | 5 ++++- .../src/components/reset-overrides-control.js | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 152d7915bf802..c941f0c8dffac 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -146,6 +146,7 @@ function ReusableBlockEdit( { ); const isMissing = hasResolved && ! record; + const { syncDerivedUpdates } = unlock( useDispatch( blockEditorStore ) ); const { setBlockEditingMode } = useDispatch( blockEditorStore ); const { @@ -232,7 +233,9 @@ function ReusableBlockEdit( { const resetContent = () => { if ( content ) { - setAttributes( { content: undefined } ); + syncDerivedUpdates( () => { + setAttributes( { content: undefined } ); + } ); } }; diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 38c4727e92601..b636e4d543c4d 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -8,11 +8,16 @@ import { import { ToolbarButton, ToolbarGroup } from '@wordpress/components'; import { useDispatch, useSelect } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; +/** + * Internal dependencies + */ +import { unlock } from '../lock-unlock'; const CONTENT = 'content'; export default function ResetOverridesControl( props ) { const name = props.attributes.metadata?.name; + const { syncDerivedUpdates } = unlock( useDispatch( blockEditorStore ) ); const { updateBlockAttributes } = useDispatch( blockEditorStore ); const { isOverriden, resetOverrides } = useSelect( @@ -34,11 +39,13 @@ export default function ResetOverridesControl( props ) { return { isOverriden: !! existingOverrides?.[ name ], resetOverrides: async () => { - updateBlockAttributes( patternClientId, { - [ CONTENT ]: { - ...existingOverrides, - [ name ]: undefined, - }, + syncDerivedUpdates( () => { + updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...existingOverrides, + [ name ]: undefined, + }, + } ); } ); }, }; From 4a4465fe82b5103ba6513b36affbebb2188b4743 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 20/44] Stop using syncDerivedUpdates --- packages/block-library/src/block/edit.js | 5 +---- .../src/components/reset-overrides-control.js | 17 +++++------------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index c941f0c8dffac..152d7915bf802 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -146,7 +146,6 @@ function ReusableBlockEdit( { ); const isMissing = hasResolved && ! record; - const { syncDerivedUpdates } = unlock( useDispatch( blockEditorStore ) ); const { setBlockEditingMode } = useDispatch( blockEditorStore ); const { @@ -233,9 +232,7 @@ function ReusableBlockEdit( { const resetContent = () => { if ( content ) { - syncDerivedUpdates( () => { - setAttributes( { content: undefined } ); - } ); + setAttributes( { content: undefined } ); } }; diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index b636e4d543c4d..38c4727e92601 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -8,16 +8,11 @@ import { import { ToolbarButton, ToolbarGroup } from '@wordpress/components'; import { useDispatch, useSelect } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; -/** - * Internal dependencies - */ -import { unlock } from '../lock-unlock'; const CONTENT = 'content'; export default function ResetOverridesControl( props ) { const name = props.attributes.metadata?.name; - const { syncDerivedUpdates } = unlock( useDispatch( blockEditorStore ) ); const { updateBlockAttributes } = useDispatch( blockEditorStore ); const { isOverriden, resetOverrides } = useSelect( @@ -39,13 +34,11 @@ export default function ResetOverridesControl( props ) { return { isOverriden: !! existingOverrides?.[ name ], resetOverrides: async () => { - syncDerivedUpdates( () => { - updateBlockAttributes( patternClientId, { - [ CONTENT ]: { - ...existingOverrides, - [ name ]: undefined, - }, - } ); + updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...existingOverrides, + [ name ]: undefined, + }, } ); }, }; From 1101667f32946fcdfb0855a5fe7b048beb97b8c7 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 21/44] Ensure change is marked as persistent in pattern overrides --- .../editor/src/bindings/pattern-overrides.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index c9727626463d9..2ddf1d584802e 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -63,17 +63,19 @@ export default { const blockName = currentBlockAttributes?.metadata?.name; const currentBindingValue = getBlockAttributes( patternClientId )?.[ CONTENT ]; - registry - .dispatch( blockEditorStore ) - .updateBlockAttributes( patternClientId, { - [ CONTENT ]: { - ...currentBindingValue, - [ blockName ]: { - ...currentBindingValue?.[ blockName ], - [ attributeName ]: value, - }, + const { updateBlockAttributes, __unstableMarkLastChangeAsPersistent } = + registry.dispatch( blockEditorStore ); + updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...currentBindingValue, + [ blockName ]: { + ...currentBindingValue?.[ blockName ], + [ attributeName ]: value, }, - } ); + }, + } ); + + __unstableMarkLastChangeAsPersistent(); }, lockAttributesEditing: false, }; From a95749f899d4e9cdff401fda7bc6c79e00d6819f Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 22/44] Try to solve undo/redo issues --- packages/block-library/src/block/edit.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 152d7915bf802..5bcf358cc8868 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -214,10 +214,11 @@ function ReusableBlockEdit( { ), } ); + // Use `blocks` variable until `innerBlocks` is populated, which has the proper clientIds. const innerBlocksProps = useInnerBlocksProps( blockProps, { templateLock: 'contentOnly', layout, - value: blocks, + value: innerBlocks.length > 0 ? innerBlocks : blocks, onInput, onChange, renderAppender: blocks?.length ? undefined : blocks.ButtonBlockAppender, From e2a1b8a591162ab49a6baf1e5b59036753e8ec15 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 23/44] Manage undefined attrs in pattern overrides --- packages/editor/src/bindings/pattern-overrides.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 2ddf1d584802e..4171d0c0b8eff 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -24,13 +24,11 @@ export default { ]?.[ attributeName ]; // If there is no pattern client ID, or it is not overwritten, return the default value. - if ( ! patternClientId || ! overridableValue ) { + if ( ! patternClientId || overridableValue === undefined ) { return currentBlockAttributes[ attributeName ]; } - return getBlockAttributes( patternClientId )?.[ CONTENT ]?.[ - currentBlockAttributes?.metadata?.name - ]?.[ attributeName ]; + return overridableValue === '' ? undefined : overridableValue; }, setValue( { registry, clientId, attributeName, value } ) { const { getBlockAttributes, getBlockParents, getBlockName, getBlocks } = @@ -70,7 +68,7 @@ export default { ...currentBindingValue, [ blockName ]: { ...currentBindingValue?.[ blockName ], - [ attributeName ]: value, + [ attributeName ]: value || '', }, }, } ); From 9d444a337ec478f4fdf3f6b3646abc55bd71388f Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Thu, 9 May 2024 13:06:18 +0200 Subject: [PATCH 24/44] Remove `content` pattern property if empty --- .../patterns/src/components/reset-overrides-control.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 38c4727e92601..299c6797b976c 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -34,11 +34,12 @@ export default function ResetOverridesControl( props ) { return { isOverriden: !! existingOverrides?.[ name ], resetOverrides: async () => { + // If all overrides are undefined, reset the whole content attribute. + const newObject = existingOverrides.hasOwnProperty( name ) + ? undefined + : { ...existingOverrides, [ name ]: undefined }; updateBlockAttributes( patternClientId, { - [ CONTENT ]: { - ...existingOverrides, - [ name ]: undefined, - }, + [ CONTENT ]: newObject, } ); }, }; From e28da536aac278d2f372019a49c5c0919d2d8c82 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 13 May 2024 12:27:19 +0900 Subject: [PATCH 25/44] Remove pattern-overrides specific code from `withBlockBindingSupport`, instead NOOP onChange and onInput Revert "Don't update non-bound attributes when using patterns" This reverts commit ef29ab3caec7f81ec0d16f135f71b591b63710d3. NOOP onChange and onInput in the pattern block - the original entity should never be updated via this code --- .../src/hooks/use-bindings-attributes.js | 29 ++----------------- packages/block-library/src/block/edit.js | 14 ++++----- 2 files changed, 9 insertions(+), 34 deletions(-) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 5d3d7d18b4c11..4bc50efdddf3c 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -11,7 +11,6 @@ import { addFilter } from '@wordpress/hooks'; * Internal dependencies */ import { unlock } from '../lock-unlock'; -import { store as blockEditorStore } from '../store'; /** @typedef {import('@wordpress/compose').WPHigherOrderComponent} WPHigherOrderComponent */ /** @typedef {import('@wordpress/blocks').WPBlockSettings} WPBlockSettings */ @@ -111,36 +110,12 @@ export const withBlockBindingSupport = createHigherOrderComponent( ( nextAttributes ) => { registry.batch( () => { if ( ! bindings ) { - return setAttributes( nextAttributes ); + setAttributes( nextAttributes ); + return; } const keptAttributes = { ...nextAttributes }; - // Don't update non-bound attributes if block is connected to pattern overrides. - // Ad-hoc fix for pattern overrides until we figure out what users really want. - // This could be needed not only for pattern overrides but all sources. - if ( - Object.values( bindings ).some( - ( binding ) => - binding.source === 'core/pattern-overrides' - ) - ) { - // Don't update non-bound attribute only when using the pattern and not when editing the original one. - const { getBlockParents, getBlockName } = - registry.select( blockEditorStore ); - const parents = getBlockParents( clientId, true ); - const patternClientId = parents.find( - ( id ) => getBlockName( id ) === 'core/block' - ); - if ( patternClientId ) { - for ( const attributeName in nextAttributes ) { - if ( ! bindings[ attributeName ] ) { - delete keptAttributes[ attributeName ]; - } - } - } - } - // Loop only over the updated attributes to avoid modifying the bound ones that haven't changed. for ( const [ attributeName, newValue ] of Object.entries( keptAttributes diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 5bcf358cc8868..6166a28cc2728 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -109,6 +109,8 @@ function RecursionWarning() { ); } +const NOOP = () => {}; + // Wrap the main Edit function for the pattern block with a recursion wrapper // that allows short-circuiting rendering as early as possible, before any // of the other effects in the block edit have run. @@ -139,11 +141,9 @@ function ReusableBlockEdit( { 'wp_block', ref ); - const [ blocks, onInput, onChange ] = useEntityBlockEditor( - 'postType', - 'wp_block', - { id: ref } - ); + const [ blocks ] = useEntityBlockEditor( 'postType', 'wp_block', { + id: ref, + } ); const isMissing = hasResolved && ! record; const { setBlockEditingMode } = useDispatch( blockEditorStore ); @@ -219,8 +219,8 @@ function ReusableBlockEdit( { templateLock: 'contentOnly', layout, value: innerBlocks.length > 0 ? innerBlocks : blocks, - onInput, - onChange, + onInput: NOOP, + onChange: NOOP, renderAppender: blocks?.length ? undefined : blocks.ButtonBlockAppender, } ); From ae534d86070bd77ca61cf252b1261cecea524419 Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 13 May 2024 13:31:47 +0900 Subject: [PATCH 26/44] Fix persistent issues with setValues API --- .../src/hooks/use-bindings-attributes.js | 43 +++++++++++++++---- packages/blocks/src/store/private-actions.js | 1 + packages/blocks/src/store/reducer.js | 1 + .../editor/src/bindings/pattern-overrides.js | 32 +++++++------- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 4bc50efdddf3c..b7a4ca0379dd1 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -115,6 +115,7 @@ export const withBlockBindingSupport = createHigherOrderComponent( } const keptAttributes = { ...nextAttributes }; + const updatesBySource = new Map(); // Loop only over the updated attributes to avoid modifying the bound ones that haven't changed. for ( const [ attributeName, newValue ] of Object.entries( @@ -129,20 +130,46 @@ export const withBlockBindingSupport = createHigherOrderComponent( const source = sources[ bindings[ attributeName ].source ]; - if ( ! source?.setValue ) { + if ( ! source?.setValue && ! source?.setValues ) { continue; } - source.setValue( { - registry, - context, - clientId, - attributeName, - args: bindings[ attributeName ].args, - value: newValue, + updatesBySource.set( source, { + ...updatesBySource.get( source ), + [ attributeName ]: newValue, } ); delete keptAttributes[ attributeName ]; } + if ( updatesBySource.size ) { + for ( const [ + source, + attributes, + ] of updatesBySource ) { + if ( source.setValues ) { + source.setValues( { + registry, + context, + clientId, + attributes, + } ); + } else { + for ( const [ + attributeName, + value, + ] of Object.entries( attributes ) ) { + source.setValue( { + registry, + context, + clientId, + attributeName, + args: bindings[ attributeName ].args, + value, + } ); + } + } + } + } + if ( Object.keys( keptAttributes ).length ) { setAttributes( keptAttributes ); } diff --git a/packages/blocks/src/store/private-actions.js b/packages/blocks/src/store/private-actions.js index 1ef9c3614922e..a47d9aacab37a 100644 --- a/packages/blocks/src/store/private-actions.js +++ b/packages/blocks/src/store/private-actions.js @@ -53,6 +53,7 @@ export function registerBlockBindingsSource( source ) { sourceLabel: source.label, getValue: source.getValue, setValue: source.setValue, + setValues: source.setValues, getPlaceholder: source.getPlaceholder, lockAttributesEditing: source.lockAttributesEditing, }; diff --git a/packages/blocks/src/store/reducer.js b/packages/blocks/src/store/reducer.js index 7a3a866485e4a..582c491f83b6d 100644 --- a/packages/blocks/src/store/reducer.js +++ b/packages/blocks/src/store/reducer.js @@ -378,6 +378,7 @@ export function blockBindingsSources( state = {}, action ) { label: action.sourceLabel, getValue: action.getValue, setValue: action.setValue, + setValues: action.setValues, getPlaceholder: action.getPlaceholder, lockAttributesEditing: action.lockAttributesEditing ?? true, }, diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 4171d0c0b8eff..ccc2e86b17789 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -30,7 +30,7 @@ export default { return overridableValue === '' ? undefined : overridableValue; }, - setValue( { registry, clientId, attributeName, value } ) { + setValues( { registry, clientId, attributes } ) { const { getBlockAttributes, getBlockParents, getBlockName, getBlocks } = registry.select( blockEditorStore ); const currentBlockAttributes = getBlockAttributes( clientId ); @@ -46,9 +46,10 @@ export default { if ( block.attributes?.metadata?.name === blockName ) { registry .dispatch( blockEditorStore ) - .updateBlockAttributes( block.clientId, { - [ attributeName ]: value, - } ); + .updateBlockAttributes( + block.clientId, + attributes + ); } syncBlocksWithSameName( block.innerBlocks ); } @@ -57,23 +58,20 @@ export default { syncBlocksWithSameName( getBlocks() ); return; } - const blockName = currentBlockAttributes?.metadata?.name; const currentBindingValue = getBlockAttributes( patternClientId )?.[ CONTENT ]; - const { updateBlockAttributes, __unstableMarkLastChangeAsPersistent } = - registry.dispatch( blockEditorStore ); - updateBlockAttributes( patternClientId, { - [ CONTENT ]: { - ...currentBindingValue, - [ blockName ]: { - ...currentBindingValue?.[ blockName ], - [ attributeName ]: value || '', + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...currentBindingValue, + [ blockName ]: { + ...currentBindingValue?.[ blockName ], + ...attributes, + }, }, - }, - } ); - - __unstableMarkLastChangeAsPersistent(); + } ); }, lockAttributesEditing: false, }; From 4ec73156243ea5de6fd15e8c8aff545c323fb4b6 Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 13 May 2024 14:48:28 +0900 Subject: [PATCH 27/44] Fall back to '' --- packages/editor/src/bindings/pattern-overrides.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index ccc2e86b17789..57fa2bce6b4e5 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -68,7 +68,13 @@ export default { ...currentBindingValue, [ blockName ]: { ...currentBindingValue?.[ blockName ], - ...attributes, + ...Object.entries( attributes ).reduce( + ( acc, [ key, value ] ) => { + acc[ key ] = value === undefined ? '' : value; + return acc; + }, + {} + ), }, }, } ); From 3a7311d12be706a2bd6d611812f77f179edf85bc Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 13 May 2024 15:32:21 +0900 Subject: [PATCH 28/44] Make sure any previous changes are persisted before resetting. --- packages/block-library/src/block/edit.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 6166a28cc2728..f3193b4bd3514 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -146,7 +146,8 @@ function ReusableBlockEdit( { } ); const isMissing = hasResolved && ! record; - const { setBlockEditingMode } = useDispatch( blockEditorStore ); + const { setBlockEditingMode, __unstableMarkLastChangeAsPersistent } = + useDispatch( blockEditorStore ); const { innerBlocks, @@ -233,6 +234,8 @@ function ReusableBlockEdit( { const resetContent = () => { if ( content ) { + // Make sure any previous changes are persisted before resetting. + __unstableMarkLastChangeAsPersistent(); setAttributes( { content: undefined } ); } }; From e013f6dd47d4fc3be774d7e600886634439b07db Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Mon, 13 May 2024 15:55:52 +0900 Subject: [PATCH 29/44] Add undo/redo test for resetting --- .../editor/various/pattern-overrides.spec.js | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index cafa3ce07b287..f75d84924d41a 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -577,6 +577,70 @@ test.describe( 'Pattern Overrides', () => { await expect( resetButton ).toBeDisabled(); } ); + // A Undo/Redo bug found when implementing and fixing https://github.com/WordPress/gutenberg/pull/60721. + // This could be merged into an existing test after we fully test it. + test( 'resets overrides immediately should not break undo/redo', async ( { + page, + admin, + requestUtils, + editor, + } ) => { + const paragraphName = 'Editable paragraph'; + const { id } = await requestUtils.createBlock( { + title: 'Pattern', + content: ` +

Paragraph

+`, + status: 'publish', + } ); + + await admin.createNewPost(); + + await editor.insertBlock( { + name: 'core/block', + attributes: { ref: id }, + } ); + + const patternBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Pattern', + } ); + const paragraphBlock = patternBlock.getByRole( 'document', { + name: 'Block: Paragraph', + } ); + const resetButton = page + .getByRole( 'toolbar', { name: 'Block tools' } ) + .getByRole( 'button', { name: 'Reset' } ); + const documentTools = page.getByRole( 'toolbar', { + name: 'Document tools', + } ); + const undoButton = documentTools.getByRole( 'button', { + name: 'Undo', + } ); + const redoButton = documentTools.getByRole( 'button', { + name: 'Redo', + } ); + + // Make an edit to the paragraph. + await editor.canvas + .getByRole( 'document', { name: 'Block: Paragraph' } ) + .click(); + await page.keyboard.type( '*' ); + await expect( paragraphBlock ).toHaveText( 'Paragraph*' ); + + // Reset immediately after making the edit. + await editor.selectBlocks( paragraphBlock ); + await editor.showBlockToolbar(); + await expect( resetButton ).toBeEnabled(); + await resetButton.click(); + await expect( paragraphBlock ).toHaveText( 'Paragraph' ); + + // Undo/Redo should work + await undoButton.click(); + await expect( paragraphBlock ).toHaveText( 'Heading*' ); + await redoButton.click(); + await expect( paragraphBlock ).toHaveText( 'Heading' ); + } ); + // Fix https://github.com/WordPress/gutenberg/issues/58708. test( 'overridden empty images should not have upload button', async ( { page, From 950aab94641a101a090409b0f98753bf34d789f9 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Mon, 13 May 2024 16:02:35 +0900 Subject: [PATCH 30/44] =?UTF-8?q?Fix=20test...=20=F0=9F=A4=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/e2e/specs/editor/various/pattern-overrides.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index f75d84924d41a..18c9ab80e1b0d 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -636,9 +636,9 @@ test.describe( 'Pattern Overrides', () => { // Undo/Redo should work await undoButton.click(); - await expect( paragraphBlock ).toHaveText( 'Heading*' ); + await expect( paragraphBlock ).toHaveText( 'Paragraph*' ); await redoButton.click(); - await expect( paragraphBlock ).toHaveText( 'Heading' ); + await expect( paragraphBlock ).toHaveText( 'Paragraph' ); } ); // Fix https://github.com/WordPress/gutenberg/issues/58708. From d99ecb7ab4e13db86f307a79774d8f4c8d95700c Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 13 May 2024 16:28:57 +0900 Subject: [PATCH 31/44] Fix remaining undo test --- .../src/components/reset-overrides-control.js | 62 ++++++++++++------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 299c6797b976c..9d90461196e51 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -6,19 +6,18 @@ import { BlockControls, } from '@wordpress/block-editor'; import { ToolbarButton, ToolbarGroup } from '@wordpress/components'; -import { useDispatch, useSelect } from '@wordpress/data'; +import { useRegistry, useSelect } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; const CONTENT = 'content'; export default function ResetOverridesControl( props ) { const name = props.attributes.metadata?.name; - const { updateBlockAttributes } = useDispatch( blockEditorStore ); - - const { isOverriden, resetOverrides } = useSelect( + const registry = useRegistry(); + const isOverriden = useSelect( ( select ) => { if ( ! name ) { - return undefined; + return; } const { getBlockAttributes, getBlockParents, getBlockName } = @@ -28,30 +27,51 @@ export default function ResetOverridesControl( props ) { ( id ) => getBlockName( id ) === 'core/block' ); - const patternAttributes = getBlockAttributes( patternClientId ); - const existingOverrides = patternAttributes?.[ CONTENT ]; - - return { - isOverriden: !! existingOverrides?.[ name ], - resetOverrides: async () => { - // If all overrides are undefined, reset the whole content attribute. - const newObject = existingOverrides.hasOwnProperty( name ) - ? undefined - : { ...existingOverrides, [ name ]: undefined }; - updateBlockAttributes( patternClientId, { - [ CONTENT ]: newObject, - } ); - }, - }; + if ( ! patternClientId ) { + return; + } + + const overrides = getBlockAttributes( patternClientId )[ CONTENT ]; + + if ( ! overrides ) { + return; + } + + return !! overrides[ name ]; }, [ props.clientId, name ] ); + function onClick() { + const { getBlockAttributes, getBlockParents, getBlockName } = + registry.select( blockEditorStore ); + const parents = getBlockParents( props.clientId, true ); + const patternClientId = parents.find( + ( id ) => getBlockName( id ) === 'core/block' + ); + + if ( ! patternClientId ) { + return; + } + + const overrides = getBlockAttributes( patternClientId )[ CONTENT ]; + // If all overrides are undefined, reset the whole content attribute. + const newObject = overrides.hasOwnProperty( name ) + ? undefined + : { ...overrides, [ name ]: undefined }; + const { updateBlockAttributes, __unstableMarkLastChangeAsPersistent } = + registry.dispatch( blockEditorStore ); + __unstableMarkLastChangeAsPersistent(); + updateBlockAttributes( patternClientId, { + [ CONTENT ]: newObject, + } ); + } + return ( From 9ec9bbad9b3368a1a09989a5f02e8ff40ad02afc Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 13 May 2024 16:34:44 +0900 Subject: [PATCH 32/44] Fix individual reset --- .../src/components/reset-overrides-control.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 9d90461196e51..bf67dc2dfe220 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -55,15 +55,16 @@ export default function ResetOverridesControl( props ) { } const overrides = getBlockAttributes( patternClientId )[ CONTENT ]; - // If all overrides are undefined, reset the whole content attribute. - const newObject = overrides.hasOwnProperty( name ) - ? undefined - : { ...overrides, [ name ]: undefined }; + + if ( ! overrides.hasOwnProperty( name ) ) { + return; + } + const { updateBlockAttributes, __unstableMarkLastChangeAsPersistent } = registry.dispatch( blockEditorStore ); __unstableMarkLastChangeAsPersistent(); updateBlockAttributes( patternClientId, { - [ CONTENT ]: newObject, + [ CONTENT ]: { ...overrides, [ name ]: undefined }, } ); } From 7d72a346978cc61fa9e5e101f2301829debc89fd Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Mon, 13 May 2024 16:54:51 +0900 Subject: [PATCH 33/44] Add test for undoing loses the focus --- .../editor/various/pattern-overrides.spec.js | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 18c9ab80e1b0d..c5bca5088dad2 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -846,4 +846,61 @@ test.describe( 'Pattern Overrides', () => { await expect( secondParagraph ).toHaveText( 'overriden content' ); } ); } ); + + // A Undo/Redo bug found when implementing and fixing https://github.com/WordPress/gutenberg/pull/60721. + // This could be merged into an existing test after we fully test it. + test( 'undo/redo should not lose focuses', async ( { + page, + admin, + requestUtils, + editor, + pageUtils, + } ) => { + const paragraphName = 'Editable paragraph'; + const { id } = await requestUtils.createBlock( { + title: 'Pattern', + content: ` +

Paragraph

+`, + status: 'publish', + } ); + + await admin.createNewPost(); + + await editor.insertBlock( { + name: 'core/block', + attributes: { ref: id }, + } ); + + const patternBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Pattern', + } ); + const paragraphBlock = patternBlock.getByRole( 'document', { + name: 'Block: Paragraph', + } ); + + // Make an edit to the paragraph. + await paragraphBlock.focus(); + // Move the text cursor to the end. + await page.keyboard.press( 'End' ); + await page.keyboard.type( '123', { delay: 1000 } ); + + const getSelectionStart = async () => { + return await editor.canvas.locator( 'body' ).evaluate( + // Disable reason: It's a test in a controlled environment. + // eslint-disable-next-line @wordpress/no-unguarded-get-range-at + () => document.getSelection().getRangeAt( 0 ).startOffset + ); + }; + + await expect.poll( getSelectionStart ).toBe( 12 ); + await pageUtils.pressKeys( 'primary+z' ); + await expect( paragraphBlock ).toHaveText( 'Paragraph12' ); + await expect.poll( getSelectionStart ).toBe( 11 ); + await pageUtils.pressKeys( 'primary+z' ); + await expect( paragraphBlock ).toHaveText( 'Paragraph1' ); + await expect.poll( getSelectionStart ).toBe( 10 ); + await pageUtils.pressKeys( 'primary+z' ); + await expect( paragraphBlock ).toHaveText( 'Paragraph' ); + } ); } ); From 4ebdb09b723f1e265d9a6865b7beaa620c23ebe5 Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 13 May 2024 17:16:19 +0900 Subject: [PATCH 34/44] Fix reset button test --- .../src/components/reset-overrides-control.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index bf67dc2dfe220..531c1420918d8 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -37,7 +37,7 @@ export default function ResetOverridesControl( props ) { return; } - return !! overrides[ name ]; + return overrides.hasOwnProperty( name ); }, [ props.clientId, name ] ); @@ -63,8 +63,16 @@ export default function ResetOverridesControl( props ) { const { updateBlockAttributes, __unstableMarkLastChangeAsPersistent } = registry.dispatch( blockEditorStore ); __unstableMarkLastChangeAsPersistent(); + + let newOverrides = { ...overrides }; + delete newOverrides[ name ]; + + if ( ! Object.keys( newOverrides ).length ) { + newOverrides = undefined; + } + updateBlockAttributes( patternClientId, { - [ CONTENT ]: { ...overrides, [ name ]: undefined }, + [ CONTENT ]: newOverrides, } ); } From 44b51ddb0087f2a6ef233cf2c00b0bd69cdc0cb0 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Mon, 13 May 2024 15:52:53 +0200 Subject: [PATCH 35/44] Simplify pattern synced blocks test --- .../editor/various/pattern-overrides.spec.js | 109 +++++------------- 1 file changed, 31 insertions(+), 78 deletions(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index c5bca5088dad2..955b58ec7a778 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -703,88 +703,34 @@ test.describe( 'Pattern Overrides', () => { ).toBeHidden(); } ); - test( 'create a pattern with synced blocks', async ( { + test( 'blocks with the same name should be synced', async ( { page, admin, + requestUtils, editor, } ) => { let patternId; const sharedName = 'Shared Name'; - await test.step( 'create a pattern with synced blocks', async () => { - await admin.visitSiteEditor( { path: '/patterns' } ); - - await page - .getByRole( 'region', { name: 'Navigation' } ) - .getByRole( 'button', { name: 'Create pattern' } ) - .click(); - - await page - .getByRole( 'menu', { name: 'Create pattern' } ) - .getByRole( 'menuitem', { name: 'Create pattern' } ) - .click(); - - const createPatternDialog = page.getByRole( 'dialog', { - name: 'Create pattern', + await test.step( 'create a pattern with synced blocks with the same name', async () => { + const { id } = await requestUtils.createBlock( { + title: 'Blocks with the same name', + content: ` +

default name

+ + +

default content

+ + +

default content

+ `, + status: 'publish', } ); - await createPatternDialog - .getByRole( 'textbox', { name: 'Name' } ) - .fill( 'Pattern with synced values' ); - await createPatternDialog - .getByRole( 'checkbox', { name: 'Synced' } ) - .setChecked( true ); - await createPatternDialog - .getByRole( 'button', { name: 'Create' } ) - .click(); - - await editor.canvas.locator( 'body' ).click(); - - // Insert heading with shared name. - await editor.insertBlock( { - name: 'core/heading', - attributes: { - content: 'default content', - metadata: { - name: sharedName, - bindings: { - content: { - source: 'core/pattern-overrides', - }, - }, - }, - }, - } ); - - // Insert first paragraph with shared name. - await editor.insertBlock( { - name: 'core/paragraph', - attributes: { - content: 'default content', - metadata: { - name: sharedName, - bindings: { - content: { - source: 'core/pattern-overrides', - }, - }, - }, - }, - } ); - - // Insert second paragraph with shared name. - await editor.insertBlock( { - name: 'core/paragraph', - attributes: { - content: 'default content', - metadata: { - name: sharedName, - bindings: { - content: { - source: 'core/pattern-overrides', - }, - }, - }, - }, + await admin.visitSiteEditor( { + postId: id, + postType: 'wp_block', + categoryType: 'pattern', + canvas: 'edit', } ); const headingBlock = editor.canvas.getByRole( 'document', { @@ -801,10 +747,17 @@ test.describe( 'Pattern Overrides', () => { } ) .last(); - await firstParagraph.fill( 'updated content' ); - await expect( headingBlock ).toHaveText( 'updated content' ); - await expect( firstParagraph ).toHaveText( 'updated content' ); - await expect( secondParagraph ).toHaveText( 'updated content' ); + // Update the content of one of the blocks. + await headingBlock.fill( 'updated content' ); + + // Check that every content has been updated. + for ( const block of [ + headingBlock, + firstParagraph, + secondParagraph, + ] ) { + await expect( block ).toHaveText( 'updated content' ); + } await page .getByRole( 'region', { name: 'Editor top bar' } ) From 5975f745b97b91f8fea8ae323a927b3c54c4cea5 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Mon, 13 May 2024 15:55:25 +0200 Subject: [PATCH 36/44] Remove focus test --- .../editor/various/pattern-overrides.spec.js | 57 ------------------- 1 file changed, 57 deletions(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 955b58ec7a778..4d3b8a97b2373 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -799,61 +799,4 @@ test.describe( 'Pattern Overrides', () => { await expect( secondParagraph ).toHaveText( 'overriden content' ); } ); } ); - - // A Undo/Redo bug found when implementing and fixing https://github.com/WordPress/gutenberg/pull/60721. - // This could be merged into an existing test after we fully test it. - test( 'undo/redo should not lose focuses', async ( { - page, - admin, - requestUtils, - editor, - pageUtils, - } ) => { - const paragraphName = 'Editable paragraph'; - const { id } = await requestUtils.createBlock( { - title: 'Pattern', - content: ` -

Paragraph

-`, - status: 'publish', - } ); - - await admin.createNewPost(); - - await editor.insertBlock( { - name: 'core/block', - attributes: { ref: id }, - } ); - - const patternBlock = editor.canvas.getByRole( 'document', { - name: 'Block: Pattern', - } ); - const paragraphBlock = patternBlock.getByRole( 'document', { - name: 'Block: Paragraph', - } ); - - // Make an edit to the paragraph. - await paragraphBlock.focus(); - // Move the text cursor to the end. - await page.keyboard.press( 'End' ); - await page.keyboard.type( '123', { delay: 1000 } ); - - const getSelectionStart = async () => { - return await editor.canvas.locator( 'body' ).evaluate( - // Disable reason: It's a test in a controlled environment. - // eslint-disable-next-line @wordpress/no-unguarded-get-range-at - () => document.getSelection().getRangeAt( 0 ).startOffset - ); - }; - - await expect.poll( getSelectionStart ).toBe( 12 ); - await pageUtils.pressKeys( 'primary+z' ); - await expect( paragraphBlock ).toHaveText( 'Paragraph12' ); - await expect.poll( getSelectionStart ).toBe( 11 ); - await pageUtils.pressKeys( 'primary+z' ); - await expect( paragraphBlock ).toHaveText( 'Paragraph1' ); - await expect.poll( getSelectionStart ).toBe( 10 ); - await pageUtils.pressKeys( 'primary+z' ); - await expect( paragraphBlock ).toHaveText( 'Paragraph' ); - } ); } ); From 10fa4e8df34e2c5ba8b719227f5f0a3c9959c950 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 09:10:52 +0200 Subject: [PATCH 37/44] Use getBlockParentsByBlockName --- .../editor/src/bindings/pattern-overrides.js | 18 ++++++++++-------- .../src/components/reset-overrides-control.js | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 57fa2bce6b4e5..1c0b4dad93c3a 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -10,12 +10,13 @@ export default { name: 'core/pattern-overrides', label: _x( 'Pattern Overrides', 'block bindings source' ), getValue( { registry, clientId, attributeName } ) { - const { getBlockAttributes, getBlockParents, getBlockName } = + const { getBlockAttributes, getBlockParentsByBlockName } = registry.select( blockEditorStore ); const currentBlockAttributes = getBlockAttributes( clientId ); - const parents = getBlockParents( clientId, true ); - const patternClientId = parents.find( - ( id ) => getBlockName( id ) === 'core/block' + const [ patternClientId ] = getBlockParentsByBlockName( + clientId, + 'core/block', + true ); const overridableValue = @@ -31,12 +32,13 @@ export default { return overridableValue === '' ? undefined : overridableValue; }, setValues( { registry, clientId, attributes } ) { - const { getBlockAttributes, getBlockParents, getBlockName, getBlocks } = + const { getBlockAttributes, getBlockParentsByBlockName, getBlocks } = registry.select( blockEditorStore ); const currentBlockAttributes = getBlockAttributes( clientId ); - const parents = getBlockParents( clientId, true ); - const patternClientId = parents.find( - ( id ) => getBlockName( id ) === 'core/block' + const [ patternClientId ] = getBlockParentsByBlockName( + clientId, + 'core/block', + true ); // If there is no pattern client ID, sync blocks with the same name and same attributes. if ( ! patternClientId ) { diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 531c1420918d8..11f282481a3ac 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -20,11 +20,12 @@ export default function ResetOverridesControl( props ) { return; } - const { getBlockAttributes, getBlockParents, getBlockName } = + const { getBlockAttributes, getBlockParentsByBlockName } = select( blockEditorStore ); - const parents = getBlockParents( props.clientId, true ); - const patternClientId = parents.find( - ( id ) => getBlockName( id ) === 'core/block' + const [ patternClientId ] = getBlockParentsByBlockName( + props.clientId, + 'core/block', + true ); if ( ! patternClientId ) { @@ -43,11 +44,12 @@ export default function ResetOverridesControl( props ) { ); function onClick() { - const { getBlockAttributes, getBlockParents, getBlockName } = + const { getBlockAttributes, getBlockParentsByBlockName } = registry.select( blockEditorStore ); - const parents = getBlockParents( props.clientId, true ); - const patternClientId = parents.find( - ( id ) => getBlockName( id ) === 'core/block' + const [ patternClientId ] = getBlockParentsByBlockName( + props.clientId, + 'core/block', + true ); if ( ! patternClientId ) { From 3cfdd447d4328b4704a7bfeada9af205fa0a799f Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 09:24:05 +0200 Subject: [PATCH 38/44] Add early return if blockName doesn't exist --- packages/editor/src/bindings/pattern-overrides.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 1c0b4dad93c3a..5b631a10527ef 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -35,14 +35,19 @@ export default { const { getBlockAttributes, getBlockParentsByBlockName, getBlocks } = registry.select( blockEditorStore ); const currentBlockAttributes = getBlockAttributes( clientId ); + const blockName = currentBlockAttributes?.metadata?.name; + if ( ! blockName ) { + return; + } + const [ patternClientId ] = getBlockParentsByBlockName( clientId, 'core/block', true ); + // If there is no pattern client ID, sync blocks with the same name and same attributes. if ( ! patternClientId ) { - const blockName = currentBlockAttributes?.metadata?.name; const syncBlocksWithSameName = ( blocks ) => { for ( const block of blocks ) { if ( block.attributes?.metadata?.name === blockName ) { @@ -60,7 +65,6 @@ export default { syncBlocksWithSameName( getBlocks() ); return; } - const blockName = currentBlockAttributes?.metadata?.name; const currentBindingValue = getBlockAttributes( patternClientId )?.[ CONTENT ]; registry From 8c5f5748fd9b88355b74d8434e9f0764f5c88639 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 09:30:29 +0200 Subject: [PATCH 39/44] Add comment for undefined values --- packages/editor/src/bindings/pattern-overrides.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/editor/src/bindings/pattern-overrides.js b/packages/editor/src/bindings/pattern-overrides.js index 5b631a10527ef..7554da26481e3 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -76,6 +76,10 @@ export default { ...currentBindingValue?.[ blockName ], ...Object.entries( attributes ).reduce( ( acc, [ key, value ] ) => { + // TODO: We need a way to represent `undefined` in the serialized overrides. + // Also see: https://github.com/WordPress/gutenberg/pull/57249#discussion_r1452987871 + // We use an empty string to represent undefined for now until + // we support a richer format for overrides and the block bindings API. acc[ key ] = value === undefined ? '' : value; return acc; }, From 38664465f22cf5015ccf5da5f496299a536b41bf Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 09:30:49 +0200 Subject: [PATCH 40/44] Remove comment typo --- test/e2e/specs/editor/various/patterns.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js index 8e598938bd0b7..c8ce445facb3a 100644 --- a/test/e2e/specs/editor/various/patterns.spec.js +++ b/test/e2e/specs/editor/various/patterns.spec.js @@ -381,7 +381,7 @@ test.describe( 'Synced pattern', () => { attributes: { ref: id }, } ); - // // Wait until the pattern is created and inserted. + // Wait until the pattern is created and inserted. await editor.canvas.locator( 'text=Hello there!' ).waitFor(); const [ syncedPattern ] = await editor.getBlocks(); From 33bfb866f3d7995bab5218f72783bf3d0262b003 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 09:40:53 +0200 Subject: [PATCH 41/44] Remove unnecessary includeHidden from tests --- test/e2e/specs/editor/various/pattern-overrides.spec.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 4d3b8a97b2373..f252c670d7b07 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -439,11 +439,9 @@ test.describe( 'Pattern Overrides', () => { // Check it renders correctly. const headingBlock = editor.canvas.getByRole( 'document', { name: 'Block: Heading', - includeHidden: true, } ); const paragraphBlock = editor.canvas.getByRole( 'document', { name: 'Block: Paragraph', - includeHidden: true, } ); await expect( headingBlock ).toHaveText( 'Outer heading (edited)' ); await expect( headingBlock ).not.toHaveAttribute( 'inert', 'true' ); From d80aba74b6f9f4eb55cc0209e4e8f55e9850b72b Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 10:32:03 +0200 Subject: [PATCH 42/44] Check the frontend in patterns test --- .../e2e/specs/editor/various/patterns.spec.js | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js index c8ce445facb3a..4937368b5503e 100644 --- a/test/e2e/specs/editor/various/patterns.spec.js +++ b/test/e2e/specs/editor/various/patterns.spec.js @@ -128,7 +128,10 @@ test.describe( 'Synced pattern', () => { } ) => { await editor.insertBlock( { name: 'core/paragraph', - attributes: { content: 'A useful paragraph to reuse' }, + attributes: { + anchor: 'reused-paragraph', + content: 'A useful paragraph to reuse', + }, } ); // Create a synced pattern from the paragraph block. @@ -157,28 +160,18 @@ test.describe( 'Synced pattern', () => { .getByRole( 'button', { name: 'Create' } ) .click(); - // Wait until the pattern is inserted. - await editor.canvas - .getByRole( 'document', { - name: 'Block: Pattern', - } ) - .waitFor(); - - const [ syncedPattern ] = await editor.getBlocks(); - const patternRecord = await getPatternRecord( - page, - syncedPattern?.attributes?.ref - ); - - expect( patternRecord?.content ).toBe( - '\n

A useful paragraph to reuse

\n' - ); - + // Check the pattern is focused. const patternBlock = editor.canvas.getByRole( 'document', { name: 'Block: Pattern', } ); await expect( patternBlock ).toBeFocused(); + // Check that only the pattern block is present. + const existingBlocks = await editor.getBlocks(); + expect( + existingBlocks.every( ( block ) => block.name === 'core/block' ) + ).toBe( true ); + // Check that the new pattern is available in the inserter. await page.getByLabel( 'Toggle block inserter' ).click(); await page @@ -195,27 +188,26 @@ test.describe( 'Synced pattern', () => { const [ firstSyncedPattern, secondSyncedPattern ] = await editor.getBlocks(); - const firstSyncedPatternRecord = await getPatternRecord( - page, - firstSyncedPattern?.attributes?.ref + // Check they are both patterns. + expect( firstSyncedPattern.name ).toBe( 'core/block' ); + expect( secondSyncedPattern.name ).toBe( 'core/block' ); + // Check they have the same ref. + expect( firstSyncedPattern.attributes.ref ).toEqual( + secondSyncedPattern.attributes.ref ); - const secondSyncedPatternRecord = await getPatternRecord( - page, - secondSyncedPattern?.attributes?.ref - ); - // Check first pattern is "My synced pattern". - expect( firstSyncedPatternRecord?.title ).toEqual( - 'My synced pattern' - ); - expect( firstSyncedPatternRecord?.content ).toEqual( - '\n

A useful paragraph to reuse

\n' - ); - // Check second pattern is "My synced pattern". - expect( secondSyncedPatternRecord?.title ).toEqual( - 'My synced pattern' + + // Check that the frontend shows the content of the pattern. + const postId = await editor.publishPost(); + await page.goto( `/?p=${ postId }` ); + const [ firstParagraph, secondParagraph ] = await page + .locator( '#reused-paragraph' ) + .all(); + + await expect( firstParagraph ).toHaveText( + 'A useful paragraph to reuse' ); - expect( secondSyncedPatternRecord?.content ).toEqual( - '\n

A useful paragraph to reuse

\n' + await expect( secondParagraph ).toHaveText( + 'A useful paragraph to reuse' ); } ); From 6b97321200cf7f0f7c225e2738fa2c1d309c4b8f Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 10:40:16 +0200 Subject: [PATCH 43/44] Check frontend in updated pattern --- .../e2e/specs/editor/various/patterns.spec.js | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js index 4937368b5503e..395c1234f5461 100644 --- a/test/e2e/specs/editor/various/patterns.spec.js +++ b/test/e2e/specs/editor/various/patterns.spec.js @@ -118,6 +118,7 @@ test.describe( 'Synced pattern', () => { } ); test.afterEach( async ( { requestUtils } ) => { + await requestUtils.deleteAllPosts(); await requestUtils.deleteAllBlocks(); await requestUtils.deleteAllPatternCategories(); } ); @@ -220,7 +221,7 @@ test.describe( 'Synced pattern', () => { const { id } = await requestUtils.createBlock( { title: 'Alternative greeting block', content: - '\n

Guten Tag!

\n', + '\n

Guten Tag!

\n', status: 'publish', } ); @@ -229,7 +230,7 @@ test.describe( 'Synced pattern', () => { attributes: { ref: id }, } ); - await editor.publishPost(); + const postId = await editor.publishPost(); await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) @@ -268,17 +269,10 @@ test.describe( 'Synced pattern', () => { .filter( { hasText: 'Pattern updated.' } ) .click(); - // Go back to the post. - await editorTopBar.getByRole( 'button', { name: 'Back' } ).click(); - - const [ syncedPattern ] = await editor.getBlocks(); - const patternRecord = await getPatternRecord( - page, - syncedPattern?.attributes?.ref - ); - - expect( patternRecord?.content ).toBe( - '\n

Einen Guten Tag!

\n' + // Check that the frontend shows the updated content. + await page.goto( `/?p=${ postId }` ); + await expect( page.locator( '#reused-paragraph' ) ).toHaveText( + 'Einen Guten Tag!' ); } ); From c122b7991898c00b8d85fa59d174f3950aaca1cf Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 14 May 2024 11:01:19 +0200 Subject: [PATCH 44/44] Stop using `getPatternRecord` --- .../e2e/specs/editor/various/patterns.spec.js | 125 ++++++------------ 1 file changed, 38 insertions(+), 87 deletions(-) diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js index 395c1234f5461..48eaf9e08c9b9 100644 --- a/test/e2e/specs/editor/various/patterns.spec.js +++ b/test/e2e/specs/editor/various/patterns.spec.js @@ -3,14 +3,6 @@ */ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); -async function getPatternRecord( page, patternRef ) { - return await page.evaluate( async ( ref ) => { - return window.wp.data - .select( 'core' ) - .getEditedEntityRecord( 'postType', 'wp_block', ref ); - }, patternRef ); -} - test.describe( 'Unsynced pattern', () => { test.beforeAll( async ( { requestUtils } ) => { await requestUtils.deleteAllBlocks(); @@ -324,35 +316,21 @@ test.describe( 'Synced pattern', () => { // Go back to the post. await editorTopBar.getByRole( 'button', { name: 'Back' } ).click(); - const expectedParagraphBlock = { - name: 'core/paragraph', - attributes: { content: 'After Edit' }, - }; - - // It seems edited values are stored in the `blocks` array. - const [ syncedPattern ] = await editor.getBlocks(); - const [ modifiedBlock ] = ( - await getPatternRecord( page, syncedPattern?.attributes?.ref ) - )?.blocks; - - expect( modifiedBlock?.name ).toBe( expectedParagraphBlock.name ); - expect( modifiedBlock?.attributes?.content ).toBe( - expectedParagraphBlock.attributes.content - ); - await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) ); await editor.clickBlockOptionsMenuItem( 'Detach' ); - await expect - .poll( editor.getBlocks ) - .toMatchObject( [ expectedParagraphBlock ] ); + await expect.poll( editor.getBlocks ).toMatchObject( [ + { + name: 'core/paragraph', + attributes: { content: 'After Edit' }, + }, + ] ); } ); test( 'can be created, inserted, and converted to a regular block', async ( { editor, - page, requestUtils, } ) => { const { id } = await requestUtils.createBlock( { @@ -367,32 +345,23 @@ test.describe( 'Synced pattern', () => { attributes: { ref: id }, } ); - // Wait until the pattern is created and inserted. - await editor.canvas.locator( 'text=Hello there!' ).waitFor(); - - const [ syncedPattern ] = await editor.getBlocks(); - const patternRecord = await getPatternRecord( - page, - syncedPattern?.attributes?.ref - ); - - expect( patternRecord?.content ).toBe( - '\n

Hello there!

\n' - ); - - const expectedParagraphBlock = { - name: 'core/paragraph', - attributes: { content: 'Hello there!' }, - }; + // Check that only the pattern block is present. + const existingBlocks = await editor.getBlocks(); + expect( + existingBlocks.every( ( block ) => block.name === 'core/block' ) + ).toBe( true ); await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) ); await editor.clickBlockOptionsMenuItem( 'Detach' ); - await expect - .poll( editor.getBlocks ) - .toMatchObject( [ expectedParagraphBlock ] ); + await expect.poll( editor.getBlocks ).toMatchObject( [ + { + name: 'core/paragraph', + attributes: { content: 'Hello there!' }, + }, + ] ); } ); test( 'can be inserted after refresh', async ( { @@ -426,20 +395,15 @@ test.describe( 'Synced pattern', () => { await page.keyboard.type( '/Awesome block' ); await page.getByRole( 'option', { name: 'Awesome block' } ).click(); - const [ syncedPattern ] = await editor.getBlocks(); - const patternRecord = await getPatternRecord( - page, - syncedPattern?.attributes?.ref - ); - - expect( patternRecord?.content ).toBe( - '\n

Awesome Paragraph

\n' - ); + // Check that the pattern block is present. + const existingBlocks = await editor.getBlocks(); + expect( + existingBlocks.every( ( block ) => block.name === 'core/block' ) + ).toBe( true ); } ); test( 'can be created from multiselection and converted back to regular blocks', async ( { editor, - page, pageUtils, } ) => { await editor.insertBlock( { @@ -467,17 +431,6 @@ test.describe( 'Synced pattern', () => { .getByRole( 'button', { name: 'Create' } ) .click(); - const expectedParagraphBlocks = [ - { - name: 'core/paragraph', - attributes: { content: 'Hello there!' }, - }, - { - name: 'core/paragraph', - attributes: { content: 'Second paragraph' }, - }, - ]; - // Wait until the pattern is created. await editor.canvas .getByRole( 'document', { @@ -485,30 +438,28 @@ test.describe( 'Synced pattern', () => { } ) .waitFor(); - const [ syncedPattern ] = await editor.getBlocks(); - const patternRecord = await getPatternRecord( - page, - syncedPattern?.attributes?.ref - ); - - expect( patternRecord?.content ).toBe( - ` -

Hello there!

- - - -

Second paragraph

-` - ); + // Check that only the pattern block is present. + const existingBlocks = await editor.getBlocks(); + expect( + existingBlocks.every( ( block ) => block.name === 'core/block' ) + ).toBe( true ); + // Convert the pattern back to regular blocks. await editor.selectBlocks( editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) ); await editor.clickBlockOptionsMenuItem( 'Detach' ); - await expect - .poll( editor.getBlocks ) - .toMatchObject( expectedParagraphBlocks ); + await expect.poll( editor.getBlocks ).toMatchObject( [ + { + name: 'core/paragraph', + attributes: { content: 'Hello there!' }, + }, + { + name: 'core/paragraph', + attributes: { content: 'Second paragraph' }, + }, + ] ); } ); // Check for regressions of https://github.com/WordPress/gutenberg/pull/26484.