From 91dcb1693ea904fe1bbc81ddfb0c64ce20d8c694 Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Fri, 13 Sep 2024 19:23:04 +0800 Subject: [PATCH] Try another approach --- .../src/components/block-list/block.js | 96 ++++++++----- packages/blocks/src/api/test/utils.js | 52 ------- test/e2e/specs/editor/blocks/list.spec.js | 23 ++-- .../editor/various/splitting-merging.spec.js | 129 +++++++++++------- 4 files changed, 156 insertions(+), 144 deletions(-) diff --git a/packages/block-editor/src/components/block-list/block.js b/packages/block-editor/src/components/block-list/block.js index 3b6486138a6e9..843ab752d9096 100644 --- a/packages/block-editor/src/components/block-list/block.js +++ b/packages/block-editor/src/components/block-list/block.js @@ -24,6 +24,7 @@ import { isReusableBlock, getBlockDefaultClassName, hasBlockSupport, + createBlock, __experimentalGetBlockAttributesNamesByRole, store as blocksStore, privateApis as blocksPrivateApis, @@ -311,6 +312,41 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { canInsertBlockType, } = registry.select( blockEditorStore ); + function isBlockEmpty( block ) { + const blockType = getBlockType( block.name ); + const contentAttributes = + __experimentalGetBlockAttributesNamesByRole( + block.name, + 'content' + ); + return contentAttributes.every( ( attribute ) => { + const definition = blockType.attributes[ attribute ]; + const value = block.attributes[ attribute ]; + return isAttributeUnmodified( definition, value ); + } ); + } + + /** + * Update the attributes to reset the block to the default state. + * @param {WPBlock} block The block to reset. + */ + function resetBlockToDefault( block ) { + // Instead of replacing the block, only update the default attributes + // to keep any metadata intact. + const defaultBlockType = getBlockType( block.name ); + const defaultBlockAttributes = createBlock( + block.name + ).attributes; + for ( const key of Object.keys( + defaultBlockType.attributes + ) ) { + if ( ! defaultBlockAttributes.hasOwnProperty( key ) ) { + defaultBlockAttributes[ key ] = undefined; + } + } + updateBlockAttributes( block.clientId, defaultBlockAttributes ); + } + function switchToDefaultOrRemove() { const block = getBlock( clientId ); const defaultBlockName = getDefaultBlockName(); @@ -333,26 +369,6 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { } } - /** - * Return true if the block content is empty. - * @param {string} blockClientId The block client ID. - * @return {boolean} Whether the block content is empty. - */ - function isBlockContentEmpty( blockClientId ) { - const block = getBlock( blockClientId ); - const blockType = getBlockType( block.name ); - const contentAttributes = - __experimentalGetBlockAttributesNamesByRole( - block.name, - 'content' - ); - return contentAttributes.every( ( key ) => { - const value = block.attributes[ key ]; - const definition = blockType.attributes[ key ]; - return isAttributeUnmodified( definition, value ); - } ); - } - /** * Moves the block with clientId up one level. If the block type * cannot be inserted at the new location, it will be attempted to @@ -374,16 +390,16 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { removeBlock( _clientId ); } else { registry.batch( () => { + const firstBlock = getBlock( firstClientId ); + const isFirstBlockEmpty = isBlockEmpty( firstBlock ); + + // Step 1: Lift the block to the parent if it's not "empty". if ( canInsertBlockType( getBlockName( firstClientId ), 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 ) + ! isFirstBlockEmpty ) { moveBlocksToPosition( [ firstClientId ], @@ -392,21 +408,22 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { getBlockIndex( _clientId ) ); } else { + const defaultBlockName = getDefaultBlockName(); const replacement = switchToBlockType( - getBlock( firstClientId ), - getDefaultBlockName() + firstBlock, + defaultBlockName ); - - if ( - replacement && - replacement.length && + const canTransformToDefaultBlock = + !! replacement?.length && replacement.every( ( block ) => canInsertBlockType( block.name, targetRootClientId ) - ) - ) { + ); + + // Step 2: Else, attempt to convert to the default block type. + if ( canTransformToDefaultBlock ) { insertBlocks( replacement, getBlockIndex( _clientId ), @@ -414,7 +431,18 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { changeSelection ); removeBlock( firstClientId, false ); - } else { + } + // Step 3: If the block is a default block but modified and "empty", + // transform it into an unmodified block. + else if ( + firstBlock.name === defaultBlockName && + ! isUnmodifiedDefaultBlock( firstBlock ) && + isFirstBlockEmpty + ) { + resetBlockToDefault( firstBlock ); + } + // Step 4: Otherwise, continue the default behavior. + else { switchToDefaultOrRemove(); } } diff --git a/packages/blocks/src/api/test/utils.js b/packages/blocks/src/api/test/utils.js index bd2e2dbdb2538..9bfef69c4c142 100644 --- a/packages/blocks/src/api/test/utils.js +++ b/packages/blocks/src/api/test/utils.js @@ -14,7 +14,6 @@ import { getBlockLabel, __experimentalSanitizeBlockAttributes, __experimentalGetBlockAttributesNamesByRole, - isAttributeUnmodified, } from '../utils'; const noop = () => {}; @@ -397,54 +396,3 @@ describe( '__experimentalGetBlockAttributesNamesByRole', () => { ).toEqual( [] ); } ); } ); - -describe( 'isAttributeUnmodified', () => { - it( 'should return true if the block is unmodified', () => { - 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 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/test/e2e/specs/editor/blocks/list.spec.js b/test/e2e/specs/editor/blocks/list.spec.js index 12a6d51172822..16126cf9cd29f 100644 --- a/test/e2e/specs/editor/blocks/list.spec.js +++ b/test/e2e/specs/editor/blocks/list.spec.js @@ -1237,18 +1237,17 @@ test.describe( 'List (@firefox)', () => { await page.keyboard.press( 'Backspace' ); await page.keyboard.press( 'Backspace' ); - await expect.poll( editor.getBlocks ).toMatchObject( [ - { name: 'core/paragraph', attributes: { content: '' } }, - { - name: 'core/list', - innerBlocks: [ - { - name: 'core/list-item', - attributes: { content: '2' }, - }, - ], - }, - ] ); + await expect.poll( editor.getEditedPostContent ).toBe( + ` +

+ + + + +` + ); } ); test( 'should not change the contents when you change the list type to Ordered', async ( { diff --git a/test/e2e/specs/editor/various/splitting-merging.spec.js b/test/e2e/specs/editor/various/splitting-merging.spec.js index d7bcb6c01b0c3..f3db190df1dd9 100644 --- a/test/e2e/specs/editor/various/splitting-merging.spec.js +++ b/test/e2e/specs/editor/various/splitting-merging.spec.js @@ -374,24 +374,38 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { } ); // Fix for https://github.com/WordPress/gutenberg/issues/65174. - test( 'should not merge blocks with empty contents', async ( { + test( 'should handle unwrapping and merging blocks', async ( { editor, page, } ) => { + const defaultUnmodifiedBlock = { + name: 'core/paragraph', + attributes: { content: '', align: undefined, dropCap: false }, + innerBlocks: [], + }; + const defaultEmptyBlock = { + name: 'core/paragraph', + attributes: { content: '', align: 'center', dropCap: false }, + innerBlocks: [], + }; + const nonDefaultEmptyBlock = { + name: 'core/heading', + attributes: { content: '', textAlign: 'center', level: 2 }, + innerBlocks: [], + }; + const nonDefaultBlockWithContent = { + name: 'core/heading', + attributes: { content: 'heading', level: 2 }, + innerBlocks: [], + }; + const placeholderBlock = { name: 'core/separator' }; await editor.insertBlock( { name: 'core/group', innerBlocks: [ - // Unmodified block. - { name: 'core/paragraph', attributes: { content: '' } }, - // An "empty" but modified block. - { - name: 'core/heading', - attributes: { content: '', textAlign: 'center' }, - }, - // A non-empty block. - { name: 'core/paragraph', attributes: { content: 'p' } }, - // Just a placeholder. - { name: 'core/spacer' }, + defaultEmptyBlock, + nonDefaultEmptyBlock, + nonDefaultBlockWithContent, + placeholderBlock, ], } ); await editor.canvas @@ -400,70 +414,93 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { await page.keyboard.press( 'Backspace' ); await expect - .poll( - editor.getBlocks, - 'Pressing backspace should remove the unmodified block' - ) - .toMatchObject( [ + .poll( editor.getBlocks, 'Reset the default empty block' ) + .toEqual( [ { name: 'core/group', + attributes: { tagName: 'div' }, innerBlocks: [ - { - name: 'core/heading', - attributes: { content: '', textAlign: 'center' }, - }, - { - name: 'core/paragraph', - attributes: { content: 'p' }, - }, - { name: 'core/spacer' }, + defaultUnmodifiedBlock, + nonDefaultEmptyBlock, + nonDefaultBlockWithContent, + expect.objectContaining( placeholderBlock ), + ], + }, + ] ); + await page.keyboard.press( 'Backspace' ); + await expect + .poll( editor.getBlocks, 'Remove the default unmodified block' ) + .toEqual( [ + { + name: 'core/group', + attributes: { tagName: 'div' }, + innerBlocks: [ + nonDefaultEmptyBlock, + nonDefaultBlockWithContent, + expect.objectContaining( placeholderBlock ), ], }, ] ); - // Move the caret to the beginning of the heading block. + // The caret is at the beginning of the empty heading block. await page.keyboard.press( 'Backspace' ); await expect .poll( editor.getBlocks, - 'Pressing backspace should still transform the empty block' + 'Lift and transform the non-default block to a default block' ) - .toMatchObject( [ - { - name: 'core/paragraph', - attributes: { content: '', align: 'center' }, - }, + .toEqual( [ + defaultEmptyBlock, { name: 'core/group', + attributes: { tagName: 'div' }, innerBlocks: [ - { - name: 'core/paragraph', - attributes: { content: 'p' }, - }, - { name: 'core/spacer' }, + nonDefaultBlockWithContent, + expect.objectContaining( placeholderBlock ), ], }, ] ); - // Move the caret to the beginning of the "p" paragraph. + // Move the caret to the beginning of the "heading" heading block. await page.keyboard.press( 'ArrowDown' ); await page.keyboard.press( 'ArrowDown' ); - await page.keyboard.press( 'ArrowLeft' ); + await page.keyboard.press( 'Home' ); await page.keyboard.press( 'Backspace' ); await expect - .poll( editor.getBlocks, 'Should "unwrap" a non-empty block' ) - .toMatchObject( [ + .poll( editor.getBlocks, 'Lift the non-empty non-default block' ) + .toEqual( [ + defaultEmptyBlock, + nonDefaultBlockWithContent, { - name: 'core/paragraph', - attributes: { content: '', align: 'center' }, + name: 'core/group', + attributes: { tagName: 'div' }, + innerBlocks: [ + expect.objectContaining( placeholderBlock ), + ], }, + ] ); + await page.keyboard.press( 'Backspace' ); + await expect + .poll( + editor.getBlocks, + 'Merge the non-empty non-default block with the default block' + ) + .toEqual( [ { name: 'core/paragraph', - attributes: { content: 'p' }, + attributes: { + content: 'heading', + align: 'center', + dropCap: false, + }, + innerBlocks: [], }, { name: 'core/group', - innerBlocks: [ { name: 'core/spacer' } ], + attributes: { tagName: 'div' }, + innerBlocks: [ + expect.objectContaining( placeholderBlock ), + ], }, ] ); } );