Block Editor: Reimplement removeBlocks previous selection as control

docs/designers-developers/developers/data/data-core-block-editor.md

@@ -840,6 +840,26 @@ reflects a reverse selection.
  * initialPosition: Optional initial position. Pass as -1 to
                                  reflect reverse selection.
 
+### selectPreviousBlock
+
+Returns an action object used in signalling that the block preceding the
+given clientId should be selected.
+
+*Parameters*
+
+ * clientId: Block client ID.
+
+### selectNextBlock
+
+Returns an action object used in signalling that the block following the
+given clientId should be selected.
+
+*Parameters*
+
+ * clientId: Block client ID.
+ * options: Optional selection options.
+ * options.isReverse: Whether to select preceding.
+
 ### startMultiSelect
 
 Returns an action object used in signalling that a block multi-selection has started.
@@ -964,7 +984,7 @@ Returns an action object used in signalling that two blocks should be merged
 
 ### removeBlocks
 
-Returns an action object used in signalling that the blocks corresponding to
+Yields action objects used in signalling that the blocks corresponding to
 the set of specified client IDs are to be removed.
 
 *Parameters*

packages/block-editor/src/store/actions.js

@@ -111,6 +111,36 @@ export function selectBlock( clientId, initialPosition = null ) {
 	};
 }
 
+/**
+ * Returns an action object used in signalling that the block preceding the
+ * given clientId should be selected.
+ *
+ * @param {string} clientId Block client ID.
+ *
+ * @return {Object} Action object.
+ */
+export function selectPreviousBlock( clientId ) {
+	return selectNextBlock( clientId, { isReverse: true } );
+}
+
+/**
+ * Returns an action object used in signalling that the block following the
+ * given clientId should be selected.
+ *
+ * @param {string}  clientId          Block client ID.
+ * @param {?Object} options           Optional selection options.
+ * @param {boolean} options.isReverse Whether to select preceding.
+ *
+ * @return {Object} Action object.
+ */
+export function selectNextBlock( clientId, options ) {
+	return {
+		type: 'SELECT_NEXT_BLOCK',
+		clientId,
+		...options,
+	};
+}
+
 /**
  * Returns an action object used in signalling that a block multi-selection has started.
  *
@@ -355,20 +385,23 @@ export function mergeBlocks( firstBlockClientId, secondBlockClientId ) {
 }
 
 /**
- * Returns an action object used in signalling that the blocks corresponding to
+ * Yields action objects used in signalling that the blocks corresponding to
  * the set of specified client IDs are to be removed.
  *
  * @param {string|string[]} clientIds      Client IDs of blocks to remove.
  * @param {boolean}         selectPrevious True if the previous block should be
  *                                         selected when a block is removed.
- *
- * @return {Object} Action object.
  */
