From af122579b452fe9e56aebe36a8cc68bfa44bb76f Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Wed, 24 Jan 2024 16:14:54 +0800 Subject: [PATCH 1/2] Change the overrides format to patch --- .../block-bindings/sources/pattern.php | 16 +++++- packages/block-library/src/block/edit.js | 56 ++++++++++++++++--- .../editor/various/pattern-overrides.spec.js | 4 +- 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/lib/experimental/block-bindings/sources/pattern.php b/lib/experimental/block-bindings/sources/pattern.php index 16bf46f4f06ab..f2d23cc3ce71a 100644 --- a/lib/experimental/block-bindings/sources/pattern.php +++ b/lib/experimental/block-bindings/sources/pattern.php @@ -9,8 +9,20 @@ if ( ! _wp_array_get( $block_instance->attributes, array( 'metadata', 'id' ), false ) ) { return null; } - $block_id = $block_instance->attributes['metadata']['id']; - return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, $attribute_name ), 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 + // This currently skip this attribute instead of removing it until the binding API supports different operations. + return null; + case 1: // replace + return $attribute_override[1]; + default: + return null; + } }; wp_block_bindings_register_source( 'pattern_attributes', diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index cd04e09605ff4..2d517983c3fe1 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -88,6 +88,26 @@ 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( @@ -104,9 +124,15 @@ function applyInitialOverrides( blocks, overrides = {}, defaultValues ) { defaultValues[ blockId ] ??= {}; defaultValues[ blockId ][ attributeKey ] = block.attributes[ attributeKey ]; - if ( overrides[ blockId ]?.[ attributeKey ] !== undefined ) { - newAttributes[ attributeKey ] = - overrides[ blockId ][ 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 ]; } } return { @@ -118,13 +144,14 @@ 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 ); @@ -134,10 +161,23 @@ function getOverridesFromBlocks( blocks, defaultValues ) { defaultValues[ blockId ][ attributeKey ] ) { overrides[ blockId ] ??= {}; - // 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 ]; + /** + * 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 ], + ]; + } } } } diff --git a/test/e2e/specs/editor/various/pattern-overrides.spec.js b/test/e2e/specs/editor/various/pattern-overrides.spec.js index 4e7bc80fdff5d..eea84c3f6eb94 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -176,7 +176,7 @@ test.describe( 'Pattern Overrides', () => { ref: patternId, overrides: { [ editableParagraphId ]: { - content: 'I would word it this way', + content: [ 1, 'I would word it this way' ], }, }, }, @@ -187,7 +187,7 @@ test.describe( 'Pattern Overrides', () => { ref: patternId, overrides: { [ editableParagraphId ]: { - content: 'This one is different', + content: [ 1, 'This one is different' ], }, }, }, From 0f31eafe117b2f995c9ea13aaec6647fce713ad2 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Wed, 24 Jan 2024 17:34:42 +0800 Subject: [PATCH 2/2] Add linkTarget and set it to an empty string for the remove op --- .../block-bindings/sources/pattern.php | 7 ++- lib/experimental/blocks.php | 2 +- packages/patterns/src/constants.js | 1 + .../editor/various/pattern-overrides.spec.js | 62 +++++++++++++++++++ 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/experimental/block-bindings/sources/pattern.php b/lib/experimental/block-bindings/sources/pattern.php index f2d23cc3ce71a..740d1983d6fe8 100644 --- a/lib/experimental/block-bindings/sources/pattern.php +++ b/lib/experimental/block-bindings/sources/pattern.php @@ -16,8 +16,11 @@ } switch ( $attribute_override[0] ) { case 0: // remove - // This currently skip this attribute instead of removing it until the binding API supports different operations. - return null; + /** + * 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: diff --git a/lib/experimental/blocks.php b/lib/experimental/blocks.php index 4c22601fa9102..39ac86970367c 100644 --- a/lib/experimental/blocks.php +++ b/lib/experimental/blocks.php @@ -101,7 +101,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' ), + 'core/button' => array( 'url', 'text', 'linkTarget' ), ); // If the block doesn't have the bindings property or isn't one of the allowed block types, return. diff --git a/packages/patterns/src/constants.js b/packages/patterns/src/constants.js index 5fa9ec44f4e3a..9ce2aff2fc46d 100644 --- a/packages/patterns/src/constants.js +++ b/packages/patterns/src/constants.js @@ -27,6 +27,7 @@ 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 eea84c3f6eb94..56c6b2a7c15f4 100644 --- a/test/e2e/specs/editor/various/pattern-overrides.spec.js +++ b/test/e2e/specs/editor/various/pattern-overrides.spec.js @@ -263,4 +263,66 @@ 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', '' ); + } ); } );