From cb00f0a73bc320e4956d2cb543767651d34984e5 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 13 Feb 2019 16:05:49 -0500 Subject: [PATCH] Block Editor: Reimplement removeBlocks previous selection as control --- .../developers/data/data-core-block-editor.md | 22 ++++++- packages/block-editor/src/store/actions.js | 47 ++++++++++++--- packages/block-editor/src/store/controls.js | 36 ++++++++++++ packages/block-editor/src/store/effects.js | 46 --------------- packages/block-editor/src/store/index.js | 2 + .../block-editor/src/store/test/actions.js | 57 ++++++++++++------- 6 files changed, 136 insertions(+), 74 deletions(-) create mode 100644 packages/block-editor/src/store/controls.js diff --git a/docs/designers-developers/developers/data/data-core-block-editor.md b/docs/designers-developers/developers/data/data-core-block-editor.md index 79477cc85687a..70fed0f2a87ee 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/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* diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 2e5f917705858..989c62f987298 100644 --- a/packages/block-editor/src/store/actions.js +++ b/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, }; } diff --git a/packages/block-editor/src/store/controls.js b/packages/block-editor/src/store/controls.js new file mode 100644 index 0000000000000..a593c4bc82290 --- /dev/null +++ b/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 ); +} ); diff --git a/packages/block-editor/src/store/effects.js b/packages/block-editor/src/store/effects.js index a902460644b69..f02ef6fbd7231 100644 --- a/packages/block-editor/src/store/effects.js +++ b/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: [ diff --git a/packages/block-editor/src/store/index.js b/packages/block-editor/src/store/index.js index cb473c33c6ada..bc4662410663f 100644 --- a/packages/block-editor/src/store/index.js +++ b/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 ); diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index c1d08c75dda50..19bf67c07c8f9 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/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 ], + }, + ] ); } ); } );