From 2a657f0c18379cb17a0ea8c0aa8b75df456703f9 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 31 Jan 2024 14:48:05 +0800 Subject: [PATCH] Revert "Use a patch format and support `linkTarget` of `core/button` for Pattern Overrides (#58165)" This reverts commit a0fccd15db464ee7f1769f63fe4135c19dba4890. --- .../block-bindings/sources/pattern.php | 19 +----- lib/compat/wordpress-6.5/blocks.php | 2 +- packages/block-library/src/block/edit.js | 56 +++------------- packages/patterns/src/constants.js | 1 - .../editor/various/pattern-overrides.spec.js | 66 +------------------ 5 files changed, 13 insertions(+), 131 deletions(-) diff --git a/lib/compat/wordpress-6.5/block-bindings/sources/pattern.php b/lib/compat/wordpress-6.5/block-bindings/sources/pattern.php index 503723596bf9cc..518348a43b8123 100644 --- a/lib/compat/wordpress-6.5/block-bindings/sources/pattern.php +++ b/lib/compat/wordpress-6.5/block-bindings/sources/pattern.php @@ -8,23 +8,8 @@ function gutenberg_block_bindings_pattern_overrides_callback( $source_attrs, $bl if ( ! _wp_array_get( $block_instance->attributes, array( 'metadata', 'id' ), false ) ) { return null; } - $block_id = $block_instance->attributes['metadata']['id']; - $attribute_override = _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, $attribute_name ), null ); - if ( null === $attribute_override ) { - return null; - } - switch ( $attribute_override[0] ) { - case 0: // remove - /** - * TODO: This currently doesn't remove the attribute, but only set it to an empty string. - * It's a temporary solution until the block binding API supports different operations. - */ - return ''; - case 1: // replace - return $attribute_override[1]; - default: - return null; - } + $block_id = $block_instance->attributes['metadata']['id']; + return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, $attribute_name ), null ); } function gutenberg_register_block_bindings_pattern_overrides_source() { diff --git a/lib/compat/wordpress-6.5/blocks.php b/lib/compat/wordpress-6.5/blocks.php index 6c4b604e97e158..637ebaf6483be7 100644 --- a/lib/compat/wordpress-6.5/blocks.php +++ b/lib/compat/wordpress-6.5/blocks.php @@ -163,7 +163,7 @@ function gutenberg_process_block_bindings( $block_content, $block, $block_instan 'core/paragraph' => array( 'content' ), 'core/heading' => array( 'content' ), 'core/image' => array( 'url', 'title', 'alt' ), - 'core/button' => array( 'url', 'text', 'linkTarget' ), + 'core/button' => array( 'url', 'text' ), ); // If the block doesn't have the bindings property or isn't one of the allowed block types, return. diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index c940f36e1a8b86..ae1cca181561f3 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -88,26 +88,6 @@ const useInferredLayout = ( blocks, parentLayout ) => { }, [ blocks, parentLayout ] ); }; -/** - * Enum for patch operations. - * We use integers here to minimize the size of the serialized data. - * This has to be deserialized accordingly on the server side. - * See block-bindings/sources/pattern.php - */ -const PATCH_OPERATIONS = { - /** @type {0} */ - Remove: 0, - /** @type {1} */ - Replace: 1, - // Other operations are reserved for future use. (e.g. Add) -}; - -/** - * @typedef {[typeof PATCH_OPERATIONS.Remove]} RemovePatch - * @typedef {[typeof PATCH_OPERATIONS.Replace, unknown]} ReplacePatch - * @typedef {RemovePatch | ReplacePatch} OverridePatch - */ - function applyInitialOverrides( blocks, overrides = {}, defaultValues ) { return blocks.map( ( block ) => { const innerBlocks = applyInitialOverrides( @@ -124,15 +104,9 @@ function applyInitialOverrides( blocks, overrides = {}, defaultValues ) { defaultValues[ blockId ] ??= {}; defaultValues[ blockId ][ attributeKey ] = block.attributes[ attributeKey ]; - /** @type {OverridePatch} */ - const overrideAttribute = overrides[ blockId ]?.[ attributeKey ]; - if ( ! overrideAttribute ) { - continue; - } - if ( overrideAttribute[ 0 ] === PATCH_OPERATIONS.Remove ) { - delete newAttributes[ attributeKey ]; - } else if ( overrideAttribute[ 0 ] === PATCH_OPERATIONS.Replace ) { - newAttributes[ attributeKey ] = overrideAttribute[ 1 ]; + if ( overrides[ blockId ]?.[ attributeKey ] !== undefined ) { + newAttributes[ attributeKey ] = + overrides[ blockId ][ attributeKey ]; } } return { @@ -144,14 +118,13 @@ function applyInitialOverrides( blocks, overrides = {}, defaultValues ) { } function getOverridesFromBlocks( blocks, defaultValues ) { - /** @type {Record>} */ + /** @type {Record>} */ const overrides = {}; for ( const block of blocks ) { Object.assign( overrides, getOverridesFromBlocks( block.innerBlocks, defaultValues ) ); - /** @type {string} */ const blockId = block.attributes.metadata?.id; if ( ! isPartiallySynced( block ) || ! blockId ) continue; const attributes = getPartiallySyncedAttributes( block ); @@ -161,23 +134,10 @@ function getOverridesFromBlocks( blocks, defaultValues ) { defaultValues[ blockId ][ attributeKey ] ) { overrides[ blockId ] ??= {}; - /** - * Create a patch operation for the binding attribute. - * We use a tuple here to minimize the size of the serialized data. - * The first item is the operation type, the second item is the value if any. - */ - if ( block.attributes[ attributeKey ] === undefined ) { - /** @type {RemovePatch} */ - overrides[ blockId ][ attributeKey ] = [ - PATCH_OPERATIONS.Remove, - ]; - } else { - /** @type {ReplacePatch} */ - overrides[ blockId ][ attributeKey ] = [ - PATCH_OPERATIONS.Replace, - block.attributes[ attributeKey ], - ]; - } + // TODO: We need a way to represent `undefined` in the serialized overrides. + // Also see: https://github.com/WordPress/gutenberg/pull/57249#discussion_r1452987871 + overrides[ blockId ][ attributeKey ] = + block.attributes[ attributeKey ]; } } } diff --git a/packages/patterns/src/constants.js b/packages/patterns/src/constants.js index 9ce2aff2fc46d4..5fa9ec44f4e3aa 100644 --- a/packages/patterns/src/constants.js +++ b/packages/patterns/src/constants.js @@ -27,7 +27,6 @@ export const PARTIAL_SYNCING_SUPPORTED_BLOCKS = { 'core/button': { text: __( 'Text' ), url: __( 'URL' ), - linkTarget: __( 'Link Target' ), }, 'core/image': { url: __( 'URL' ), diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 417827a9071581..ca9fbc8d07560e 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -172,7 +172,7 @@ test.describe( 'Pattern Overrides', () => { ref: patternId, overrides: { [ editableParagraphId ]: { - content: [ 1, 'I would word it this way' ], + content: 'I would word it this way', }, }, }, @@ -183,7 +183,7 @@ test.describe( 'Pattern Overrides', () => { ref: patternId, overrides: { [ editableParagraphId ]: { - content: [ 1, 'This one is different' ], + content: 'This one is different', }, }, }, @@ -259,66 +259,4 @@ test.describe( 'Pattern Overrides', () => { }, ] ); } ); - - test( 'Supports `undefined` attribute values in patterns', async ( { - page, - admin, - editor, - requestUtils, - } ) => { - const buttonId = 'button-id'; - const { id } = await requestUtils.createBlock( { - title: 'Pattern with overrides', - content: ` -
-
wp.org
-
-`, - status: 'publish', - } ); - - await admin.createNewPost(); - - await editor.insertBlock( { - name: 'core/block', - attributes: { ref: id }, - } ); - - await editor.canvas - .getByRole( 'document', { name: 'Block: Button' } ) - .getByRole( 'textbox', { name: 'Button text' } ) - .focus(); - - await expect( - page.getByRole( 'link', { name: 'wp.org' } ) - ).toContainText( 'opens in a new tab' ); - - const openInNewTabCheckbox = page.getByRole( 'checkbox', { - name: 'Open in new tab', - } ); - await expect( openInNewTabCheckbox ).toBeChecked(); - - await openInNewTabCheckbox.setChecked( false ); - - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/block', - attributes: { - ref: id, - overrides: { - [ buttonId ]: { - linkTarget: [ 0 ], - }, - }, - }, - }, - ] ); - - const postId = await editor.publishPost(); - await page.goto( `/?p=${ postId }` ); - - const link = page.getByRole( 'link', { name: 'wp.org' } ); - await expect( link ).toHaveAttribute( 'href', 'http://wp.org' ); - await expect( link ).toHaveAttribute( 'target', '' ); - } ); } );