From b4bbd1c999e39afbd8ac18caa728140c05f19dfc Mon Sep 17 00:00:00 2001 From: Jorge Date: Fri, 1 Mar 2019 11:24:25 +0000 Subject: [PATCH 1/4] Fix: deleting the last block triggers a focus loss. --- .../developers/data/data-core-block-editor.md | 10 +++ packages/block-editor/src/store/actions.js | 40 +++++++++-- packages/block-editor/src/store/controls.js | 23 ++++++ packages/block-editor/src/store/effects.js | 3 - .../block-editor/src/store/test/actions.js | 72 ++++++++++++++----- 5 files changed, 124 insertions(+), 24 deletions(-) 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 3166400bd3fbf..2012820d1a6e9 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -958,6 +958,16 @@ Returns an action object used in signalling that two blocks should be merged * firstBlockClientId: Client ID of the first block to merge. * secondBlockClientId: Client ID of the second block to merge. +### __internalRemoveBlocksPure + +Returns action objects used in signalling that the blocks corresponding to +the set of specified client IDs are to be removed. +This action does not trigger any required side effects and it is not recommended for public usage. + +*Parameters* + + * clientIds: Client IDs of blocks to remove. + ### removeBlocks Yields action objects used in signalling that the blocks corresponding to diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 382a25f438d34..e26aec427330c 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -11,7 +11,7 @@ import { getDefaultBlockName, createBlock } from '@wordpress/blocks'; /** * Internal dependencies */ -import { select } from './controls'; +import { select, dispatch } from './controls'; /** * Returns an action object used in signalling that blocks state should be @@ -374,6 +374,23 @@ export function mergeBlocks( firstBlockClientId, secondBlockClientId ) { }; } +/** + * Returns action objects used in signalling that the blocks corresponding to + * the set of specified client IDs are to be removed. + * This action does not trigger any required side effects and it is not recommended for public usage. + * + * @param {string|string[]} clientIds Client IDs of blocks to remove. + * + * @return {Object} Action object. + * + */ +export function __internalRemoveBlocksPure( clientIds ) { + return { + type: 'REMOVE_BLOCKS', + clientIds, + }; +} + /** * Yields action objects used in signalling that the blocks corresponding to * the set of specified client IDs are to be removed. @@ -386,13 +403,26 @@ export function* removeBlocks( clientIds, selectPrevious = true ) { clientIds = castArray( clientIds ); if ( selectPrevious ) { - yield selectPreviousBlock( clientIds[ 0 ] ); + yield dispatch( + 'core/block-editor', + 'selectPreviousBlock', + clientIds[ 0 ] + ); } - yield { - type: 'REMOVE_BLOCKS', + yield dispatch( + 'core/block-editor', + '__internalRemoveBlocksPure', clientIds, - }; + ); + + const count = yield select( + 'core/block-editor', + 'getBlockCount', + ); + if ( count === 0 ) { + yield insertDefaultBlock(); + } } /** diff --git a/packages/block-editor/src/store/controls.js b/packages/block-editor/src/store/controls.js index 5012ab244c21c..542911c7f6d91 100644 --- a/packages/block-editor/src/store/controls.js +++ b/packages/block-editor/src/store/controls.js @@ -21,10 +21,33 @@ export function select( storeName, selectorName, ...args ) { }; } +/** + * Dispatches a control action for triggering a registry dispatch. + * + * @param {string} storeKey + * @param {string} actionName + * @param {Array} args Arguments for the dispatch action. + * + * @return {Object} control descriptor. + */ +export function dispatch( storeKey, actionName, ...args ) { + return { + type: 'DISPATCH', + storeKey, + actionName, + args, + }; +} + const controls = { SELECT: createRegistryControl( ( registry ) => ( { storeName, selectorName, args } ) => { return registry.select( storeName )[ selectorName ]( ...args ); } ), + DISPATCH: createRegistryControl( + ( registry ) => ( { storeKey, actionName, args } ) => { + return registry.dispatch( storeKey )[ actionName ]( ...args ); + } + ), }; export default controls; diff --git a/packages/block-editor/src/store/effects.js b/packages/block-editor/src/store/effects.js index f02ef6fbd7231..ca46e1afb8f4a 100644 --- a/packages/block-editor/src/store/effects.js +++ b/packages/block-editor/src/store/effects.js @@ -127,9 +127,6 @@ export default { RESET_BLOCKS: [ validateBlocksToTemplate, ], - REMOVE_BLOCKS: [ - ensureDefaultBlock, - ], REPLACE_BLOCKS: [ ensureDefaultBlock, ], diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index 3ae9039505356..7c6c30dcc102b 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -12,7 +12,6 @@ import { updateBlockAttributes, updateBlock, selectBlock, - selectPreviousBlock, startMultiSelect, stopMultiSelect, multiSelect, @@ -27,8 +26,11 @@ import { removeBlock, toggleBlockMode, updateBlockListSettings, + __internalRemoveBlocksPure, } from '../actions'; +import { select, dispatch } from '../controls'; + describe( 'actions', () => { describe( 'resetBlocks', () => { it( 'should return the RESET_BLOCKS actions', () => { @@ -205,6 +207,21 @@ describe( 'actions', () => { } ); } ); + describe( '__internalRemoveBlocksPure', () => { + it( 'should return REMOVE_BLOCKS action', () => { + const clientIds = [ 'myclientid' ]; + + const action = __internalRemoveBlocksPure( clientIds ); + + expect( action ).toEqual( + { + type: 'REMOVE_BLOCKS', + clientIds, + }, + ); + } ); + } ); + describe( 'removeBlocks', () => { it( 'should return REMOVE_BLOCKS action', () => { const clientId = 'clientId'; @@ -213,11 +230,20 @@ describe( 'actions', () => { const actions = Array.from( removeBlocks( clientIds ) ); expect( actions ).toEqual( [ - selectPreviousBlock( clientId ), - { - type: 'REMOVE_BLOCKS', - clientIds, - }, + dispatch( + 'core/block-editor', + 'selectPreviousBlock', + clientId + ), + dispatch( + 'core/block-editor', + '__internalRemoveBlocksPure', + [ clientId ], + ), + select( + 'core/block-editor', + 'getBlockCount', + ), ] ); } ); } ); @@ -229,24 +255,38 @@ describe( 'actions', () => { const actions = Array.from( removeBlock( clientId ) ); expect( actions ).toEqual( [ - selectPreviousBlock( clientId ), - { - type: 'REMOVE_BLOCKS', - clientIds: [ clientId ], - }, + dispatch( + 'core/block-editor', + 'selectPreviousBlock', + clientId + ), + dispatch( + 'core/block-editor', + '__internalRemoveBlocksPure', + [ clientId ], + ), + select( + 'core/block-editor', + 'getBlockCount', + ), ] ); } ); - it( 'should return REMOVE_BLOCKS action, opting out of remove previous', () => { + it( 'should return REMOVE_BLOCKS action, opting out of select previous', () => { const clientId = 'myclientid'; const actions = Array.from( removeBlock( clientId, false ) ); expect( actions ).toEqual( [ - { - type: 'REMOVE_BLOCKS', - clientIds: [ clientId ], - }, + dispatch( + 'core/block-editor', + '__internalRemoveBlocksPure', + [ clientId ], + ), + select( + 'core/block-editor', + 'getBlockCount', + ), ] ); } ); } ); From 94f8c9d9e3b3fcd387c3cc5d315c87d26dd6fd4e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 1 Mar 2019 13:53:28 +0000 Subject: [PATCH 2/4] Update packages/block-editor/src/store/test/actions.js Co-Authored-By: jorgefilipecosta --- packages/block-editor/src/store/test/actions.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index 7c6c30dcc102b..a1c1cfaa725d9 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -28,7 +28,6 @@ import { updateBlockListSettings, __internalRemoveBlocksPure, } from '../actions'; - import { select, dispatch } from '../controls'; describe( 'actions', () => { From 19b099810696c69c681a0e6b76fea947ac23b011 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 1 Mar 2019 14:18:01 -0500 Subject: [PATCH 3/4] Block Editor: Yield remove actions directly --- .../developers/data/data-core-block-editor.md | 10 ---- packages/block-editor/src/store/actions.js | 33 ++--------- packages/block-editor/src/store/controls.js | 23 -------- .../block-editor/src/store/test/actions.js | 58 +++++-------------- 4 files changed, 22 insertions(+), 102 deletions(-) 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 2012820d1a6e9..3166400bd3fbf 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -958,16 +958,6 @@ Returns an action object used in signalling that two blocks should be merged * firstBlockClientId: Client ID of the first block to merge. * secondBlockClientId: Client ID of the second block to merge. -### __internalRemoveBlocksPure - -Returns action objects used in signalling that the blocks corresponding to -the set of specified client IDs are to be removed. -This action does not trigger any required side effects and it is not recommended for public usage. - -*Parameters* - - * clientIds: Client IDs of blocks to remove. - ### removeBlocks Yields action objects used in signalling that the blocks corresponding to diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index e26aec427330c..a01fcb1a64450 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -11,7 +11,7 @@ import { getDefaultBlockName, createBlock } from '@wordpress/blocks'; /** * Internal dependencies */ -import { select, dispatch } from './controls'; +import { select } from './controls'; /** * Returns an action object used in signalling that blocks state should be @@ -374,23 +374,6 @@ export function mergeBlocks( firstBlockClientId, secondBlockClientId ) { }; } -/** - * Returns action objects used in signalling that the blocks corresponding to - * the set of specified client IDs are to be removed. - * This action does not trigger any required side effects and it is not recommended for public usage. - * - * @param {string|string[]} clientIds Client IDs of blocks to remove. - * - * @return {Object} Action object. - * - */ -export function __internalRemoveBlocksPure( clientIds ) { - return { - type: 'REMOVE_BLOCKS', - clientIds, - }; -} - /** * Yields action objects used in signalling that the blocks corresponding to * the set of specified client IDs are to be removed. @@ -403,23 +386,19 @@ export function* removeBlocks( clientIds, selectPrevious = true ) { clientIds = castArray( clientIds ); if ( selectPrevious ) { - yield dispatch( - 'core/block-editor', - 'selectPreviousBlock', - clientIds[ 0 ] - ); + yield selectPreviousBlock( clientIds[ 0 ] ); } - yield dispatch( - 'core/block-editor', - '__internalRemoveBlocksPure', + yield { + type: 'REMOVE_BLOCKS', clientIds, - ); + }; const count = yield select( 'core/block-editor', 'getBlockCount', ); + if ( count === 0 ) { yield insertDefaultBlock(); } diff --git a/packages/block-editor/src/store/controls.js b/packages/block-editor/src/store/controls.js index 542911c7f6d91..5012ab244c21c 100644 --- a/packages/block-editor/src/store/controls.js +++ b/packages/block-editor/src/store/controls.js @@ -21,33 +21,10 @@ export function select( storeName, selectorName, ...args ) { }; } -/** - * Dispatches a control action for triggering a registry dispatch. - * - * @param {string} storeKey - * @param {string} actionName - * @param {Array} args Arguments for the dispatch action. - * - * @return {Object} control descriptor. - */ -export function dispatch( storeKey, actionName, ...args ) { - return { - type: 'DISPATCH', - storeKey, - actionName, - args, - }; -} - const controls = { SELECT: createRegistryControl( ( registry ) => ( { storeName, selectorName, args } ) => { return registry.select( storeName )[ selectorName ]( ...args ); } ), - DISPATCH: createRegistryControl( - ( registry ) => ( { storeKey, actionName, args } ) => { - return registry.dispatch( storeKey )[ actionName ]( ...args ); - } - ), }; export default controls; diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index a1c1cfaa725d9..2ca6839dc57e2 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -12,6 +12,7 @@ import { updateBlockAttributes, updateBlock, selectBlock, + selectPreviousBlock, startMultiSelect, stopMultiSelect, multiSelect, @@ -26,9 +27,8 @@ import { removeBlock, toggleBlockMode, updateBlockListSettings, - __internalRemoveBlocksPure, } from '../actions'; -import { select, dispatch } from '../controls'; +import { select } from '../controls'; describe( 'actions', () => { describe( 'resetBlocks', () => { @@ -206,21 +206,6 @@ describe( 'actions', () => { } ); } ); - describe( '__internalRemoveBlocksPure', () => { - it( 'should return REMOVE_BLOCKS action', () => { - const clientIds = [ 'myclientid' ]; - - const action = __internalRemoveBlocksPure( clientIds ); - - expect( action ).toEqual( - { - type: 'REMOVE_BLOCKS', - clientIds, - }, - ); - } ); - } ); - describe( 'removeBlocks', () => { it( 'should return REMOVE_BLOCKS action', () => { const clientId = 'clientId'; @@ -229,16 +214,11 @@ describe( 'actions', () => { const actions = Array.from( removeBlocks( clientIds ) ); expect( actions ).toEqual( [ - dispatch( - 'core/block-editor', - 'selectPreviousBlock', - clientId - ), - dispatch( - 'core/block-editor', - '__internalRemoveBlocksPure', - [ clientId ], - ), + selectPreviousBlock( clientId ), + { + type: 'REMOVE_BLOCKS', + clientIds, + }, select( 'core/block-editor', 'getBlockCount', @@ -254,16 +234,11 @@ describe( 'actions', () => { const actions = Array.from( removeBlock( clientId ) ); expect( actions ).toEqual( [ - dispatch( - 'core/block-editor', - 'selectPreviousBlock', - clientId - ), - dispatch( - 'core/block-editor', - '__internalRemoveBlocksPure', - [ clientId ], - ), + selectPreviousBlock( clientId ), + { + type: 'REMOVE_BLOCKS', + clientIds: [ clientId ], + }, select( 'core/block-editor', 'getBlockCount', @@ -277,11 +252,10 @@ describe( 'actions', () => { const actions = Array.from( removeBlock( clientId, false ) ); expect( actions ).toEqual( [ - dispatch( - 'core/block-editor', - '__internalRemoveBlocksPure', - [ clientId ], - ), + { + type: 'REMOVE_BLOCKS', + clientIds: [ clientId ], + }, select( 'core/block-editor', 'getBlockCount', From 3c5cd955bd2e4caf1a0423c6b35e6756353415dc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 1 Mar 2019 15:47:46 -0500 Subject: [PATCH 4/4] Editor: Clarify behavior of insertDefaultBlock on remove last --- packages/block-editor/src/store/actions.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index a01fcb1a64450..c01475af2bdb1 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -399,6 +399,8 @@ export function* removeBlocks( clientIds, selectPrevious = true ) { 'getBlockCount', ); + // 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. if ( count === 0 ) { yield insertDefaultBlock(); }