Editor: Reimplement select previous/next as independent controls

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

@@ -1560,6 +1560,24 @@ reflects a reverse selection.
  * initialPosition: Optional initial position. Pass as -1 to
                                  reflect reverse selection.
 
+### selectPreviousBlock
+
+Yields action objects used in signalling that the block preceding the given
+clientId should be selected.
+
+*Parameters*
+
+ * clientId: Block client ID.
+
+### selectNextBlock
+
+Yields action objects used in signalling that the block following the given
+clientId should be selected.
+
+*Parameters*
+
+ * clientId: Block client ID.
+
 ### toggleSelection
 
 Returns an action object that enables or disables block selection.
@@ -1703,7 +1721,7 @@ be created.
 
 ### 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/editor/src/store/actions.js

@@ -8,6 +8,11 @@ import { castArray } from 'lodash';
  */
 import { getDefaultBlockName, createBlock } from '@wordpress/blocks';
 
+/**
+ * Internal dependencies
+ */
+import { select } from './controls';
+
 /**
  * Returns an action object used in signalling that editor has initialized with
  * the specified post object and editor settings.
@@ -172,6 +177,38 @@ export function selectBlock( clientId, initialPosition = null ) {
 	};
 }
 
+/**
+ * Yields action objects used in signalling that the block preceding the given
+ * clientId should be selected.
+ *
+ * @param {string} clientId Block client ID.
+ */
+export function* selectPreviousBlock( clientId ) {
+	const previousBlockClientId = yield select(
+		'core/editor',
+		'getPreviousBlockClientId',
+		clientId
+	);
+
+	yield selectBlock( previousBlockClientId, -1 );
+}
+
+/**
+ * Yields action objects used in signalling that the block following the given
+ * clientId should be selected.
+ *
+ * @param {string} clientId Block client ID.
+ */
+export function* selectNextBlock( clientId ) {
+	const nextBlockClientId = yield select(
+		'core/editor',
+		'getNextBlockClientId',
+		clientId
+	);
+
+	yield selectBlock( nextBlockClientId );
+}
+
 export function startMultiSelect() {
 	return {
 		type: 'START_MULTI_SELECT',
@@ -477,20 +514,23 @@ export function createUndoLevel() {
 }
 
 /**
- * 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/editor/src/store/controls.js

@@ -0,0 +1,30 @@
+/**
+ * WordPress dependencies
+ */
+import { createRegistryControl } from '@wordpress/data';
+
+/**
+ * Calls a selector using the current state.
+ *
+ * @param {string} storeName    Store name.
+ * @param {string} selectorName Selector name.
+ * @param  {Array} args         Selector arguments.
+ *
+ * @return {Object} control descriptor.
+ */
+export function select( storeName, selectorName, ...args ) {
+	return {
+		type: 'SELECT',
+		storeName,
+		selectorName,
+		args,
+	};
+}
+
+const controls = {
+	SELECT: createRegistryControl( ( registry ) => ( { storeName, selectorName, args } ) => {
+		return registry.select( storeName )[ selectorName ]( ...args );
+	} ),
+};
+
+export default controls;

packages/editor/src/store/effects.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { compact, last, has } from 'lodash';
+import { compact, has } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -29,11 +29,8 @@ import {
 } from './actions';
 import {
 	getBlock,
-	getBlockRootClientId,
 	getBlocks,
 	getBlockCount,
-	getPreviousBlockClientId,
-	getSelectedBlockClientId,
 	getSelectedBlockCount,
 	getTemplate,
 	getTemplateLock,
@@ -86,43 +83,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
@@ -258,7 +218,6 @@ export default {
 	CONVERT_BLOCK_TO_STATIC: convertBlockToStatic,
 	CONVERT_BLOCK_TO_REUSABLE: convertBlockToReusable,
 	REMOVE_BLOCKS: [
-		selectPreviousBlock,
 		ensureDefaultBlock,
 	],
 	REPLACE_BLOCKS: [

packages/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 controls from './controls';
 
 /**
  * Module Constants
@@ -20,6 +21,7 @@ const store = registerStore( MODULE_KEY, {
 	reducer,
 	selectors,
 	actions,
+	controls,
 	persist: [ 'preferences' ],
 } );
 applyMiddlewares( store );

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

@@ -19,6 +19,7 @@ import {
 	updateBlockAttributes,
 	updateBlock,
 	selectBlock,
+	selectPreviousBlock,
 	startMultiSelect,
 	stopMultiSelect,
 	multiSelect,
@@ -293,32 +294,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 ],
+				},
+			] );
 		} );
 	} );