-export function removeBlocks( clientIds, selectPrevious = true ) {
-	return {
+export function* removeBlocks( clientIds, selectPrevious = true ) {
+	clientIds = castArray( clientIds );
+
+	if ( selectPrevious ) {
+		yield selectPreviousBlock( clientIds[ 0 ] );
+	}
+
+	yield {
 		type: 'REMOVE_BLOCKS',
-		clientIds: castArray( clientIds ),
-		selectPrevious,
+		clientIds,
 	};
 }
 

packages/block-editor/src/store/controls.js

@@ -0,0 +1,36 @@
+/**
+ * WordPress dependencies
+ */
+import { createRegistryControl } from '@wordpress/data';
+
+export const SELECT_NEXT_BLOCK = createRegistryControl( ( registry ) => ( action ) => {
+	const { clientId, isReverse } = action;
+	const {
+		getPreviousBlockClientId,
+		getNextBlockClientId,
+		getSelectedBlockClientId,
+	} = registry.select( 'core/editor' );
+
+	let targetClientId;
+	if ( isReverse ) {
+		targetClientId = getPreviousBlockClientId( clientId );
+	} else {
+		targetClientId = getNextBlockClientId( clientId );
+	}
+
+	// Only dispatch select block action if the currently selected block is
+	// is not already the block we want to be selected.
+	if ( ! targetClientId || targetClientId === getSelectedBlockClientId() ) {
+		return;
+	}
+
+	// When selecting in reverse, invert the default focus transition
+	// behavior, selecting the last available focusable.
+	let initialPosition;
+	if ( isReverse ) {
+		initialPosition = -1;
+	}
+
+	const { selectBlock } = registry.dispatch( 'core/editor' );
+	selectBlock( targetClientId, initialPosition );
+} );

packages/block-editor/src/store/effects.js

@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import { last } from 'lodash';
-
 /**
  * WordPress dependencies
  */
@@ -30,9 +25,6 @@ import {
 	getBlocks,
 	getSelectedBlockCount,
 	getBlockCount,
-	getBlockRootClientId,
-	getPreviousBlockClientId,
-	getSelectedBlockClientId,
 	getTemplateLock,
 	getTemplate,
 	isValidTemplate,
@@ -68,43 +60,6 @@ export function validateBlocksToTemplate( action, store ) {
 	}
 }
 
-/**
- * Effect handler which will return a block select action to select the block
- * occurring before the selected block in the previous state, unless it is the
- * same block or the action includes a falsey `selectPrevious` option flag.
- *
- * @param {Object} action Action which had initiated the effect handler.
- * @param {Object} store  Store instance.
- *
- * @return {?Object} Block select action to select previous, if applicable.
- */
-export function selectPreviousBlock( action, store ) {
-	// if the action says previous block should not be selected don't do anything.
-	if ( ! action.selectPrevious ) {
-		return;
-	}
-
-	const firstRemovedBlockClientId = action.clientIds[ 0 ];
-	const state = store.getState();
-	const selectedBlockClientId = getSelectedBlockClientId( state );
-
-	// recreate the state before the block was removed.
-	const previousState = { ...state, editor: { present: last( state.editor.past ) } };
-
-	// rootClientId of the removed block.
-	const rootClientId = getBlockRootClientId( previousState, firstRemovedBlockClientId );
-
-	// Client ID of the block that was before the removed block or the
-	// rootClientId if the removed block was first amongst its siblings.
-	const blockClientIdToSelect = getPreviousBlockClientId( previousState, firstRemovedBlockClientId ) || rootClientId;
-
-	// Dispatch select block action if the currently selected block
-	// is not already the block we want to be selected.
-	if ( blockClientIdToSelect !== selectedBlockClientId ) {
-		return selectBlock( blockClientIdToSelect, -1 );
-	}
-}
-
 /**
  * Effect handler which will return a default block insertion action if there
  * are no other blocks at the root of the editor. This is expected to be used
@@ -173,7 +128,6 @@ export default {
 		validateBlocksToTemplate,
 	],
 	REMOVE_BLOCKS: [
-		selectPreviousBlock,
 		ensureDefaultBlock,
 	],
 	REPLACE_BLOCKS: [

packages/block-editor/src/store/index.js

@@ -10,6 +10,7 @@ import reducer from './reducer';
 import applyMiddlewares from './middlewares';
 import * as selectors from './selectors';
 import * as actions from './actions';
+import * as controls from './controls';
 
 /**
  * Module Constants
@@ -20,6 +21,7 @@ const store = registerStore( MODULE_KEY, {
 	reducer,
 	selectors,
 	actions,
+	controls,
 	persist: [ 'preferences' ],
 } );
 applyMiddlewares( store );

packages/block-editor/src/store/test/actions.js

@@ -12,6 +12,8 @@ import {
 	updateBlockAttributes,
 	updateBlock,
 	selectBlock,
+	selectPreviousBlock,
+	selectNextBlock,
 	startMultiSelect,
 	stopMultiSelect,
 	multiSelect,
@@ -224,32 +226,47 @@ describe( 'actions', () => {
 
 	describe( 'removeBlocks', () => {
 		it( 'should return REMOVE_BLOCKS action', () => {
-			const clientIds = [ 'clientId' ];
-			expect( removeBlocks( clientIds ) ).toEqual( {
-				type: 'REMOVE_BLOCKS',
-				clientIds,
-				selectPrevious: true,
-			} );
+			const clientId = 'clientId';
+			const clientIds = [ clientId ];
+
+			const actions = Array.from( removeBlocks( clientIds ) );
+
+			expect( actions ).toEqual( [
+				selectPreviousBlock( clientId ),
+				{
+					type: 'REMOVE_BLOCKS',
+					clientIds,
+				},
+			] );
 		} );
 	} );
 
 	describe( 'removeBlock', () => {
 		it( 'should return REMOVE_BLOCKS action', () => {
 			const clientId = 'myclientid';
-			expect( removeBlock( clientId ) ).toEqual( {
-				type: 'REMOVE_BLOCKS',
-				clientIds: [
-					clientId,
-				],
-				selectPrevious: true,
-			} );
-			expect( removeBlock( clientId, false ) ).toEqual( {
-				type: 'REMOVE_BLOCKS',
-				clientIds: [
-					clientId,
-				],
-				selectPrevious: false,
-			} );
+
+			const actions = Array.from( removeBlock( clientId ) );
+
+			expect( actions ).toEqual( [
+				selectPreviousBlock( clientId ),
+				{
+					type: 'REMOVE_BLOCKS',
+					clientIds: [ clientId ],
+				},
+			] );
+		} );
+
+		it( 'should return REMOVE_BLOCKS action, opting out of remove previous', () => {
+			const clientId = 'myclientid';
+
+			const actions = Array.from( removeBlock( clientId, false ) );
+
+			expect( actions ).toEqual( [
+				{
+					type: 'REMOVE_BLOCKS',
+					clientIds: [ clientId ],
+				},
+			] );
 		} );
 	} );