Focus 1st parent block on block remove, if no previous block is avail…

…able (#48204) * change default selection on remove blocks action to include selecting first available parent if there is no previous block * revert renaming the selectPreviousBlock API and add a param to also select the parent when needed * Fix removeBlock call in list item block * updates remove block test to account for new selectPreviousBlock param * Try to fix editing-widgets e2e test * focus back on widget body * Addressed feedback for documentation being more explanative Co-authored-by: Dave Smith <444434+getdave@users.noreply.github.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> * remove superfluous fix attewmpt * adds a test for parent selection by default --------- Co-authored-by: Ella van Durpe <ella@vandurpe.com> Co-authored-by: Tetsuaki Hamano <tetsuaki.hamano@gmail.com> Co-authored-by: Dave Smith <444434+getdave@users.noreply.github.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
WordPress · Mar 1, 2023 · 65794bf · 65794bf
1 parent 58809f8
commit 65794bf
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 9 deletions.
diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md
@@ -1496,11 +1496,13 @@ _Parameters_
 ### selectPreviousBlock
 
 Yields action objects used in signalling that the block preceding the given
-clientId should be selected.
+clientId (or optionally, its first parent from bottom to top)
+should be selected.
 
 _Parameters_
 
 -   _clientId_ `string`: Block client ID.
+-   _orFirstParent_ `boolean`: If true, select the first parent if there is no previous block.
 
 ### setBlockMovingClientId
 

diff --git a/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js b/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js
@@ -147,7 +147,9 @@ export function BlockSettingsDropdown( {
 		__experimentalSelectBlock
 			? () => {
 					const blockToSelect =
-						previousBlockClientId || nextBlockClientId;
+						previousBlockClientId ||
+						nextBlockClientId ||
+						firstParentClientId;
 
 					if (
 						blockToSelect &&
@@ -166,6 +168,7 @@ export function BlockSettingsDropdown( {
 			__experimentalSelectBlock,
 			previousBlockClientId,
 			nextBlockClientId,
+			firstParentClientId,
 			selectedBlockClientIds,
 		]
 	);

diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js
@@ -232,17 +232,24 @@ export function selectBlock( clientId, initialPosition = 0 ) {
 
 /**
  * Yields action objects used in signalling that the block preceding the given
- * clientId should be selected.
+ * clientId (or optionally, its first parent from bottom to top)
+ * should be selected.
  *
- * @param {string} clientId Block client ID.
+ * @param {string}  clientId      Block client ID.
+ * @param {boolean} orFirstParent If true, select the first parent if there is no previous block.
  */
 export const selectPreviousBlock =
-	( clientId ) =>
+	( clientId, orFirstParent = false ) =>
 	( { select, dispatch } ) => {
 		const previousBlockClientId =
 			select.getPreviousBlockClientId( clientId );
 		if ( previousBlockClientId ) {
 			dispatch.selectBlock( previousBlockClientId, -1 );
+		} else if ( orFirstParent ) {
+			const firstParentClientId = select.getBlockRootClientId( clientId );
+			if ( firstParentClientId ) {
+				dispatch.selectBlock( firstParentClientId, -1 );
+			}
 		}
 	};
 
@@ -1197,7 +1204,8 @@ export const removeBlocks =
 		}
 
 		if ( selectPrevious ) {
-			dispatch.selectPreviousBlock( clientIds[ 0 ] );
+			const shouldSelectParent = true;
+			dispatch.selectPreviousBlock( clientIds[ 0 ], shouldSelectParent );
 		}
 
 		dispatch( { type: 'REMOVE_BLOCKS', clientIds } );

diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js
@@ -625,7 +625,8 @@ describe( 'actions', () => {
 			removeBlocks( clientIds )( { select, dispatch } );
 
 			expect( dispatch.selectPreviousBlock ).toHaveBeenCalledWith(
-				clientId
+				clientId,
+				true
 			);
 
 			expect( dispatch ).toHaveBeenCalledWith( {
@@ -734,7 +735,8 @@ describe( 'actions', () => {
 			removeBlock( clientId )( { select, dispatch } );
 
 			expect( dispatch.selectPreviousBlock ).toHaveBeenCalledWith(
-				clientId
+				clientId,
+				true
 			);
 
 			expect( dispatch ).toHaveBeenCalledWith( {

diff --git a/packages/block-library/src/list-item/hooks/use-outdent-list-item.js b/packages/block-library/src/list-item/hooks/use-outdent-list-item.js
@@ -112,7 +112,8 @@ export default function useOutdentListItem( clientId ) {
 					getBlockIndex( parentListItemId ) + 1
 				);
 				if ( ! getBlockOrder( parentListId ).length ) {
-					removeBlock( parentListId );
+					const shouldSelectParent = false;
+					removeBlock( parentListId, shouldSelectParent );
 				}
 			} );
 		}, [] ),

diff --git a/packages/e2e-tests/specs/widgets/editing-widgets.test.js b/packages/e2e-tests/specs/widgets/editing-widgets.test.js
@@ -384,6 +384,11 @@ describe( 'Widgets screen', () => {
 
 	describe( 'Function widgets', () => {
 		async function addMarquee( nbExpectedMarquees ) {
+			const [ firstWidgetArea ] = await findAll( {
+				role: 'document',
+				name: 'Block: Widget Area',
+			} );
+			await firstWidgetArea.focus();
 			const marqueeBlock = await getBlockInGlobalInserter(
 				'Marquee Greeting'
 			);

diff --git a/test/e2e/specs/editor/various/block-deletion.spec.js b/test/e2e/specs/editor/various/block-deletion.spec.js
@@ -62,6 +62,53 @@ test.describe( 'Block deletion', () => {
 		] );
 	} );
 
+	// this test should be moved to a new testing story about focus management.
+	test( 'deleting a block focuses the parent block', async ( {
+		editor,
+		page,
+	} ) => {
+		// Add a group with a paragraph in it.
+		await editor.insertBlock( {
+			name: 'core/group',
+			innerBlocks: [
+				{
+					name: 'core/paragraph',
+					attributes: { content: 'Paragraph child of group' },
+				},
+			],
+		} );
+
+		// Select the paragraph.
+		const paragraph = editor.canvas.getByRole( 'document', {
+			name: 'Paragraph block',
+		} );
+		await editor.selectBlocks( paragraph );
+
+		// Remove the current paragraph via the Block Toolbar options menu.
+		await editor.showBlockToolbar();
+		await editor.canvas
+			.getByRole( 'toolbar', { name: 'Block tools' } )
+			.getByRole( 'button', { name: 'Options' } )
+			.click();
+		await page
+			.getByRole( 'menuitem', { name: 'Remove Paragraph' } )
+			.click();
+
+		// Ensure the paragraph was removed.
+		await expect
+			.poll( editor.getBlocks )
+			.toMatchObject( [ { name: 'core/group', attributes: {} } ] );
+
+		// Ensure the group block is focused.
+		await expect(
+			editor.canvas
+				.getByRole( 'document', {
+					name: 'Block: Group',
+				} )
+				.last()
+		).toBeFocused();
+	} );
+
 	test( 'deleting the last block via the keyboard shortcut', async ( {
 		editor,
 		page,