diff --git a/packages/block-editor/src/components/block-list/block.js b/packages/block-editor/src/components/block-list/block.js index 3b6486138a6e9b..ab59c69c9c2de8 100644 --- a/packages/block-editor/src/components/block-list/block.js +++ b/packages/block-editor/src/components/block-list/block.js @@ -24,9 +24,7 @@ import { isReusableBlock, getBlockDefaultClassName, hasBlockSupport, - __experimentalGetBlockAttributesNamesByRole, store as blocksStore, - privateApis as blocksPrivateApis, } from '@wordpress/blocks'; import { withFilters } from '@wordpress/components'; import { withDispatch, useDispatch, useSelect } from '@wordpress/data'; @@ -48,8 +46,6 @@ 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. * @@ -333,26 +329,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 +350,30 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { removeBlock( _clientId ); } else { registry.batch( () => { - 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 ) + const replacement = switchToBlockType( + getBlock( firstClientId ), + getDefaultBlockName() + ); + const canTransformToDefaultBlock = + !! replacement?.length && + replacement.every( ( block ) => + canInsertBlockType( + block.name, + targetRootClientId + ) + ); + + if ( canTransformToDefaultBlock ) { + insertBlocks( + replacement, + getBlockIndex( _clientId ), + targetRootClientId, + changeSelection + ); + removeBlock( firstClientId, false ); + } else if ( + getBlockName( firstClientId ) === + getDefaultBlockName() ) { moveBlocksToPosition( [ firstClientId ], @@ -392,31 +382,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, registry ) => { getBlockIndex( _clientId ) ); } else { - const replacement = switchToBlockType( - getBlock( firstClientId ), - getDefaultBlockName() - ); - - if ( - replacement && - replacement.length && - replacement.every( ( block ) => - canInsertBlockType( - block.name, - targetRootClientId - ) - ) - ) { - insertBlocks( - replacement, - getBlockIndex( _clientId ), - targetRootClientId, - changeSelection - ); - removeBlock( firstClientId, false ); - } else { - switchToDefaultOrRemove(); - } + switchToDefaultOrRemove(); } if ( diff --git a/packages/blocks/src/api/index.js b/packages/blocks/src/api/index.js index 251efb2520420e..803467cb2187e2 100644 --- a/packages/blocks/src/api/index.js +++ b/packages/blocks/src/api/index.js @@ -8,7 +8,6 @@ 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` @@ -183,5 +182,4 @@ 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 bd2e2dbdb2538b..9bfef69c4c1428 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/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js index 8034071b2d1a4f..a68937586f9273 100644 --- a/packages/blocks/src/api/utils.js +++ b/packages/blocks/src/api/utils.js @@ -29,30 +29,6 @@ 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. @@ -66,7 +42,20 @@ export function isUnmodifiedBlock( block ) { ( [ key, definition ] ) => { const value = block.attributes[ key ]; - return isAttributeUnmodified( definition, value ); + // Every attribute that has a default must match the default. + if ( definition.hasOwnProperty( 'default' ) ) { + return value === definition.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 ( 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/blocks/list.spec.js b/test/e2e/specs/editor/blocks/list.spec.js index 12a6d511728228..16126cf9cd29f6 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 d7bcb6c01b0c32..2442df5cc76dc4 100644 --- a/test/e2e/specs/editor/various/splitting-merging.spec.js +++ b/test/e2e/specs/editor/various/splitting-merging.spec.js @@ -374,22 +374,25 @@ 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 not merge blocks with non-default block', async ( { editor, page, } ) => { await editor.insertBlock( { name: 'core/group', innerBlocks: [ - // Unmodified block. - { name: 'core/paragraph', attributes: { content: '' } }, - // An "empty" but modified block. + // A default empty block. + { + name: 'core/paragraph', + attributes: { content: '', align: 'center' }, + }, + // A non-default empty block. { name: 'core/heading', attributes: { content: '', textAlign: 'center' }, }, - // A non-empty block. - { name: 'core/paragraph', attributes: { content: 'p' } }, + // A non-default block with content. + { name: 'core/heading', attributes: { content: 'heading' } }, // Just a placeholder. { name: 'core/spacer' }, ], @@ -400,11 +403,12 @@ 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' - ) + .poll( editor.getBlocks, 'Lift the default block' ) .toMatchObject( [ + { + name: 'core/paragraph', + attributes: { content: '', align: 'center' }, + }, { name: 'core/group', innerBlocks: [ @@ -413,8 +417,8 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { attributes: { content: '', textAlign: 'center' }, }, { - name: 'core/paragraph', - attributes: { content: 'p' }, + name: 'core/heading', + attributes: { content: 'heading' }, }, { name: 'core/spacer' }, ], @@ -422,13 +426,19 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { ] ); // Move the caret to the beginning of the heading block. + await page.keyboard.press( 'ArrowDown' ); + await page.keyboard.press( 'ArrowDown' ); 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' }, + }, { name: 'core/paragraph', attributes: { content: '', align: 'center' }, @@ -437,21 +447,24 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { name: 'core/group', innerBlocks: [ { - name: 'core/paragraph', - attributes: { content: 'p' }, + name: 'core/heading', + attributes: { content: 'heading' }, }, { name: 'core/spacer' }, ], }, ] ); - // 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' ) + .poll( + editor.getBlocks, + 'Lift and transform the non-empty non-default block' + ) .toMatchObject( [ { name: 'core/paragraph', @@ -459,8 +472,9 @@ test.describe( 'splitting and merging blocks (@firefox, @webkit)', () => { }, { name: 'core/paragraph', - attributes: { content: 'p' }, + attributes: { content: '', align: 'center' }, }, + { name: 'core/paragraph', attributes: { content: 'heading' } }, { name: 'core/group', innerBlocks: [ { name: 'core/spacer' } ],