diff --git a/packages/block-editor/src/hooks/use-bindings-attributes.js b/packages/block-editor/src/hooks/use-bindings-attributes.js index 5ca526b5749b7..b7a4ca0379dd1 100644 --- a/packages/block-editor/src/hooks/use-bindings-attributes.js +++ b/packages/block-editor/src/hooks/use-bindings-attributes.js @@ -25,8 +25,8 @@ import { unlock } from '../lock-unlock'; const BLOCK_BINDINGS_ALLOWED_BLOCKS = { 'core/paragraph': [ 'content' ], 'core/heading': [ 'content' ], - 'core/image': [ 'url', 'title', 'alt' ], - 'core/button': [ 'url', 'text', 'linkTarget' ], + 'core/image': [ 'id', 'url', 'title', 'alt' ], + 'core/button': [ 'url', 'text', 'linkTarget', 'rel' ], }; /** @@ -110,34 +110,66 @@ export const withBlockBindingSupport = createHigherOrderComponent( ( nextAttributes ) => { registry.batch( () => { if ( ! bindings ) { - return setAttributes( nextAttributes ); + setAttributes( nextAttributes ); + return; } const keptAttributes = { ...nextAttributes }; + const updatesBySource = new Map(); - for ( const [ - attributeName, - boundAttribute, - ] of Object.entries( bindings ) ) { - const source = sources[ boundAttribute.source ]; + // Loop only over the updated attributes to avoid modifying the bound ones that haven't changed. + for ( const [ attributeName, newValue ] of Object.entries( + keptAttributes + ) ) { if ( - ! source?.setValue || + ! bindings[ attributeName ] || ! canBindAttribute( name, attributeName ) ) { continue; } - source.setValue( { - registry, - context, - clientId, - attributeName, - args: boundAttribute.args, - value: nextAttributes[ attributeName ], + const source = + sources[ bindings[ attributeName ].source ]; + if ( ! source?.setValue && ! source?.setValues ) { + continue; + } + 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/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index a274843ff37ed..f3193b4bd3514 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 = @@ -232,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. @@ -257,33 +136,22 @@ function ReusableBlockEdit( { clientId: patternClientId, setAttributes, } ) { - const registry = useRegistry(); - const { record, editedRecord, hasResolved } = useEntityRecord( + const { record, hasResolved } = useEntityRecord( 'postType', 'wp_block', ref ); + const [ blocks ] = 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, __unstableMarkLastChangeAsPersistent } = + useDispatch( blockEditorStore ); const { innerBlocks, userCanEdit, - getBlockEditingMode, onNavigateToEntityRecord, editingMode, hasPatternOverridesSource, @@ -296,12 +164,11 @@ function ReusableBlockEdit( { 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, + innerBlocks: getBlocks( patternClientId ), userCanEdit: canEdit, getBlockEditingMode: _getBlockEditingMode, onNavigateToEntityRecord: @@ -333,64 +200,11 @@ function ReusableBlockEdit( { ] ); const canOverrideBlocks = useMemo( - () => hasPatternOverridesSource && hasOverridableBlocks( innerBlocks ), - [ hasPatternOverridesSource, innerBlocks ] - ); - - 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 ] + () => hasPatternOverridesSource && hasOverridableBlocks( blocks ), + [ hasPatternOverridesSource, blocks ] ); - 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( { @@ -401,47 +215,16 @@ function ReusableBlockEdit( { ), } ); + // Use `blocks` variable until `innerBlocks` is populated, which has the proper clientIds. const innerBlocksProps = useInnerBlocksProps( blockProps, { templateLock: 'contentOnly', layout, - renderAppender: innerBlocks?.length - ? undefined - : InnerBlocks.ButtonBlockAppender, + value: innerBlocks.length > 0 ? innerBlocks : blocks, + onInput: NOOP, + onChange: NOOP, + 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 +234,9 @@ function ReusableBlockEdit( { const resetContent = () => { if ( content ) { - replaceInnerBlocks( patternClientId, initialBlocks ); + // Make sure any previous changes are persisted before resetting. + __unstableMarkLastChangeAsPersistent(); + setAttributes( { content: undefined } ); } }; 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 5f7f475a649a3..7554da26481e3 100644 --- a/packages/editor/src/bindings/pattern-overrides.js +++ b/packages/editor/src/bindings/pattern-overrides.js @@ -2,10 +2,92 @@ * 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, getBlockParentsByBlockName } = + registry.select( blockEditorStore ); + const currentBlockAttributes = getBlockAttributes( clientId ); + const [ patternClientId ] = getBlockParentsByBlockName( + clientId, + 'core/block', + true + ); + + 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 === undefined ) { + return currentBlockAttributes[ attributeName ]; + } + + return overridableValue === '' ? undefined : overridableValue; + }, + setValues( { registry, clientId, attributes } ) { + 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 syncBlocksWithSameName = ( blocks ) => { + for ( const block of blocks ) { + if ( block.attributes?.metadata?.name === blockName ) { + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( + block.clientId, + attributes + ); + } + syncBlocksWithSameName( block.innerBlocks ); + } + }; + + syncBlocksWithSameName( getBlocks() ); + return; + } + const currentBindingValue = + getBlockAttributes( patternClientId )?.[ CONTENT ]; + registry + .dispatch( blockEditorStore ) + .updateBlockAttributes( patternClientId, { + [ CONTENT ]: { + ...currentBindingValue, + [ blockName ]: { + ...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; + }, + {} + ), + }, + }, + } ); + }, lockAttributesEditing: false, }; diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 1d3ae013addd3..11f282481a3ac 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -6,80 +6,84 @@ 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 { useRegistry, 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 registry = useRegistry(); + const isOverriden = useSelect( ( select ) => { if ( ! name ) { - return undefined; + return; } - const { getBlockParentsByBlockName, getBlocksByClientId } = + const { getBlockAttributes, getBlockParentsByBlockName } = select( blockEditorStore ); - const patternBlock = getBlocksByClientId( - getBlockParentsByBlockName( props.clientId, 'core/block' ) - )[ 0 ]; + const [ patternClientId ] = getBlockParentsByBlockName( + props.clientId, + 'core/block', + true + ); + + if ( ! patternClientId ) { + return; + } + + const overrides = getBlockAttributes( patternClientId )[ CONTENT ]; - if ( ! patternBlock?.attributes.content?.[ name ] ) { - return undefined; + if ( ! overrides ) { + return; } - return patternBlock; + return overrides.hasOwnProperty( name ); }, [ 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 + function onClick() { + const { getBlockAttributes, getBlockParentsByBlockName } = + registry.select( blockEditorStore ); + const [ patternClientId ] = getBlockParentsByBlockName( + props.clientId, + 'core/block', + true ); - props.setAttributes( newAttributes ); - }; + if ( ! patternClientId ) { + return; + } + + const overrides = getBlockAttributes( patternClientId )[ CONTENT ]; + + if ( ! overrides.hasOwnProperty( name ) ) { + return; + } + + const { updateBlockAttributes, __unstableMarkLastChangeAsPersistent } = + registry.dispatch( blockEditorStore ); + __unstableMarkLastChangeAsPersistent(); + + let newOverrides = { ...overrides }; + delete newOverrides[ name ]; + + if ( ! Object.keys( newOverrides ).length ) { + newOverrides = undefined; + } + + updateBlockAttributes( patternClientId, { + [ CONTENT ]: newOverrides, + } ); + } return ( { __( 'Reset' ) } diff --git a/packages/patterns/src/store/actions.js b/packages/patterns/src/store/actions.js index 7b9090ae4de66..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, @@ -115,11 +134,15 @@ export const convertSyncedPatternToStatic = } ); } + const patternInnerBlocks = registry + .select( blockEditorStore ) + .getBlocks( patternBlock.clientId ); + registry .dispatch( blockEditorStore ) .replaceBlocks( patternBlock.clientId, - cloneBlocksAndRemoveBindings( patternBlock.innerBlocks ) + cloneBlocksAndRemoveBindings( patternInnerBlocks ) ); }; diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 4dbe65436aa95..f252c670d7b07 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,20 @@ 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', + } ); + const paragraphBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Paragraph', + } ); + 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 +461,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. @@ -587,6 +575,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( 'Paragraph*' ); + await redoButton.click(); + await expect( paragraphBlock ).toHaveText( 'Paragraph' ); + } ); + // Fix https://github.com/WordPress/gutenberg/issues/58708. test( 'overridden empty images should not have upload button', async ( { page, @@ -648,4 +700,101 @@ test.describe( 'Pattern Overrides', () => { } ) ).toBeHidden(); } ); + + 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 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 admin.visitSiteEditor( { + postId: id, + postType: 'wp_block', + categoryType: 'pattern', + canvas: 'edit', + } ); + + 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(); + + // 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' } ) + .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' ); + } ); + } ); } ); diff --git a/test/e2e/specs/editor/various/patterns.spec.js b/test/e2e/specs/editor/various/patterns.spec.js index fc1754330b3f5..48eaf9e08c9b9 100644 --- a/test/e2e/specs/editor/various/patterns.spec.js +++ b/test/e2e/specs/editor/various/patterns.spec.js @@ -110,6 +110,7 @@ test.describe( 'Synced pattern', () => { } ); test.afterEach( async ( { requestUtils } ) => { + await requestUtils.deleteAllPosts(); await requestUtils.deleteAllBlocks(); await requestUtils.deleteAllPatternCategories(); } ); @@ -120,7 +121,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. @@ -149,34 +153,18 @@ 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(); - + // 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 @@ -191,7 +179,29 @@ 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(); + // 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 + ); + + // 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' + ); + await expect( secondParagraph ).toHaveText( + 'A useful paragraph to reuse' + ); } ); // Check for regressions of https://github.com/WordPress/gutenberg/issues/33072. @@ -203,7 +213,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', } ); @@ -212,7 +222,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' } ) @@ -251,25 +261,11 @@ test.describe( 'Synced pattern', () => { .filter( { hasText: 'Pattern updated.' } ) .click(); - // 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: [], - }, - ], - }, - ] ); + // Check that the frontend shows the updated content. + await page.goto( `/?p=${ postId }` ); + await expect( page.locator( '#reused-paragraph' ) ).toHaveText( + 'Einen Guten Tag!' + ); } ); // Check for regressions of https://github.com/WordPress/gutenberg/issues/26421. @@ -320,27 +316,17 @@ 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' }, - }; - - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: id }, - innerBlocks: [ expectedParagraphBlock ], - }, - ] ); - 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 ( { @@ -359,27 +345,23 @@ test.describe( 'Synced pattern', () => { attributes: { ref: id }, } ); - const expectedParagraphBlock = { - name: 'core/paragraph', - attributes: { content: 'Hello there!' }, - }; - - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: id }, - innerBlocks: [ expectedParagraphBlock ], - }, - ] ); + // 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 ( { @@ -413,18 +395,11 @@ 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' }, - }, - ], - }, - ] ); + // 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 ( { @@ -456,7 +431,26 @@ test.describe( 'Synced pattern', () => { .getByRole( 'button', { name: 'Create' } ) .click(); - const expectedParagraphBlocks = [ + // Wait until the pattern is created. + await editor.canvas + .getByRole( 'document', { + name: 'Block: Pattern', + } ) + .waitFor(); + + // 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( [ { name: 'core/paragraph', attributes: { content: 'Hello there!' }, @@ -465,24 +459,7 @@ test.describe( 'Synced pattern', () => { name: 'core/paragraph', attributes: { content: 'Second paragraph' }, }, - ]; - - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { ref: expect.any( Number ) }, - innerBlocks: expectedParagraphBlocks, - }, ] ); - - await editor.selectBlocks( - editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } ) - ); - await editor.clickBlockOptionsMenuItem( 'Detach' ); - - await expect - .poll( editor.getBlocks ) - .toMatchObject( expectedParagraphBlocks ); } ); // Check for regressions of https://github.com/WordPress/gutenberg/pull/26484.