From 34bc91f17e08629280598c9df39ef4ba26231e39 Mon Sep 17 00:00:00 2001 From: Staci Cooper Date: Thu, 10 Jun 2021 17:14:06 -0700 Subject: [PATCH 1/5] Do not copy unique attributes on block duplication --- packages/blocks/src/api/factory.js | 3 ++- packages/blocks/src/api/utils.js | 25 +++++++++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/blocks/src/api/factory.js b/packages/blocks/src/api/factory.js index 5b1239c46fc7cf..3f1e37a31ec37a 100644 --- a/packages/blocks/src/api/factory.js +++ b/packages/blocks/src/api/factory.js @@ -115,7 +115,8 @@ export function __experimentalCloneSanitizedBlock( { ...block.attributes, ...mergeAttributes, - } + }, + true ); return { diff --git a/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js index d6b588668b0edd..13c25700f30a36 100644 --- a/packages/blocks/src/api/utils.js +++ b/packages/blocks/src/api/utils.js @@ -237,11 +237,16 @@ export function getAccessibleBlockLabel( * Ensure attributes contains only values defined by block type, and merge * default values for missing attributes. * - * @param {string} name The block's name. - * @param {Object} attributes The block's attributes. + * @param {string} name The block's name. + * @param {Object} attributes The block's attributes. + * @param {boolean} sanitizeUniqueAttributes Whether to sanitize unique attributes. * @return {Object} The sanitized attributes. */ -export function __experimentalSanitizeBlockAttributes( name, attributes ) { +export function __experimentalSanitizeBlockAttributes( + name, + attributes, + sanitizeUniqueAttributes = false +) { // Get the type definition associated with a registered block. const blockType = getBlockType( name ); @@ -255,7 +260,19 @@ export function __experimentalSanitizeBlockAttributes( name, attributes ) { const value = attributes[ key ]; if ( undefined !== value ) { - accumulator[ key ] = value; + if ( sanitizeUniqueAttributes ) { + // Remove unique attributes and merge default values. + const unique = + schema.hasOwnProperty( 'unique' ) && schema.unique; + + if ( ! unique ) { + accumulator[ key ] = value; + } else if ( schema.hasOwnProperty( 'default' ) ) { + accumulator[ key ] = schema.default; + } + } else { + accumulator[ key ] = value; + } } else if ( schema.hasOwnProperty( 'default' ) ) { accumulator[ key ] = schema.default; } From 05c701de14d311f7d25503e307dd4c3226aa6c9f Mon Sep 17 00:00:00 2001 From: Staci Cooper Date: Thu, 10 Jun 2021 17:14:49 -0700 Subject: [PATCH 2/5] Add tests --- packages/blocks/src/api/test/factory.js | 28 +++++++++++++ packages/blocks/src/api/test/utils.js | 53 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/packages/blocks/src/api/test/factory.js b/packages/blocks/src/api/test/factory.js index fc38d753e997b6..473212d680ffa9 100644 --- a/packages/blocks/src/api/test/factory.js +++ b/packages/blocks/src/api/test/factory.js @@ -448,6 +448,34 @@ describe( 'block factory', () => { expect( clonedBlock.attributes ).toEqual( {} ); } ); + + it( 'should not duplicate unique attributes, but fallback to available defaults', () => { + registerBlockType( 'core/test-block', { + ...defaultBlockSettings, + attributes: { + uniqueAttribute: { + type: 'string', + unique: true, + }, + uniqueAttributeWithDefault: { + type: 'string', + unique: true, + default: 'default-value', + }, + }, + } ); + + const block = createBlock( 'core/test-block', { + uniqueAttribute: 'unique-value', + uniqueAttributeWithDefault: 'unique-non-default-value', + } ); + + const clonedBlock = __experimentalCloneSanitizedBlock( block, {} ); + + expect( clonedBlock.attributes ).toEqual( { + uniqueAttribueWithDefault: 'default-value', + } ); + } ); } ); describe( 'getPossibleBlockTransformations()', () => { diff --git a/packages/blocks/src/api/test/utils.js b/packages/blocks/src/api/test/utils.js index 3ffc1bf5944bc5..4a34c106040c27 100644 --- a/packages/blocks/src/api/test/utils.js +++ b/packages/blocks/src/api/test/utils.js @@ -280,6 +280,59 @@ describe( 'sanitizeBlockAttributes', () => { } ); } ); + it( 'does not strip unique attributes by default', () => { + registerBlockType( 'core/test-block', { + attributes: { + uniqueAttr: { + type: 'string', + unique: true, + }, + }, + title: 'Test block', + } ); + + const attributes = __experimentalSanitizeBlockAttributes( + 'core/test-block', + { + uniqueAttr: 'unique-value', + } + ); + + expect( attributes ).toEqual( { + uniqueAttr: 'unique-value', + } ); + } ); + + it( 'sanitizes unique attributes and falls back to defaults when sanitizeUniqueAttributes is true', () => { + registerBlockType( 'core/test-block', { + attributes: { + uniqueAttr: { + type: 'string', + unique: true, + }, + uniqueAttrWithDefault: { + type: 'string', + unique: true, + default: 'default-value', + }, + }, + title: 'Test block', + } ); + + const attributes = __experimentalSanitizeBlockAttributes( + 'core/test-block', + { + uniqueAttr: 'unique-value', + uniqueAttrWithDefault: 'unique-non-default-value', + }, + true + ); + + expect( attributes ).toEqual( { + uniqueAttrWithDefault: 'default-value', + } ); + } ); + it( 'handles node and children sources as arrays', () => { registerBlockType( 'core/test-block', { attributes: { From d11fb07e31fed0450d677b772b43caeb20e0a78e Mon Sep 17 00:00:00 2001 From: Staci Cooper Date: Thu, 10 Jun 2021 17:15:40 -0700 Subject: [PATCH 3/5] Update docs --- .../block-api/block-attributes.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/reference-guides/block-api/block-attributes.md b/docs/reference-guides/block-api/block-attributes.md index 1ace0ffac58a66..fea26f939c798e 100644 --- a/docs/reference-guides/block-api/block-attributes.md +++ b/docs/reference-guides/block-api/block-attributes.md @@ -167,6 +167,21 @@ _Example_: Extract `src` and `alt` from each image element in the block's markup // } ``` +## Unique Attributes + +The `unique` key is used to mark block attributes that should not be copied when a block is duplicated. By default all attribute values will be cloned to the duplicate block. If an attribute sets the `unique` key to `true`, its value will not be copied, and it will instead fall back to the default value if one is supplied. + +_Example_: A `resourceId` attribute is marked as `unique` so that it will not be copied to duplicate blocks. + +```js +{ + resourceId: { + type: 'string', + unique: true + } +} +``` + ## Meta (deprecated)
From 6ae7faef8c03965f7df6758257b4f294d0da975f Mon Sep 17 00:00:00 2001 From: Staci Cooper Date: Fri, 11 Jun 2021 09:59:13 -0700 Subject: [PATCH 4/5] Fix typo in test --- packages/blocks/src/api/test/factory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/blocks/src/api/test/factory.js b/packages/blocks/src/api/test/factory.js index 473212d680ffa9..91408aceb340e2 100644 --- a/packages/blocks/src/api/test/factory.js +++ b/packages/blocks/src/api/test/factory.js @@ -473,7 +473,7 @@ describe( 'block factory', () => { const clonedBlock = __experimentalCloneSanitizedBlock( block, {} ); expect( clonedBlock.attributes ).toEqual( { - uniqueAttribueWithDefault: 'default-value', + uniqueAttributeWithDefault: 'default-value', } ); } ); } ); From 9999bfc04d47a9f2d76a7ed93516cf67c43036f3 Mon Sep 17 00:00:00 2001 From: Staci Cooper Date: Thu, 8 Jul 2021 14:20:26 -0700 Subject: [PATCH 5/5] Rename key to `duplicable` and invert logic The use of `unique` implies strict uniqueness across blocks of the same type, which is not actually enforced here. For clarity the key is changed to `duplicable` and the logic inverted. --- .../block-api/block-attributes.md | 10 ++++--- packages/blocks/src/api/factory.js | 2 +- packages/blocks/src/api/test/factory.js | 14 +++++----- packages/blocks/src/api/test/utils.js | 28 +++++++++---------- packages/blocks/src/api/utils.js | 22 ++++++++------- 5 files changed, 40 insertions(+), 36 deletions(-) diff --git a/docs/reference-guides/block-api/block-attributes.md b/docs/reference-guides/block-api/block-attributes.md index fea26f939c798e..df4f3031d38d10 100644 --- a/docs/reference-guides/block-api/block-attributes.md +++ b/docs/reference-guides/block-api/block-attributes.md @@ -167,17 +167,19 @@ _Example_: Extract `src` and `alt` from each image element in the block's markup // } ``` -## Unique Attributes +## Duplicable Attributes -The `unique` key is used to mark block attributes that should not be copied when a block is duplicated. By default all attribute values will be cloned to the duplicate block. If an attribute sets the `unique` key to `true`, its value will not be copied, and it will instead fall back to the default value if one is supplied. +The `duplicable` key is used to indicate whether a block attribute should be copied when a block is duplicated. If an attribute sets the `duplicable` key to `false`, its value will not be copied, and it will instead fall back to the default value if one is supplied. If the key is not explicitly set, the attribute will be considered duplicable by default. -_Example_: A `resourceId` attribute is marked as `unique` so that it will not be copied to duplicate blocks. +The `duplicable` key should not be used for attributes that are extracted from block content using an [attribute source](#common-sources). + +_Example_: A `resourceId` attribute which should not be copied to duplicate blocks. ```js { resourceId: { type: 'string', - unique: true + duplicable: false } } ``` diff --git a/packages/blocks/src/api/factory.js b/packages/blocks/src/api/factory.js index 3f1e37a31ec37a..1d2f1f5c881927 100644 --- a/packages/blocks/src/api/factory.js +++ b/packages/blocks/src/api/factory.js @@ -116,7 +116,7 @@ export function __experimentalCloneSanitizedBlock( ...block.attributes, ...mergeAttributes, }, - true + { shouldRemoveDuplicateAttributes: true } ); return { diff --git a/packages/blocks/src/api/test/factory.js b/packages/blocks/src/api/test/factory.js index 91408aceb340e2..85cb1a8c0e022c 100644 --- a/packages/blocks/src/api/test/factory.js +++ b/packages/blocks/src/api/test/factory.js @@ -453,27 +453,27 @@ describe( 'block factory', () => { registerBlockType( 'core/test-block', { ...defaultBlockSettings, attributes: { - uniqueAttribute: { + nonDuplicableAttr: { type: 'string', - unique: true, + duplicable: false, }, - uniqueAttributeWithDefault: { + nonDuplicableAttrWithDefault: { type: 'string', - unique: true, + duplicable: false, default: 'default-value', }, }, } ); const block = createBlock( 'core/test-block', { - uniqueAttribute: 'unique-value', - uniqueAttributeWithDefault: 'unique-non-default-value', + nonDuplicableAttr: 'unique-value', + nonDuplicableAttrWithDefault: 'unique-non-default-value', } ); const clonedBlock = __experimentalCloneSanitizedBlock( block, {} ); expect( clonedBlock.attributes ).toEqual( { - uniqueAttributeWithDefault: 'default-value', + nonDuplicableAttrWithDefault: 'default-value', } ); } ); } ); diff --git a/packages/blocks/src/api/test/utils.js b/packages/blocks/src/api/test/utils.js index 4a34c106040c27..41d71b39d42e10 100644 --- a/packages/blocks/src/api/test/utils.js +++ b/packages/blocks/src/api/test/utils.js @@ -280,12 +280,12 @@ describe( 'sanitizeBlockAttributes', () => { } ); } ); - it( 'does not strip unique attributes by default', () => { + it( 'does not strip non-duplicable attributes by default', () => { registerBlockType( 'core/test-block', { attributes: { - uniqueAttr: { + nonDupicableAttr: { type: 'string', - unique: true, + duplicable: false, }, }, title: 'Test block', @@ -294,25 +294,25 @@ describe( 'sanitizeBlockAttributes', () => { const attributes = __experimentalSanitizeBlockAttributes( 'core/test-block', { - uniqueAttr: 'unique-value', + nonDupicableAttr: 'unique-value', } ); expect( attributes ).toEqual( { - uniqueAttr: 'unique-value', + nonDupicableAttr: 'unique-value', } ); } ); - it( 'sanitizes unique attributes and falls back to defaults when sanitizeUniqueAttributes is true', () => { + it( 'removes non-duplicable attributes and falls back to defaults when shouleRemoveDuplicateAttributes is true', () => { registerBlockType( 'core/test-block', { attributes: { - uniqueAttr: { + nonDuplicableAttr: { type: 'string', - unique: true, + duplicable: false, }, - uniqueAttrWithDefault: { + nonDuplicableAttrWithDefault: { type: 'string', - unique: true, + duplicable: false, default: 'default-value', }, }, @@ -322,14 +322,14 @@ describe( 'sanitizeBlockAttributes', () => { const attributes = __experimentalSanitizeBlockAttributes( 'core/test-block', { - uniqueAttr: 'unique-value', - uniqueAttrWithDefault: 'unique-non-default-value', + nonDuplicableAttr: 'unique-value', + nonDuplicableAttrWithDefault: 'unique-non-default-value', }, - true + { shouldRemoveDuplicateAttributes: true } ); expect( attributes ).toEqual( { - uniqueAttrWithDefault: 'default-value', + nonDuplicableAttrWithDefault: 'default-value', } ); } ); diff --git a/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js index 13c25700f30a36..1ac0f7563454f3 100644 --- a/packages/blocks/src/api/utils.js +++ b/packages/blocks/src/api/utils.js @@ -237,15 +237,15 @@ export function getAccessibleBlockLabel( * Ensure attributes contains only values defined by block type, and merge * default values for missing attributes. * - * @param {string} name The block's name. - * @param {Object} attributes The block's attributes. - * @param {boolean} sanitizeUniqueAttributes Whether to sanitize unique attributes. + * @param {string} name The block's name. + * @param {Object} attributes The block's attributes. + * @param {boolean} shouldRemoveDuplicateAttributes Whether to remove non-duplicable attributes. * @return {Object} The sanitized attributes. */ export function __experimentalSanitizeBlockAttributes( name, attributes, - sanitizeUniqueAttributes = false + { shouldRemoveDuplicateAttributes = false } = {} ) { // Get the type definition associated with a registered block. const blockType = getBlockType( name ); @@ -260,12 +260,14 @@ export function __experimentalSanitizeBlockAttributes( const value = attributes[ key ]; if ( undefined !== value ) { - if ( sanitizeUniqueAttributes ) { - // Remove unique attributes and merge default values. - const unique = - schema.hasOwnProperty( 'unique' ) && schema.unique; - - if ( ! unique ) { + // Remove non-duplicable attributes and merge default values. + if ( shouldRemoveDuplicateAttributes ) { + // An attribute is duplicable by default if not specified in the schema. + const duplicable = + ! schema.hasOwnProperty( 'duplicable' ) || + schema.duplicable; + + if ( duplicable ) { accumulator[ key ] = value; } else if ( schema.hasOwnProperty( 'default' ) ) { accumulator[ key ] = schema.default;