From 7ed5e9cb4fe316a7aecc5a726ca7e31012008cc9 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Fri, 13 Sep 2024 17:43:17 +0800 Subject: [PATCH] Convert it to a separate util --- .../src/components/block-list/block.js | 19 +++-- packages/blocks/README.md | 1 - packages/blocks/src/api/index.js | 2 + packages/blocks/src/api/test/utils.js | 76 +++++++++++-------- packages/blocks/src/api/utils.js | 58 +++++++------- .../editor/various/splitting-merging.spec.js | 4 +- 6 files changed, 94 insertions(+), 66 deletions(-) diff --git a/packages/block-editor/src/components/block-list/block.js b/packages/block-editor/src/components/block-list/block.js index 8b3ec48b68ff96..3b6486138a6e9b 100644 --- a/packages/block-editor/src/components/block-list/block.js +++ b/packages/block-editor/src/components/block-list/block.js @@ -26,6 +26,7 @@ import { hasBlockSupport, __experimentalGetBlockAttributesNamesByRole, store as blocksStore, + privateApis as blocksPrivateApis, } from '@wordpress/blocks'; import { withFilters } from '@wordpress/components'; import { withDispatch, useDispatch, useSelect } from '@wordpress/data'; @@ -47,6 +48,8 @@ import { PrivateBlockContext } from './private-block-context'; import { unlock } from '../../lock-unlock'; +const { isAttributeUnmodified } = unlock( blocksPrivateApis ); + /** * Merges wrapper props with special handling for classNames and styles. * @@ -336,16 +339,18 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { * @return {boolean} Whether the block content is empty. */ function isBlockContentEmpty( blockClientId ) { - const blockName = getBlockName( blockClientId ); + const block = getBlock( blockClientId ); + const blockType = getBlockType( block.name ); const contentAttributes = __experimentalGetBlockAttributesNamesByRole( - blockName, + block.name, 'content' ); - return isUnmodifiedBlock( - getBlock( blockClientId ), - contentAttributes - ); + return contentAttributes.every( ( key ) => { + const value = block.attributes[ key ]; + const definition = blockType.attributes[ key ]; + return isAttributeUnmodified( definition, value ); + } ); } /** @@ -375,6 +380,8 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { targetRootClientId ) && // Don't move the block if its content is considered empty. + // Note that for a modified non-empty block, we still try to transform + // it to a default block. // Fix for https://github.com/WordPress/gutenberg/issues/65174. ! isBlockContentEmpty( firstClientId ) ) { diff --git a/packages/blocks/README.md b/packages/blocks/README.md index 760fe1c7b5ea6a..d724f986b0ca81 100644 --- a/packages/blocks/README.md +++ b/packages/blocks/README.md @@ -355,7 +355,6 @@ Determines whether the block's attributes are equal to the default attributes wh _Parameters_ - _block_ `WPBlock`: Block Object -- _attributeKeys_ `?string[]`: The optional keys of the attributes to check. _Returns_ diff --git a/packages/blocks/src/api/index.js b/packages/blocks/src/api/index.js index 803467cb2187e2..251efb2520420e 100644 --- a/packages/blocks/src/api/index.js +++ b/packages/blocks/src/api/index.js @@ -8,6 +8,7 @@ import { getBlockBindingsSource, getBlockBindingsSources, } from './registration'; +import { isAttributeUnmodified } from './utils'; // The blocktype is the most important concept within the block API. It defines // all aspects of the block configuration and its interfaces, including `edit` @@ -182,4 +183,5 @@ lock( privateApis, { unregisterBlockBindingsSource, getBlockBindingsSource, getBlockBindingsSources, + isAttributeUnmodified, } ); diff --git a/packages/blocks/src/api/test/utils.js b/packages/blocks/src/api/test/utils.js index 86347cab4fb1e2..bd2e2dbdb2538b 100644 --- a/packages/blocks/src/api/test/utils.js +++ b/packages/blocks/src/api/test/utils.js @@ -9,12 +9,12 @@ import { setDefaultBlockName, } from '../registration'; import { - isUnmodifiedBlock, isUnmodifiedDefaultBlock, getAccessibleBlockLabel, getBlockLabel, __experimentalSanitizeBlockAttributes, __experimentalGetBlockAttributesNamesByRole, + isAttributeUnmodified, } from '../utils'; const noop = () => {}; @@ -398,37 +398,53 @@ describe( '__experimentalGetBlockAttributesNamesByRole', () => { } ); } ); -describe( 'isUnmodifiedBlock', () => { - beforeAll( () => { - registerBlockType( 'core/test-block', { - title: 'Test block', - attributes: { - content: { - type: 'rich-text', - __experimentalRole: 'content', - }, - placeholder: { type: 'rich-text' }, - }, - } ); - } ); - - afterAll( () => { - unregisterBlockType( 'core/test-block' ); - } ); - +describe( 'isAttributeUnmodified', () => { it( 'should return true if the block is unmodified', () => { - const block = createBlock( 'core/test-block' ); - expect( isUnmodifiedBlock( block ) ).toBe( true ); - - block.attributes.content = 'modified'; - expect( isUnmodifiedBlock( block ) ).toBe( false ); + expect( + isAttributeUnmodified( + { type: 'rich-text', __experimentalRole: 'content' }, + '' + ) + ).toBe( true ); + expect( + isAttributeUnmodified( + { type: 'rich-text', __experimentalRole: 'content' }, + undefined + ) + ).toBe( true ); + expect( isAttributeUnmodified( { type: 'string' }, undefined ) ).toBe( + true + ); + expect( + isAttributeUnmodified( + { type: 'string', default: 'default-value' }, + 'default-value' + ) + ).toBe( true ); } ); - it( 'should only check the attribute keys', () => { - const block = createBlock( 'core/test-block', { - placeholder: 'Type here...', - } ); - expect( isUnmodifiedBlock( block ) ).toBe( false ); - expect( isUnmodifiedBlock( block, [ 'content' ] ) ).toBe( true ); + it( 'should return false if the block is modified', () => { + expect( + isAttributeUnmodified( + { type: 'rich-text', __experimentalRole: 'content' }, + 'something else' + ) + ).toBe( false ); + expect( + isAttributeUnmodified( { type: 'string' }, 'something else' ) + ).toBe( false ); + expect( isAttributeUnmodified( { type: 'string' }, '' ) ).toBe( false ); + expect( + isAttributeUnmodified( + { type: 'string', default: 'default-value' }, + '' + ) + ).toBe( false ); + expect( + isAttributeUnmodified( + { type: 'string', default: 'default-value' }, + undefined + ) + ).toBe( false ); } ); } ); diff --git a/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js index 696130ac061f8e..8034071b2d1a4f 100644 --- a/packages/blocks/src/api/utils.js +++ b/packages/blocks/src/api/utils.js @@ -29,42 +29,46 @@ extend( [ namesPlugin, a11yPlugin ] ); */ const ICON_COLORS = [ '#191e23', '#f8f9f9' ]; +/** + * Determines whether the block's attribute is equal to the default attribute + * which means the attribute is unmodified. + * @param {Object} attributeDefinition The attribute's definition of the block type. + * @param {*} value The attribute's value. + * @return {boolean} Whether the attribute is unmodified. + */ +export function isAttributeUnmodified( attributeDefinition, value ) { + // Every attribute that has a default must match the default. + if ( attributeDefinition.hasOwnProperty( 'default' ) ) { + return value === attributeDefinition.default; + } + + // The rich text type is a bit different from the rest because it + // has an implicit default value of an empty RichTextData instance, + // so check the length of the value. + if ( attributeDefinition.type === 'rich-text' ) { + return ! value?.length; + } + + // Every attribute that doesn't have a default should be undefined. + return value === undefined; +} + /** * Determines whether the block's attributes are equal to the default attributes * which means the block is unmodified. * - * @param {WPBlock} block Block Object - * @param {?string[]} attributeKeys The optional keys of the attributes to check. + * @param {WPBlock} block Block Object * * @return {boolean} Whether the block is an unmodified block. */ -export function isUnmodifiedBlock( block, attributeKeys ) { - let attributeEntries = Object.entries( - getBlockType( block.name )?.attributes ?? {} - ); - if ( attributeKeys ) { - attributeEntries = attributeEntries.filter( ( [ key ] ) => - attributeKeys.includes( key ) - ); - } - return attributeEntries.every( ( [ key, definition ] ) => { - const value = block.attributes[ key ]; +export function isUnmodifiedBlock( block ) { + return Object.entries( getBlockType( block.name )?.attributes ?? {} ).every( + ( [ key, definition ] ) => { + const value = block.attributes[ key ]; - // Every attribute that has a default must match the default. - if ( definition.hasOwnProperty( 'default' ) ) { - return value === definition.default; + return isAttributeUnmodified( definition, value ); } - - // The rich text type is a bit different from the rest because it - // has an implicit default value of an empty RichTextData instance, - // so check the length of the value. - if ( definition.type === 'rich-text' ) { - return ! value?.length; - } - - // Every attribute that doesn't have a default should be undefined. - return value === undefined; - } ); + ); } /** diff --git a/test/e2e/specs/editor/various/splitting-merging.spec.js b/test/e2e/specs/editor/various/splitting-merging.spec.js index 2e5595d5c02056..d7bcb6c01b0c32 100644 --- a/test/e2e/specs/editor/various/splitting-merging.spec.js +++ b/test/e2e/specs/editor/various/splitting-merging.spec.js @@ -421,12 +421,12 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { }, ] ); - // Move the caret is moved to the beginning of the heading block. + // Move the caret to the beginning of the heading block. await page.keyboard.press( 'Backspace' ); await expect .poll( editor.getBlocks, - 'Pressing backspace should transform the non-default empty block' + 'Pressing backspace should still transform the empty block' ) .toMatchObject( [ {