From 8b337a01b026df1955695172a2e20456f6931d04 Mon Sep 17 00:00:00 2001 From: Ella Date: Mon, 9 Oct 2023 14:23:26 +0300 Subject: [PATCH] Remove an unmodified wrapper when all its inner blocks are removed Removing the last inner blocks of a wrapper that is otherwise unmodified removes the wrapper too, so deleting the sole list item through the block toolbar no longer leaves an empty list behind. A wrapper with any attribute set survives and keeps its empty state. Co-Authored-By: Claude Fable 5 --- .../block-editor/src/store/private-actions.js | 35 +++++++++++++++++-- test/e2e/specs/editor/blocks/list.spec.js | 35 +++++++++++-------- .../editor/various/block-deletion.spec.js | 9 +++-- .../various/multi-block-selection.spec.js | 7 ++-- 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js index b41b28d974d79e..02b7a96950e11a 100644 --- a/packages/block-editor/src/store/private-actions.js +++ b/packages/block-editor/src/store/private-actions.js @@ -1,6 +1,7 @@ import deprecated from '@wordpress/deprecated'; import { speak } from '@wordpress/a11y'; import { __ } from '@wordpress/i18n'; +import { getBlockType, isUnmodifiedBlock } from '@wordpress/blocks'; const castArray = ( maybeArray ) => Array.isArray( maybeArray ) ? maybeArray : [ maybeArray ]; @@ -163,14 +164,44 @@ export const privateRemoveBlocks = } } + // When the removal covers all the inner blocks of an otherwise + // unmodified container, remove the container as well: an empty + // husk requiring a second removal is never the intention. + let removeRoot = false; + const rootClientId = select.getBlockRootClientId( clientIds[ 0 ] ); + // A wrapper that only exists as a slot of a larger block (a column + // in columns) keeps its place: emptying it must not reshape the + // parent layout. + if ( + rootClientId && + ! getBlockType( select.getBlockName( rootClientId ) )?.parent + ?.length + ) { + const blockOrder = select.getBlockOrder( rootClientId ); + removeRoot = + blockOrder.length === clientIds.length && + blockOrder.every( ( clientId ) => + clientIds.includes( clientId ) + ) && + isUnmodifiedBlock( + select.__unstableGetBlockWithoutInnerBlocks( rootClientId ) + ); + } + if ( selectPrevious ) { - dispatch.selectPreviousBlock( clientIds[ 0 ], selectPrevious ); + dispatch.selectPreviousBlock( + removeRoot ? rootClientId : clientIds[ 0 ], + selectPrevious + ); } // We're batching these two actions because an extra `undo/redo` step can // be created, based on whether we insert a default block or not. registry.batch( () => { - dispatch( { type: 'REMOVE_BLOCKS', clientIds } ); + dispatch( { + type: 'REMOVE_BLOCKS', + clientIds: removeRoot ? [ rootClientId ] : clientIds, + } ); // To avoid a focus loss when removing the last block, assure there is // always a default block if the last of the blocks have been removed. dispatch( ensureDefaultBlock() ); diff --git a/test/e2e/specs/editor/blocks/list.spec.js b/test/e2e/specs/editor/blocks/list.spec.js index e891efab27f78c..f692b003d68a01 100644 --- a/test/e2e/specs/editor/blocks/list.spec.js +++ b/test/e2e/specs/editor/blocks/list.spec.js @@ -1268,6 +1268,21 @@ test.describe( 'List (@firefox)', () => { ); } ); + test( 'remove empty list graciously through UI', async ( { + editor, + page, + } ) => { + await editor.canvas + .locator( 'role=document[name="Add default block"i]' ) + .click(); + await page.keyboard.type( '* 1' ); + + await editor.clickBlockToolbarButton( 'Options' ); + await page.getByRole( 'menuitem', { name: 'Delete' } ).click(); + + expect( await editor.getEditedPostContent() ).toBe( '' ); + } ); + test( 'should not change the contents when you change the list type to Ordered', async ( { editor, page, @@ -2159,14 +2174,10 @@ test.describe( 'List (@firefox)', () => { editor.canvas.locator( '.is-multi-selected' ) ).toHaveCount( 2 ); - // Both items, including the nested one, are removed. + // Both items, including the nested one, are removed, and with + // them the emptied list itself. await page.keyboard.press( 'Backspace' ); - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/list', - innerBlocks: [], - }, - ] ); + await expect.poll( editor.getBlocks ).toEqual( [] ); } ); test( 'should multi-select the top level items when extending a selection from a nested item to the previous top level item', async ( { @@ -2226,14 +2237,10 @@ test.describe( 'List (@firefox)', () => { '2 blocks selected.' ); - // Both items, including the nested one, are removed. + // Both items, including the nested one, are removed, and with + // them the emptied list itself. await page.keyboard.press( 'Backspace' ); - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/list', - innerBlocks: [], - }, - ] ); + await expect.poll( editor.getBlocks ).toEqual( [] ); } ); test( 'should merge a following paragraph into the outermost list with Delete from a nested item (#77245)', async ( { diff --git a/test/e2e/specs/editor/various/block-deletion.spec.js b/test/e2e/specs/editor/various/block-deletion.spec.js index d8c47df159e8e7..f74c2b2e47d866 100644 --- a/test/e2e/specs/editor/various/block-deletion.spec.js +++ b/test/e2e/specs/editor/various/block-deletion.spec.js @@ -64,9 +64,12 @@ test.describe( 'Block deletion', () => { editor, page, } ) => { - // Add a group with a paragraph in it. + // Add a group with a paragraph in it. The anchor keeps the group + // from being removed along with its last child: emptying an + // unmodified wrapper removes the wrapper too. await editor.insertBlock( { name: 'core/group', + attributes: { anchor: 'group' }, innerBlocks: [ { name: 'core/paragraph', @@ -92,7 +95,9 @@ test.describe( 'Block deletion', () => { // Ensure the paragraph was removed. await expect .poll( editor.getBlocks ) - .toMatchObject( [ { name: 'core/group', attributes: {} } ] ); + .toMatchObject( [ + { name: 'core/group', attributes: { anchor: 'group' } }, + ] ); // Ensure the group block is focused. await expect( diff --git a/test/e2e/specs/editor/various/multi-block-selection.spec.js b/test/e2e/specs/editor/various/multi-block-selection.spec.js index 6946791fd22867..87a19deb3dc51f 100644 --- a/test/e2e/specs/editor/various/multi-block-selection.spec.js +++ b/test/e2e/specs/editor/various/multi-block-selection.spec.js @@ -1038,10 +1038,9 @@ test.describe( 'Multi-block selection (@firefox, @webkit)', () => { await page.keyboard.press( 'Backspace' ); - // Expect both columns to be deleted. - await expect - .poll( editor.getBlocks ) - .toMatchObject( [ { name: 'core/columns', innerBlocks: [] } ] ); + // Expect the columns block to be deleted along with its emptied + // columns. + await expect.poll( editor.getBlocks ).toEqual( [] ); } ); test( 'should multi-select from within the list block', async ( {