From ba91d58f1c92c13b62ee7ad42b173065f260bee5 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 22 Feb 2024 16:05:05 +0800 Subject: [PATCH 01/20] Try using block metadata names instead of nano ids for pattern override bindings - Remove 'Allow instance overrides' checkbox - In its place, add a hook that detects whether a metadata name is set, and use that to create bindings - Update the PHP binding callback to use the block metadata name --- .../block-bindings/pattern-overrides.php | 6 +- packages/block-library/src/block/edit.js | 20 +-- .../src/hooks/pattern-partial-syncing.js | 24 ++- .../components/partial-syncing-controls.js | 155 +++++++++--------- packages/patterns/src/private-apis.js | 4 +- 5 files changed, 106 insertions(+), 103 deletions(-) diff --git a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php index 76c3d49ca8085f..c2ed7a3b4a4194 100644 --- a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php +++ b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php @@ -15,11 +15,11 @@ * @return mixed The value computed for the source. */ function gutenberg_block_bindings_pattern_overrides_callback( $source_attrs, $block_instance, $attribute_name ) { - if ( empty( $block_instance->attributes['metadata']['id'] ) ) { + if ( empty( $block_instance->attributes['metadata']['name'] ) ) { return null; } - $block_id = $block_instance->attributes['metadata']['id']; - return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, 'values', $attribute_name ), null ); + $metadata_name = $block_instance->attributes['metadata']['name']; + return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $metadata_name, 'values', $attribute_name ), null ); } /** diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 5efe245c935fc8..be7299c82e75e5 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -109,17 +109,17 @@ function applyInitialContentValuesToInnerBlocks( content, defaultValues ); - const blockId = block.attributes.metadata?.id; - if ( ! hasOverridableAttributes( block ) || ! blockId ) + const metadataName = block.attributes.metadata?.name; + if ( ! metadataName || ! hasOverridableAttributes( block ) ) return { ...block, innerBlocks }; const attributes = getOverridableAttributes( block ); const newAttributes = { ...block.attributes }; for ( const attributeKey of attributes ) { - defaultValues[ blockId ] ??= {}; - defaultValues[ blockId ][ attributeKey ] = + defaultValues[ metadataName ] ??= {}; + defaultValues[ metadataName ][ attributeKey ] = block.attributes[ attributeKey ]; - const contentValues = content[ blockId ]?.values; + const contentValues = content[ metadataName ]?.values; if ( contentValues?.[ attributeKey ] !== undefined ) { newAttributes[ attributeKey ] = contentValues[ attributeKey ]; } @@ -151,20 +151,20 @@ function getContentValuesFromInnerBlocks( blocks, defaultValues ) { content, getContentValuesFromInnerBlocks( block.innerBlocks, defaultValues ) ); - const blockId = block.attributes.metadata?.id; - if ( ! hasOverridableAttributes( block ) || ! blockId ) continue; + const metadataName = block.attributes.metadata?.name; + if ( ! metadataName || ! hasOverridableAttributes( block ) ) continue; const attributes = getOverridableAttributes( block ); for ( const attributeKey of attributes ) { if ( ! isAttributeEqual( block.attributes[ attributeKey ], - defaultValues[ blockId ][ attributeKey ] + defaultValues[ metadataName ][ attributeKey ] ) ) { - content[ blockId ] ??= { values: {}, blockName: block.name }; + content[ metadataName ] ??= { values: {} }; // TODO: We need a way to represent `undefined` in the serialized overrides. // Also see: https://github.com/WordPress/gutenberg/pull/57249#discussion_r1452987871 - content[ blockId ].values[ attributeKey ] = + content[ metadataName ].values[ 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. diff --git a/packages/editor/src/hooks/pattern-partial-syncing.js b/packages/editor/src/hooks/pattern-partial-syncing.js index f86268cb495463..793537ec2f7040 100644 --- a/packages/editor/src/hooks/pattern-partial-syncing.js +++ b/packages/editor/src/hooks/pattern-partial-syncing.js @@ -14,7 +14,7 @@ import { store as editorStore } from '../store'; import { unlock } from '../lock-unlock'; const { - PartialSyncingControls, + useSetPatternBindings, ResetOverridesControl, PATTERN_TYPES, PARTIAL_SYNCING_SUPPORTED_BLOCKS, @@ -29,7 +29,7 @@ const { * * @return {Component} Wrapped component. */ -const withPartialSyncingControls = createHigherOrderComponent( +const withPatternOverrideControls = createHigherOrderComponent( ( BlockEdit ) => ( props ) => { const isSupportedBlock = Object.keys( PARTIAL_SYNCING_SUPPORTED_BLOCKS @@ -38,6 +38,7 @@ const withPartialSyncingControls = createHigherOrderComponent( return ( <> + { isSupportedBlock && } { props.isSelected && isSupportedBlock && ( ) } @@ -46,6 +47,15 @@ const withPartialSyncingControls = createHigherOrderComponent( } ); +function BindingUpdater( props ) { + const postType = useSelect( + ( select ) => select( editorStore ).getCurrentPostType(), + [] + ); + useSetPatternBindings( props, postType ); + return null; +} + // Split into a separate component to avoid a store subscription // on every block. function ControlsWithStoreSubscription( props ) { @@ -55,6 +65,7 @@ function ControlsWithStoreSubscription( props ) { select( editorStore ).getCurrentPostType() === PATTERN_TYPES.user, [] ); + const bindings = props.attributes.metadata?.bindings; const hasPatternBindings = !! bindings && @@ -62,8 +73,6 @@ function ControlsWithStoreSubscription( props ) { ( binding ) => binding.source === 'core/pattern-overrides' ); - const shouldShowPartialSyncingControls = - isEditingPattern && blockEditingMode === 'default'; const shouldShowResetOverridesControl = ! isEditingPattern && !! props.attributes.metadata?.id && @@ -72,9 +81,6 @@ function ControlsWithStoreSubscription( props ) { return ( <> - { shouldShowPartialSyncingControls && ( - - ) } { shouldShowResetOverridesControl && ( ) } @@ -84,6 +90,6 @@ function ControlsWithStoreSubscription( props ) { addFilter( 'editor.BlockEdit', - 'core/editor/with-partial-syncing-controls', - withPartialSyncingControls + 'core/editor/with-pattern-override-controls', + withPatternOverrideControls ); diff --git a/packages/patterns/src/components/partial-syncing-controls.js b/packages/patterns/src/components/partial-syncing-controls.js index 7b3e5cb312e82e..6834be781808d4 100644 --- a/packages/patterns/src/components/partial-syncing-controls.js +++ b/packages/patterns/src/components/partial-syncing-controls.js @@ -1,108 +1,105 @@ -/** - * External dependencies - */ -import { nanoid } from 'nanoid'; - /** * WordPress dependencies */ -import { InspectorControls } from '@wordpress/block-editor'; -import { BaseControl, CheckboxControl } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; +import { usePrevious } from '@wordpress/compose'; +import { useEffect } from '@wordpress/element'; /** * Internal dependencies */ import { PARTIAL_SYNCING_SUPPORTED_BLOCKS } from '../constants'; -function PartialSyncingControls( { name, attributes, setAttributes } ) { - const syncedAttributes = PARTIAL_SYNCING_SUPPORTED_BLOCKS[ name ]; - const attributeSources = syncedAttributes.map( - ( attributeName ) => - attributes.metadata?.bindings?.[ attributeName ]?.source - ); - const isConnectedToOtherSources = attributeSources.every( - ( source ) => source && source !== 'core/pattern-overrides' - ); +function removeBindings( bindings, syncedAttributes ) { + let updatedBindings = {}; + for ( const attributeName of syncedAttributes ) { + // Omit any pattern override bindings from the `updatedBindings` object. + if ( + bindings?.[ attributeName ]?.source !== 'core/pattern-overrides' && + bindings?.[ attributeName ]?.source !== undefined + ) { + updatedBindings[ attributeName ] = bindings[ attributeName ]; + } + } + if ( ! Object.keys( updatedBindings ).length ) { + updatedBindings = undefined; + } + return updatedBindings; +} - // Render nothing if all supported attributes are connected to other sources. - if ( isConnectedToOtherSources ) { - return null; +function setBindings( bindings, syncedAttributes ) { + const updatedBindings = { ...bindings }; + for ( const attributeName of syncedAttributes ) { + if ( ! bindings?.[ attributeName ] ) { + updatedBindings[ attributeName ] = { + source: 'core/pattern-overrides', + }; + } } + return updatedBindings; +} + +export default function useSetPatternBindings( + { name, attributes, setAttributes }, + currentPostType +) { + const metadataName = attributes?.metadata?.name; + const prevMetadataName = usePrevious( metadataName ) ?? metadataName; + const bindings = attributes?.metadata?.bindings; + + useEffect( () => { + // Bindings should only be created when editing a wp_block post type, + // and also when there's a change to the user given name for the block. + if ( + currentPostType !== 'wp_block' || + metadataName === prevMetadataName + ) { + return; + } - function updateBindings( isChecked ) { - let updatedBindings = { - ...attributes?.metadata?.bindings, - }; + const syncedAttributes = PARTIAL_SYNCING_SUPPORTED_BLOCKS[ name ]; + const attributeSources = syncedAttributes.map( + ( attributeName ) => + attributes.metadata?.bindings?.[ attributeName ]?.source + ); + const isConnectedToOtherSources = attributeSources.every( + ( source ) => source && source !== 'core/pattern-overrides' + ); - if ( ! isChecked ) { - for ( const attributeName of syncedAttributes ) { - if ( - updatedBindings[ attributeName ]?.source === - 'core/pattern-overrides' - ) { - delete updatedBindings[ attributeName ]; - } - } - if ( ! Object.keys( updatedBindings ).length ) { - updatedBindings = undefined; - } + // Avoid overwriting other (e.g. meta) bindings. + if ( isConnectedToOtherSources ) { + return; + } + + let updatedBindings; + + // The user given name for the block was deleted, remove the bindings. + if ( ! metadataName?.length && prevMetadataName?.length ) { + updatedBindings = removeBindings( bindings, syncedAttributes ); setAttributes( { metadata: { ...attributes.metadata, bindings: updatedBindings, }, } ); - return; - } - - for ( const attributeName of syncedAttributes ) { - if ( ! updatedBindings[ attributeName ] ) { - updatedBindings[ attributeName ] = { - source: 'core/pattern-overrides', - }; - } } - if ( typeof attributes.metadata?.id === 'string' ) { + // The user given name for the block was set, set the bindings. + if ( ! prevMetadataName?.length && metadataName.length ) { + updatedBindings = setBindings( bindings, syncedAttributes ); setAttributes( { metadata: { ...attributes.metadata, bindings: updatedBindings, }, } ); - return; } - - const id = nanoid( 6 ); - setAttributes( { - metadata: { - ...attributes.metadata, - id, - bindings: updatedBindings, - }, - } ); - } - - return ( - - - - { __( 'Pattern overrides' ) } - - source === 'core/pattern-overrides' - ) } - onChange={ ( isChecked ) => { - updateBindings( isChecked ); - } } - /> - - - ); + }, [ + bindings, + prevMetadataName, + metadataName, + currentPostType, + name, + attributes.metadata, + setAttributes, + ] ); } - -export default PartialSyncingControls; diff --git a/packages/patterns/src/private-apis.js b/packages/patterns/src/private-apis.js index a5fbddb62fd62c..289f8ddf8527db 100644 --- a/packages/patterns/src/private-apis.js +++ b/packages/patterns/src/private-apis.js @@ -13,7 +13,7 @@ import { import RenamePatternModal from './components/rename-pattern-modal'; import PatternsMenuItems from './components'; import RenamePatternCategoryModal from './components/rename-pattern-category-modal'; -import PartialSyncingControls from './components/partial-syncing-controls'; +import useSetPatternBindings from './components/partial-syncing-controls'; import ResetOverridesControl from './components/reset-overrides-control'; import { useAddPatternCategory } from './private-hooks'; import { @@ -34,7 +34,7 @@ lock( privateApis, { RenamePatternModal, PatternsMenuItems, RenamePatternCategoryModal, - PartialSyncingControls, + useSetPatternBindings, ResetOverridesControl, useAddPatternCategory, PATTERN_TYPES, From 92fb5db5f1e453386c32c052f18489c065f3b08d Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 22 Feb 2024 16:31:09 +0800 Subject: [PATCH 02/20] Switch to using a const --- .../patterns/src/components/partial-syncing-controls.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/patterns/src/components/partial-syncing-controls.js b/packages/patterns/src/components/partial-syncing-controls.js index 6834be781808d4..b44430792704b1 100644 --- a/packages/patterns/src/components/partial-syncing-controls.js +++ b/packages/patterns/src/components/partial-syncing-controls.js @@ -70,11 +70,12 @@ export default function useSetPatternBindings( return; } - let updatedBindings; - // The user given name for the block was deleted, remove the bindings. if ( ! metadataName?.length && prevMetadataName?.length ) { - updatedBindings = removeBindings( bindings, syncedAttributes ); + const updatedBindings = removeBindings( + bindings, + syncedAttributes + ); setAttributes( { metadata: { ...attributes.metadata, @@ -85,7 +86,7 @@ export default function useSetPatternBindings( // The user given name for the block was set, set the bindings. if ( ! prevMetadataName?.length && metadataName.length ) { - updatedBindings = setBindings( bindings, syncedAttributes ); + const updatedBindings = setBindings( bindings, syncedAttributes ); setAttributes( { metadata: { ...attributes.metadata, From 6d4b66e9527c173a825ef303295bfbd831a11894 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 26 Feb 2024 14:54:33 +0800 Subject: [PATCH 03/20] Remove `values` property, reverting back to attributes being at root of `content` attribute --- .../wordpress-6.5/block-bindings/pattern-overrides.php | 2 +- packages/block-library/src/block/edit.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php index c2ed7a3b4a4194..a4dfba2e19dd71 100644 --- a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php +++ b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php @@ -19,7 +19,7 @@ function gutenberg_block_bindings_pattern_overrides_callback( $source_attrs, $bl return null; } $metadata_name = $block_instance->attributes['metadata']['name']; - return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $metadata_name, 'values', $attribute_name ), null ); + return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $metadata_name, $attribute_name ), null ); } /** diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index be7299c82e75e5..ab9ec116d198de 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -119,7 +119,7 @@ function applyInitialContentValuesToInnerBlocks( defaultValues[ metadataName ][ attributeKey ] = block.attributes[ attributeKey ]; - const contentValues = content[ metadataName ]?.values; + const contentValues = content[ metadataName ]; if ( contentValues?.[ attributeKey ] !== undefined ) { newAttributes[ attributeKey ] = contentValues[ attributeKey ]; } @@ -161,10 +161,10 @@ function getContentValuesFromInnerBlocks( blocks, defaultValues ) { defaultValues[ metadataName ][ attributeKey ] ) ) { - content[ metadataName ] ??= { values: {} }; + 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 ].values[ attributeKey ] = + 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. From ef591504f39695aa9af848f32e7dfc729ce2ca49 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 28 Feb 2024 14:43:07 +0800 Subject: [PATCH 04/20] Add a migration to the new format --- .../block-library/src/block/deprecated.js | 87 ++++++++++++++++--- packages/block-library/src/block/index.php | 34 +++++--- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/packages/block-library/src/block/deprecated.js b/packages/block-library/src/block/deprecated.js index 7bc243bbf4ce98..70c548055fa3e2 100644 --- a/packages/block-library/src/block/deprecated.js +++ b/packages/block-library/src/block/deprecated.js @@ -1,4 +1,75 @@ -// v1: Migrate and rename the `overrides` attribute to the `content` attribute. +const isObject = ( obj ) => + typeof obj === 'object' && ! Array.isArray( obj ) && obj !== null; + +// v2: Migrate to a more condensed version of the 'content' attribute attribute. +const v2 = { + attributes: { + ref: { + type: 'number', + }, + overrides: { + type: 'object', + }, + }, + supports: { + customClassName: false, + html: false, + inserter: false, + renaming: false, + }, + // Force this deprecation to run whenever there's a values sub-property that's an object. + // + // This could fail in the future if a block ever has binding to a `values` attribute. + // Some extra protection is added to ensure `values` is an object, but this only reduces + // the likelihood, it doesn't solve it completely. + isEligible( { content } ) { + return ( + !! content && + Object.keys( content ).every( + ( contentKey ) => + content[ contentKey ].values && + isObject( content[ contentKey ].values ) + ) + ); + }, + /* + * Old attribute format: + * content: { + * "V98q_x": { + * // The attribute values are now stored as a 'values' sub-property. + * values: { content: 'My content value' }, + * // ... additional metadata, like the block name can be stored here. + * } + * } + * + * New attribute format: + * content: { + * "V98q_x": { + * content: 'My content value', + * } + * } + */ + migrate( attributes ) { + const { content, ...retainedAttributes } = attributes; + + if ( content && Object.keys( content ).length ) { + const updatedContent = { ...content }; + + for ( const contentKey in content ) { + updatedContent[ contentKey ] = content[ contentKey ].values; + } + + return { + ...retainedAttributes, + content, + }; + } + + return attributes; + }, +}; + +// v1: Rename the `overrides` attribute to the `content` attribute. const v1 = { attributes: { ref: { @@ -23,16 +94,12 @@ const v1 = { * overrides: { * // An key is an id that represents a block. * // The values are the attribute values of the block. - * "V98q_x": { content: 'dwefwefwefwe' } + * "V98q_x": { content: 'My content value' } * } * * New attribute format: * content: { - * "V98q_x": { - * // The attribute values are now stored as a 'values' sub-property. - * values: { content: 'dwefwefwefwe' }, - * // ... additional metadata, like the block name can be stored here. - * } + * "V98q_x": { content: 'My content value' } * } * */ @@ -42,9 +109,7 @@ const v1 = { const content = {}; Object.keys( overrides ).forEach( ( id ) => { - content[ id ] = { - values: overrides[ id ], - }; + content[ id ] = overrides[ id ]; } ); return { @@ -54,4 +119,4 @@ const v1 = { }, }; -export default [ v1 ]; +export default [ v2, v1 ]; diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 4aabe98ffa4650..3d154d8a59076b 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -48,26 +48,36 @@ function render_block_core_block( $attributes ) { $content = $wp_embed->run_shortcode( $reusable_block->post_content ); $content = $wp_embed->autoembed( $content ); - // Back compat, the content attribute was previously named overrides and - // had a slightly different format. For blocks that have not been migrated, - // also convert the format here so that the provided `pattern/overrides` - // context is correct. - if ( isset( $attributes['overrides'] ) && ! isset( $attributes['content'] ) ) { - $migrated_content = array(); - foreach ( $attributes['overrides'] as $id => $values ) { - $migrated_content[ $id ] = array( - 'values' => $values, - ); + // Back compat. + // For blocks that have not been migrated in the editor, add some back compat + // so that front-end rendering continues to work. + + // This matches the `v2` deprecation. Removes the inner `values` property + // from every item. + if ( isset( $attributes['content' ] ) ) { + foreach ( $attributes['content'] as $content_key => &$content_data ) { + if ( isset( $content_data[ 'values' ] ) ) { + $is_assoc_array = is_array( $content_data[ 'values' ] ) && ! wp_is_numeric_array( $content_data[ 'values' ] ); + + if ( $is_assoc_array ) { + $content_data = $content_data[ 'values' ]; + } + } } - $attributes['content'] = $migrated_content; } - $has_pattern_overrides = isset( $attributes['content'] ); + + // This matches the `v1` deprecation. Rename `overrides` to `content`. + if ( isset( $attributes['overrides'] ) && ! isset( $attributes['content'] ) ) { + $attributes['content'] = $attributes['overrides']; + } + /** * We set the `pattern/overrides` context through the `render_block_context` * filter so that it is available when a pattern's inner blocks are * rendering via do_blocks given it only receives the inner content. */ + $has_pattern_overrides = isset( $attributes['content'] ); if ( $has_pattern_overrides ) { $filter_block_context = static function ( $context ) use ( $attributes ) { $context['pattern/overrides'] = $attributes['content']; From fa3bb13bd6f9b91c6dfa8d4c76aec1b2d08f10c1 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 28 Feb 2024 16:04:23 +0800 Subject: [PATCH 05/20] Add backwards compatibility --- .../block-bindings/pattern-overrides.php | 25 ++++++- .../block-library/src/block/deprecated.js | 4 +- packages/block-library/src/block/edit.js | 67 +++++++++++++++---- 3 files changed, 79 insertions(+), 17 deletions(-) diff --git a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php index a4dfba2e19dd71..95c6d1816ecc32 100644 --- a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php +++ b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php @@ -15,11 +15,30 @@ * @return mixed The value computed for the source. */ function gutenberg_block_bindings_pattern_overrides_callback( $source_attrs, $block_instance, $attribute_name ) { - if ( empty( $block_instance->attributes['metadata']['name'] ) ) { + if ( ! isset( $block_instance->context[ 'pattern/overrides'] ) ) { return null; } - $metadata_name = $block_instance->attributes['metadata']['name']; - return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $metadata_name, $attribute_name ), null ); + + $override_content = $block_instance->context[ 'pattern/overrides']; + + // Back compat. Pattern overrides previously used a metadata `id` instead of `name`. + // We check first for the name, and if it exists, use that value. + if ( isset( $block_instance->attributes['metadata']['name'] ) ) { + $metadata_name = $block_instance->attributes['metadata']['name']; + if ( array_key_exists( $metadata_name, $override_content ) ) { + return _wp_array_get( $override_content, array( $metadata_name, $attribute_name ), null ); + } + } + + // Next check for the `id`. + if ( isset( $block_instance->attributes['metadata']['id'] ) ) { + $metadata_id = $block_instance->attributes['metadata']['id']; + if ( array_key_exists( $metadata_id, $override_content ) ) { + return _wp_array_get( $override_content, array( $metadata_id, $attribute_name ), null ); + } + } + + return null; } /** diff --git a/packages/block-library/src/block/deprecated.js b/packages/block-library/src/block/deprecated.js index 70c548055fa3e2..f820867fff6271 100644 --- a/packages/block-library/src/block/deprecated.js +++ b/packages/block-library/src/block/deprecated.js @@ -7,7 +7,7 @@ const v2 = { ref: { type: 'number', }, - overrides: { + content: { type: 'object', }, }, @@ -61,7 +61,7 @@ const v2 = { return { ...retainedAttributes, - content, + content: updatedContent, }; } diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index ab9ec116d198de..77ad6cd1941691 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -110,18 +110,41 @@ function applyInitialContentValuesToInnerBlocks( defaultValues ); const metadataName = block.attributes.metadata?.name; - if ( ! metadataName || ! hasOverridableAttributes( block ) ) + const metadataId = block.attributes.metadata?.id; + + if ( + ( ! metadataName && ! metadataId ) || + ! hasOverridableAttributes( 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 ]; + // Back compat. Pattern overrides used to use a metadata `id` + // before using a `name`. Check for the name first, but if it + // doesn't exist use the id. + if ( Object.hasOwn( content, metadataName ) ) { + defaultValues[ metadataName ] ??= {}; + defaultValues[ metadataName ][ attributeKey ] = + block.attributes[ attributeKey ]; + + const contentValues = content[ metadataName ]; + if ( contentValues?.[ attributeKey ] !== undefined ) { + newAttributes[ attributeKey ] = + contentValues[ attributeKey ]; + } + } else if ( Object.hasOwn( content, metadataId ) ) { + defaultValues[ metadataId ] ??= {}; + defaultValues[ metadataId ][ attributeKey ] = + block.attributes[ attributeKey ]; + + const contentValues = content[ metadataId ]; + if ( contentValues?.[ attributeKey ] !== undefined ) { + newAttributes[ attributeKey ] = + contentValues[ attributeKey ]; + } } } return { @@ -152,19 +175,39 @@ function getContentValuesFromInnerBlocks( blocks, defaultValues ) { getContentValuesFromInnerBlocks( block.innerBlocks, defaultValues ) ); const metadataName = block.attributes.metadata?.name; - if ( ! metadataName || ! hasOverridableAttributes( block ) ) continue; + const metadataId = block.attributes.metadata?.id; + if ( + ( ! metadataName && ! metadataId ) || + ! hasOverridableAttributes( block ) + ) { + continue; + } + const attributes = getOverridableAttributes( block ); + + // Back compat. Pattern overrides previously used an 'id' instead of a 'name'. + // Check first if the metadata name or id is in use in the existing content, + // and if so use the appropriate one. If not, prefer the name when defined. + let contentKey; + if ( Object.hasOwn( content, metadataName ) ) { + contentKey = metadataName; + } else if ( Object.hasOwn( content, metadataId ) ) { + contentKey = metadataId; + } else { + contentKey = metadataName ? metadataName : metadataId; + } + for ( const attributeKey of attributes ) { if ( ! isAttributeEqual( block.attributes[ attributeKey ], - defaultValues[ metadataName ][ attributeKey ] + defaultValues?.[ contentKey ]?.[ attributeKey ] ) ) { - content[ metadataName ] ??= {}; + content[ contentKey ] ??= {}; // 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 ] = + content[ contentKey ][ 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. From 3b4571003fba71ea235361b145adca8ddd7bdba7 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 12:40:06 +0800 Subject: [PATCH 06/20] Use a map of clientIds to ids for back compat --- packages/block-library/src/block/edit.js | 122 ++++++++++++----------- 1 file changed, 63 insertions(+), 59 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 77ad6cd1941691..d8840eeb3edef2 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -42,6 +42,25 @@ const { PARTIAL_SYNCING_SUPPORTED_BLOCKS } = 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(); @@ -101,50 +120,33 @@ function getOverridableAttributes( block ) { function applyInitialContentValuesToInnerBlocks( blocks, content = {}, - defaultValues + defaultValues, + legacyIdMap ) { return blocks.map( ( block ) => { const innerBlocks = applyInitialContentValuesToInnerBlocks( block.innerBlocks, content, - defaultValues + defaultValues, + legacyIdMap ); - const metadataName = block.attributes.metadata?.name; - const metadataId = block.attributes.metadata?.id; + const metadataName = + legacyIdMap?.[ block.clientId ] ?? block.attributes.metadata?.name; - if ( - ( ! metadataName && ! metadataId ) || - ! hasOverridableAttributes( block ) - ) { + if ( ! metadataName || ! hasOverridableAttributes( block ) ) { return { ...block, innerBlocks }; } const attributes = getOverridableAttributes( block ); const newAttributes = { ...block.attributes }; for ( const attributeKey of attributes ) { - // Back compat. Pattern overrides used to use a metadata `id` - // before using a `name`. Check for the name first, but if it - // doesn't exist use the id. - if ( Object.hasOwn( content, metadataName ) ) { - defaultValues[ metadataName ] ??= {}; - defaultValues[ metadataName ][ attributeKey ] = - block.attributes[ attributeKey ]; - - const contentValues = content[ metadataName ]; - if ( contentValues?.[ attributeKey ] !== undefined ) { - newAttributes[ attributeKey ] = - contentValues[ attributeKey ]; - } - } else if ( Object.hasOwn( content, metadataId ) ) { - defaultValues[ metadataId ] ??= {}; - defaultValues[ metadataId ][ attributeKey ] = - block.attributes[ attributeKey ]; - - const contentValues = content[ metadataId ]; - if ( contentValues?.[ attributeKey ] !== undefined ) { - newAttributes[ attributeKey ] = - contentValues[ attributeKey ]; - } + defaultValues[ metadataName ] ??= {}; + defaultValues[ metadataName ][ attributeKey ] = + block.attributes[ attributeKey ]; + + const contentValues = content[ metadataName ]; + if ( contentValues?.[ attributeKey ] !== undefined ) { + newAttributes[ attributeKey ] = contentValues[ attributeKey ]; } } return { @@ -165,49 +167,40 @@ function isAttributeEqual( attribute1, attribute2 ) { return attribute1 === attribute2; } -function getContentValuesFromInnerBlocks( blocks, defaultValues ) { +function getContentValuesFromInnerBlocks( blocks, defaultValues, legacyIdMap ) { /** @type {Record}>} */ const content = {}; for ( const block of blocks ) { if ( block.name === patternBlockName ) continue; - Object.assign( - content, - getContentValuesFromInnerBlocks( block.innerBlocks, defaultValues ) - ); - const metadataName = block.attributes.metadata?.name; - const metadataId = block.attributes.metadata?.id; - if ( - ( ! metadataName && ! metadataId ) || - ! hasOverridableAttributes( block ) - ) { + if ( block.innerBlocks.length ) { + Object.assign( + content, + getContentValuesFromInnerBlocks( + block.innerBlocks, + defaultValues, + legacyIdMap + ) + ); + } + const metadataName = + legacyIdMap?.[ block.clientId ] ?? block.attributes.metadata?.name; + if ( ! metadataName || ! hasOverridableAttributes( block ) ) { continue; } const attributes = getOverridableAttributes( block ); - // Back compat. Pattern overrides previously used an 'id' instead of a 'name'. - // Check first if the metadata name or id is in use in the existing content, - // and if so use the appropriate one. If not, prefer the name when defined. - let contentKey; - if ( Object.hasOwn( content, metadataName ) ) { - contentKey = metadataName; - } else if ( Object.hasOwn( content, metadataId ) ) { - contentKey = metadataId; - } else { - contentKey = metadataName ? metadataName : metadataId; - } - for ( const attributeKey of attributes ) { if ( ! isAttributeEqual( block.attributes[ attributeKey ], - defaultValues?.[ contentKey ]?.[ attributeKey ] + defaultValues?.[ metadataName ]?.[ attributeKey ] ) ) { - content[ contentKey ] ??= {}; + 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[ contentKey ][ attributeKey ] = + 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. @@ -321,6 +314,15 @@ export default function ReusableBlockEdit( { [ editedRecord.blocks, editedRecord.content ] ); + // A map of clientIds to the old nano id system to provide back compat. + const legacyIdMap = useRef( {} ); + useEffect( () => { + legacyIdMap.current = getLegacyIdMap( + initialBlocks, + initialContent.current + ); + }, [ initialBlocks ] ); + // Apply the initial overrides from the pattern block to the inner blocks. useEffect( () => { defaultContent.current = {}; @@ -334,7 +336,8 @@ export default function ReusableBlockEdit( { applyInitialContentValuesToInnerBlocks( initialBlocks, initialContent.current, - defaultContent.current + defaultContent.current, + legacyIdMap.current ) ); } ); @@ -386,7 +389,8 @@ export default function ReusableBlockEdit( { setAttributes( { content: getContentValuesFromInnerBlocks( blocks, - defaultContent.current + defaultContent.current, + legacyIdMap.current ), } ); } ); From 9a48af8be3c7429ac4965c20475d1cdc6c7a2779 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 15:09:16 +0800 Subject: [PATCH 07/20] Fix bindings not being applied when using the rename option in List View --- packages/patterns/src/components/partial-syncing-controls.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/patterns/src/components/partial-syncing-controls.js b/packages/patterns/src/components/partial-syncing-controls.js index b44430792704b1..2d6175fd5bcad3 100644 --- a/packages/patterns/src/components/partial-syncing-controls.js +++ b/packages/patterns/src/components/partial-syncing-controls.js @@ -42,8 +42,8 @@ export default function useSetPatternBindings( { name, attributes, setAttributes }, currentPostType ) { - const metadataName = attributes?.metadata?.name; - const prevMetadataName = usePrevious( metadataName ) ?? metadataName; + const metadataName = attributes?.metadata?.name ?? ''; + const prevMetadataName = usePrevious( metadataName ) ?? ''; const bindings = attributes?.metadata?.bindings; useEffect( () => { From 4cbb6c5b162b2e5b3dba42d5d175ab94bfb04511 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 15:10:50 +0800 Subject: [PATCH 08/20] Rename to addBindings to complement removeBindings --- packages/patterns/src/components/partial-syncing-controls.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/patterns/src/components/partial-syncing-controls.js b/packages/patterns/src/components/partial-syncing-controls.js index 2d6175fd5bcad3..bdee43d35766a8 100644 --- a/packages/patterns/src/components/partial-syncing-controls.js +++ b/packages/patterns/src/components/partial-syncing-controls.js @@ -26,7 +26,7 @@ function removeBindings( bindings, syncedAttributes ) { return updatedBindings; } -function setBindings( bindings, syncedAttributes ) { +function addBindings( bindings, syncedAttributes ) { const updatedBindings = { ...bindings }; for ( const attributeName of syncedAttributes ) { if ( ! bindings?.[ attributeName ] ) { @@ -86,7 +86,7 @@ export default function useSetPatternBindings( // The user given name for the block was set, set the bindings. if ( ! prevMetadataName?.length && metadataName.length ) { - const updatedBindings = setBindings( bindings, syncedAttributes ); + const updatedBindings = addBindings( bindings, syncedAttributes ); setAttributes( { metadata: { ...attributes.metadata, From 8c7656a89bbfd6097300aa55080e6bfd4ef6427e Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 15:14:59 +0800 Subject: [PATCH 09/20] Rename partial-syncing-controls file --- ...{partial-syncing-controls.js => use-set-pattern-bindings.js} | 0 packages/patterns/src/private-apis.js | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/patterns/src/components/{partial-syncing-controls.js => use-set-pattern-bindings.js} (100%) diff --git a/packages/patterns/src/components/partial-syncing-controls.js b/packages/patterns/src/components/use-set-pattern-bindings.js similarity index 100% rename from packages/patterns/src/components/partial-syncing-controls.js rename to packages/patterns/src/components/use-set-pattern-bindings.js diff --git a/packages/patterns/src/private-apis.js b/packages/patterns/src/private-apis.js index 289f8ddf8527db..54ad5a4aa47d1b 100644 --- a/packages/patterns/src/private-apis.js +++ b/packages/patterns/src/private-apis.js @@ -13,7 +13,7 @@ import { import RenamePatternModal from './components/rename-pattern-modal'; import PatternsMenuItems from './components'; import RenamePatternCategoryModal from './components/rename-pattern-category-modal'; -import useSetPatternBindings from './components/partial-syncing-controls'; +import useSetPatternBindings from './components/use-set-pattern-bindings'; import ResetOverridesControl from './components/reset-overrides-control'; import { useAddPatternCategory } from './private-hooks'; import { From a2cf150412b5b4604bab61b014af85d83dc5c0c7 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 15:17:01 +0800 Subject: [PATCH 10/20] Rename pattern-partial-syncing hooks file to pattern-overrides --- packages/editor/src/hooks/index.js | 2 +- .../hooks/{pattern-partial-syncing.js => pattern-overrides.js} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/editor/src/hooks/{pattern-partial-syncing.js => pattern-overrides.js} (100%) diff --git a/packages/editor/src/hooks/index.js b/packages/editor/src/hooks/index.js index 5a48ec1bf49566..75bb34abf6cfa3 100644 --- a/packages/editor/src/hooks/index.js +++ b/packages/editor/src/hooks/index.js @@ -3,4 +3,4 @@ */ import './custom-sources-backwards-compatibility'; import './default-autocompleters'; -import './pattern-partial-syncing'; +import './pattern-overrides'; diff --git a/packages/editor/src/hooks/pattern-partial-syncing.js b/packages/editor/src/hooks/pattern-overrides.js similarity index 100% rename from packages/editor/src/hooks/pattern-partial-syncing.js rename to packages/editor/src/hooks/pattern-overrides.js From 42048f9e77c33ffbaa75ca8148d106b3544518b7 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 15:19:59 +0800 Subject: [PATCH 11/20] Remove nanoid dependency --- package-lock.json | 6 ++---- packages/patterns/package.json | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9c03b2e0e1fafa..6dca2aa7912ad2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -55967,8 +55967,7 @@ "@wordpress/icons": "file:../icons", "@wordpress/notices": "file:../notices", "@wordpress/private-apis": "file:../private-apis", - "@wordpress/url": "file:../url", - "nanoid": "^3.3.4" + "@wordpress/url": "file:../url" }, "engines": { "node": ">=16.0.0" @@ -70828,8 +70827,7 @@ "@wordpress/icons": "file:../icons", "@wordpress/notices": "file:../notices", "@wordpress/private-apis": "file:../private-apis", - "@wordpress/url": "file:../url", - "nanoid": "^3.3.4" + "@wordpress/url": "file:../url" } }, "@wordpress/plugins": { diff --git a/packages/patterns/package.json b/packages/patterns/package.json index eaef15fe2a9327..f35a629508cb08 100644 --- a/packages/patterns/package.json +++ b/packages/patterns/package.json @@ -44,8 +44,7 @@ "@wordpress/icons": "file:../icons", "@wordpress/notices": "file:../notices", "@wordpress/private-apis": "file:../private-apis", - "@wordpress/url": "file:../url", - "nanoid": "^3.3.4" + "@wordpress/url": "file:../url" }, "peerDependencies": { "react": "^18.0.0", From 5e7c6503f736faaab56f2617859d4773ebff2e55 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 29 Feb 2024 15:22:04 +0800 Subject: [PATCH 12/20] Update doc string --- .../patterns/src/components/use-set-pattern-bindings.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/patterns/src/components/use-set-pattern-bindings.js b/packages/patterns/src/components/use-set-pattern-bindings.js index bdee43d35766a8..df16d2b2b05916 100644 --- a/packages/patterns/src/components/use-set-pattern-bindings.js +++ b/packages/patterns/src/components/use-set-pattern-bindings.js @@ -48,7 +48,7 @@ export default function useSetPatternBindings( useEffect( () => { // Bindings should only be created when editing a wp_block post type, - // and also when there's a change to the user given name for the block. + // and also when there's a change to the user-given name for the block. if ( currentPostType !== 'wp_block' || metadataName === prevMetadataName @@ -70,7 +70,7 @@ export default function useSetPatternBindings( return; } - // The user given name for the block was deleted, remove the bindings. + // The user-given name for the block was deleted, remove the bindings. if ( ! metadataName?.length && prevMetadataName?.length ) { const updatedBindings = removeBindings( bindings, @@ -84,7 +84,7 @@ export default function useSetPatternBindings( } ); } - // The user given name for the block was set, set the bindings. + // The user-given name for the block was set, set the bindings. if ( ! prevMetadataName?.length && metadataName.length ) { const updatedBindings = addBindings( bindings, syncedAttributes ); setAttributes( { From 1642af4d04fc7d94d48fa6f8d0c8af203ded7d8e Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:05:02 +1000 Subject: [PATCH 13/20] Fix linting issues --- lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php index 95c6d1816ecc32..e5f9891f04c471 100644 --- a/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php +++ b/lib/compat/wordpress-6.5/block-bindings/pattern-overrides.php @@ -15,11 +15,11 @@ * @return mixed The value computed for the source. */ function gutenberg_block_bindings_pattern_overrides_callback( $source_attrs, $block_instance, $attribute_name ) { - if ( ! isset( $block_instance->context[ 'pattern/overrides'] ) ) { + if ( ! isset( $block_instance->context['pattern/overrides'] ) ) { return null; } - $override_content = $block_instance->context[ 'pattern/overrides']; + $override_content = $block_instance->context['pattern/overrides']; // Back compat. Pattern overrides previously used a metadata `id` instead of `name`. // We check first for the name, and if it exists, use that value. From 4a415c1ca9bb2f4c5832c0316f5104b51077edfb Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:12:13 +1000 Subject: [PATCH 14/20] More linting fixes --- packages/block-library/src/block/index.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 3d154d8a59076b..a27281583c3053 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -54,13 +54,13 @@ function render_block_core_block( $attributes ) { // This matches the `v2` deprecation. Removes the inner `values` property // from every item. - if ( isset( $attributes['content' ] ) ) { + if ( isset( $attributes['content'] ) ) { foreach ( $attributes['content'] as $content_key => &$content_data ) { - if ( isset( $content_data[ 'values' ] ) ) { - $is_assoc_array = is_array( $content_data[ 'values' ] ) && ! wp_is_numeric_array( $content_data[ 'values' ] ); + if ( isset( $content_data['values'] ) ) { + $is_assoc_array = is_array( $content_data['values'] ) && ! wp_is_numeric_array( $content_data['values'] ); if ( $is_assoc_array ) { - $content_data = $content_data[ 'values' ]; + $content_data = $content_data['values']; } } } @@ -71,7 +71,6 @@ function render_block_core_block( $attributes ) { $attributes['content'] = $attributes['overrides']; } - /** * We set the `pattern/overrides` context through the `render_block_context` * filter so that it is available when a pattern's inner blocks are From bd5f4b4369bcb2a78e1e1a1a3ab200c6b28e6399 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:24:47 +1000 Subject: [PATCH 15/20] Linting again --- packages/block-library/src/block/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index a27281583c3053..49b5786eacc79f 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -55,7 +55,7 @@ function render_block_core_block( $attributes ) { // This matches the `v2` deprecation. Removes the inner `values` property // from every item. if ( isset( $attributes['content'] ) ) { - foreach ( $attributes['content'] as $content_key => &$content_data ) { + foreach ( $attributes['content'] as &$content_data ) { if ( isset( $content_data['values'] ) ) { $is_assoc_array = is_array( $content_data['values'] ) && ! wp_is_numeric_array( $content_data['values'] ); From 035d0d67502fd1488c871fd4590d45a9f2f945d6 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 1 Mar 2024 12:08:37 +0800 Subject: [PATCH 16/20] Update e2e tests --- .../editor/various/pattern-overrides.spec.js | 59 ++++++++----------- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 86d7b9117bef53..e5c0455708c4a9 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -32,7 +32,7 @@ test.describe( 'Pattern Overrides', () => { editor, } ) => { let patternId; - let editableParagraphId; + const editableParagraphName = 'Editable Paragraph'; await test.step( 'Create a synced pattern and assign blocks to allow overrides', async () => { await admin.visitSiteEditor( { path: '/patterns' } ); @@ -85,8 +85,9 @@ test.describe( 'Pattern Overrides', () => { await advancedPanel.click(); } await editorSettings - .getByRole( 'checkbox', { name: 'Allow instance overrides' } ) - .setChecked( true ); + .getByRole( 'textbox', { name: 'Block Name' } ) + .click(); + await page.keyboard.type( editableParagraphName ); await expect.poll( editor.getBlocks ).toMatchObject( [ { @@ -94,7 +95,7 @@ test.describe( 'Pattern Overrides', () => { attributes: { content: 'This paragraph can be edited', metadata: { - id: expect.any( String ), + name: editableParagraphName, bindings: { content: { source: 'core/pattern-overrides', @@ -123,8 +124,6 @@ test.describe( 'Pattern Overrides', () => { ).toBeVisible(); patternId = new URL( page.url() ).searchParams.get( 'postId' ); - const blocks = await editor.getBlocks(); - editableParagraphId = blocks[ 0 ].attributes.metadata.id; } ); await test.step( 'Create a post and insert the pattern with overrides', async () => { @@ -176,10 +175,8 @@ test.describe( 'Pattern Overrides', () => { attributes: { ref: patternId, content: { - [ editableParagraphId ]: { - values: { - content: 'I would word it this way', - }, + [ editableParagraphName ]: { + content: 'I would word it this way', }, }, }, @@ -189,10 +186,8 @@ test.describe( 'Pattern Overrides', () => { attributes: { ref: patternId, content: { - [ editableParagraphId ]: { - values: { - content: 'This one is different', - }, + [ editableParagraphName ]: { + content: 'This one is different', }, }, }, @@ -276,11 +271,11 @@ test.describe( 'Pattern Overrides', () => { editor, context, } ) => { - const buttonId = 'button-id'; + const buttonName = 'Editable button'; const { id } = await requestUtils.createBlock( { title: 'Button with target', content: ` -
+ `, @@ -386,21 +381,21 @@ test.describe( 'Pattern Overrides', () => { requestUtils, editor, } ) => { - const paragraphId = 'paragraph-id'; - const headingId = 'heading-id'; + const paragraphName = 'Editable paragraph'; + const headingName = 'Editable heading'; const innerPattern = await requestUtils.createBlock( { title: 'Inner Pattern', - content: ` + content: `

Inner paragraph

`, status: 'publish', } ); const outerPattern = await requestUtils.createBlock( { title: 'Outer Pattern', - content: ` + content: `

Outer heading

-`, +`, status: 'publish', } ); @@ -425,8 +420,8 @@ test.describe( 'Pattern Overrides', () => { attributes: { ref: outerPattern.id, content: { - [ headingId ]: { - values: { content: 'Outer heading (edited)' }, + [ headingName ]: { + content: 'Outer heading (edited)', }, }, }, @@ -440,10 +435,8 @@ test.describe( 'Pattern Overrides', () => { attributes: { ref: innerPattern.id, content: { - [ paragraphId ]: { - values: { - content: 'Inner paragraph (edited)', - }, + [ paragraphName ]: { + content: 'Inner paragraph (edited)', }, }, }, @@ -505,14 +498,14 @@ test.describe( 'Pattern Overrides', () => { requestUtils, editor, } ) => { - const headingId = 'heading-id'; - const paragraphId = 'paragraph-id'; + const headingName = 'Editable heading'; + const paragraphName = 'Editable paragraph'; const { id } = await requestUtils.createBlock( { title: 'Pattern', - content: ` + content: `

Heading

- +

Paragraph

`, status: 'publish', @@ -597,14 +590,14 @@ test.describe( 'Pattern Overrides', () => { requestUtils, editor, } ) => { - const imageId = 'image-id'; + const imageName = 'Editable image'; const TEST_IMAGE_FILE_PATH = path.resolve( __dirname, '../../../assets/10x10_e2e_test_image_z9T8jK.png' ); const { id } = await requestUtils.createBlock( { title: 'Pattern', - content: ` + content: `
`, status: 'publish', From 121ff2dd2692e61508f1a2c25f158579d7c58ffb Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 1 Mar 2024 12:21:45 +0800 Subject: [PATCH 17/20] Fix reset button on individual blocks --- packages/editor/src/hooks/pattern-overrides.js | 2 +- .../src/components/reset-overrides-control.js | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/editor/src/hooks/pattern-overrides.js b/packages/editor/src/hooks/pattern-overrides.js index 793537ec2f7040..442ce70a2bf71c 100644 --- a/packages/editor/src/hooks/pattern-overrides.js +++ b/packages/editor/src/hooks/pattern-overrides.js @@ -75,7 +75,7 @@ function ControlsWithStoreSubscription( props ) { const shouldShowResetOverridesControl = ! isEditingPattern && - !! props.attributes.metadata?.id && + !! props.attributes.metadata?.name && blockEditingMode !== 'disabled' && hasPatternBindings; diff --git a/packages/patterns/src/components/reset-overrides-control.js b/packages/patterns/src/components/reset-overrides-control.js index 586f4608352340..1d3ae013addd3d 100644 --- a/packages/patterns/src/components/reset-overrides-control.js +++ b/packages/patterns/src/components/reset-overrides-control.js @@ -11,13 +11,13 @@ import { store as coreStore } from '@wordpress/core-data'; import { parse } from '@wordpress/blocks'; import { __ } from '@wordpress/i18n'; -function recursivelyFindBlockWithId( blocks, id ) { +function recursivelyFindBlockWithName( blocks, name ) { for ( const block of blocks ) { - if ( block.attributes.metadata?.id === id ) { + if ( block.attributes.metadata?.name === name ) { return block; } - const found = recursivelyFindBlockWithId( block.innerBlocks, id ); + const found = recursivelyFindBlockWithName( block.innerBlocks, name ); if ( found ) { return found; } @@ -26,10 +26,10 @@ function recursivelyFindBlockWithId( blocks, id ) { export default function ResetOverridesControl( props ) { const registry = useRegistry(); - const id = props.attributes.metadata?.id; + const name = props.attributes.metadata?.name; const patternWithOverrides = useSelect( ( select ) => { - if ( ! id ) { + if ( ! name ) { return undefined; } @@ -39,13 +39,13 @@ export default function ResetOverridesControl( props ) { getBlockParentsByBlockName( props.clientId, 'core/block' ) )[ 0 ]; - if ( ! patternBlock?.attributes.content?.[ id ] ) { + if ( ! patternBlock?.attributes.content?.[ name ] ) { return undefined; } return patternBlock; }, - [ props.clientId, id ] + [ props.clientId, name ] ); const resetOverrides = async () => { @@ -57,7 +57,7 @@ export default function ResetOverridesControl( props ) { patternWithOverrides.attributes.ref ); const blocks = editedRecord.blocks ?? parse( editedRecord.content ); - const block = recursivelyFindBlockWithId( blocks, id ); + const block = recursivelyFindBlockWithName( blocks, name ); const newAttributes = Object.assign( // Reset every existing attribute to undefined. From 2f683777a4f2732023580fec6a43d7159231d75a Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 1 Mar 2024 12:24:32 +0800 Subject: [PATCH 18/20] Address feedback --- packages/block-library/src/block/edit.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index d8840eeb3edef2..ddacc47dbd0391 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -314,17 +314,15 @@ export default function ReusableBlockEdit( { [ editedRecord.blocks, editedRecord.content ] ); - // A map of clientIds to the old nano id system to provide back compat. 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, initialContent.current ); - }, [ initialBlocks ] ); - - // Apply the initial overrides from the pattern block to the inner blocks. - useEffect( () => { defaultContent.current = {}; const originalEditingMode = getBlockEditingMode( patternClientId ); // Replace the contents of the blocks with the overrides. From 72299f0b41aad6fc0e14d06072c1e83e54299b07 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 1 Mar 2024 14:05:16 +0800 Subject: [PATCH 19/20] Add fixtures for deprecations --- .../core__block__overrides__deprecated-1.html | 1 + .../core__block__overrides__deprecated-1.json | 15 +++++++++++++++ ..._block__overrides__deprecated-1.parsed.json | 16 ++++++++++++++++ ...ck__overrides__deprecated-1.serialized.html | 1 + .../core__block__overrides__deprecated-2.html | 1 + .../core__block__overrides__deprecated-2.json | 15 +++++++++++++++ ..._block__overrides__deprecated-2.parsed.json | 18 ++++++++++++++++++ ...ck__overrides__deprecated-2.serialized.html | 1 + 8 files changed, 68 insertions(+) create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-1.html create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-1.json create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-1.parsed.json create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-1.serialized.html create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-2.html create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-2.json create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-2.parsed.json create mode 100644 test/integration/fixtures/blocks/core__block__overrides__deprecated-2.serialized.html diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.html b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.html new file mode 100644 index 00000000000000..5b7b2cdf95cfb8 --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.html @@ -0,0 +1 @@ + diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.json b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.json new file mode 100644 index 00000000000000..3f0292c83ac35e --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.json @@ -0,0 +1,15 @@ +[ + { + "name": "core/block", + "isValid": true, + "attributes": { + "ref": 123, + "content": { + "V98q_x": { + "content": "Some value" + } + } + }, + "innerBlocks": [] + } +] diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.parsed.json b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.parsed.json new file mode 100644 index 00000000000000..2cf2ac37974450 --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.parsed.json @@ -0,0 +1,16 @@ +[ + { + "blockName": "core/block", + "attrs": { + "ref": 123, + "overrides": { + "V98q_x": { + "content": "Some value" + } + } + }, + "innerBlocks": [], + "innerHTML": "", + "innerContent": [] + } +] diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.serialized.html b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.serialized.html new file mode 100644 index 00000000000000..3d91f47859d018 --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-1.serialized.html @@ -0,0 +1 @@ + diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.html b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.html new file mode 100644 index 00000000000000..d75c773c630efd --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.html @@ -0,0 +1 @@ + diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.json b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.json new file mode 100644 index 00000000000000..3f0292c83ac35e --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.json @@ -0,0 +1,15 @@ +[ + { + "name": "core/block", + "isValid": true, + "attributes": { + "ref": 123, + "content": { + "V98q_x": { + "content": "Some value" + } + } + }, + "innerBlocks": [] + } +] diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.parsed.json b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.parsed.json new file mode 100644 index 00000000000000..41cd1dc9afd443 --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.parsed.json @@ -0,0 +1,18 @@ +[ + { + "blockName": "core/block", + "attrs": { + "ref": 123, + "content": { + "V98q_x": { + "values": { + "content": "Some value" + } + } + } + }, + "innerBlocks": [], + "innerHTML": "", + "innerContent": [] + } +] diff --git a/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.serialized.html b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.serialized.html new file mode 100644 index 00000000000000..3d91f47859d018 --- /dev/null +++ b/test/integration/fixtures/blocks/core__block__overrides__deprecated-2.serialized.html @@ -0,0 +1 @@ + From c0440bdabc39764d3f3d1b6b8451a40f0d161e5c Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 1 Mar 2024 14:06:20 +0800 Subject: [PATCH 20/20] Use fill instead of click & type --- test/e2e/specs/editor/various/pattern-overrides.spec.js | 3 +-- 1 file changed, 1 insertion(+), 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 e5c0455708c4a9..4542e8c789ad15 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -86,8 +86,7 @@ test.describe( 'Pattern Overrides', () => { } await editorSettings .getByRole( 'textbox', { name: 'Block Name' } ) - .click(); - await page.keyboard.type( editableParagraphName ); + .fill( editableParagraphName ); await expect.poll( editor.getBlocks ).toMatchObject( [ {