From ac0c041efcbc3725da0542341b877c8dd1932774 Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 14 Sep 2023 12:13:51 +0300 Subject: [PATCH 1/2] Fix extra undo/redo step when removing or replacing all blocks --- packages/block-editor/src/store/actions.js | 22 ++++++++------- .../block-editor/src/store/private-actions.js | 13 ++++----- .../block-editor/src/store/test/actions.js | 27 +++++++++++++------ 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 32108de713f75..91434626f8d75 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -375,7 +375,7 @@ function getBlocksWithDefaultStylesApplied( blocks, blockEditorSettings ) { */ export const replaceBlocks = ( clientIds, blocks, indexToSelect, initialPosition = 0, meta ) => - ( { select, dispatch } ) => { + ( { select, dispatch, registry } ) => { /* eslint-enable jsdoc/valid-types */ clientIds = castArray( clientIds ); blocks = getBlocksWithDefaultStylesApplied( @@ -394,16 +394,18 @@ export const replaceBlocks = return; } } - dispatch( { - type: 'REPLACE_BLOCKS', - clientIds, - blocks, - time: Date.now(), - indexToSelect, - initialPosition, - meta, + registry.batch( () => { + dispatch( { + type: 'REPLACE_BLOCKS', + clientIds, + blocks, + time: Date.now(), + indexToSelect, + initialPosition, + meta, + } ); + dispatch.ensureDefaultBlock(); } ); - dispatch.ensureDefaultBlock(); }; /** diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js index cc3ac41983c8c..d467aa791ebfb 100644 --- a/packages/block-editor/src/store/private-actions.js +++ b/packages/block-editor/src/store/private-actions.js @@ -92,7 +92,7 @@ export function showBlockInterface() { */ export const privateRemoveBlocks = ( clientIds, selectPrevious = true, forceRemove = false ) => - ( { select, dispatch } ) => { + ( { select, dispatch, registry } ) => { if ( ! clientIds || ! clientIds.length ) { return; } @@ -154,11 +154,12 @@ export const privateRemoveBlocks = dispatch.selectPreviousBlock( clientIds[ 0 ], selectPrevious ); } - dispatch( { type: 'REMOVE_BLOCKS', clientIds } ); - - // 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. - dispatch( ensureDefaultBlock() ); + registry.batch( () => { + dispatch( { type: 'REMOVE_BLOCKS', clientIds } ); + // 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. + dispatch( ensureDefaultBlock() ); + } ); }; /** diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index 4dc9adefb82b5..f1f8cb29f1406 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -219,8 +219,9 @@ describe( 'actions', () => { }; const dispatch = jest.fn(); dispatch.ensureDefaultBlock = jest.fn(); + const registry = createRegistry(); - replaceBlock( 'chicken', block )( { select, dispatch } ); + replaceBlock( 'chicken', block )( { select, dispatch, registry } ); expect( dispatch ).toHaveBeenCalledWith( { type: 'REPLACE_BLOCKS', @@ -285,8 +286,12 @@ describe( 'actions', () => { }; const dispatch = jest.fn(); dispatch.ensureDefaultBlock = jest.fn(); + const registry = createRegistry(); - replaceBlocks( [ 'chicken' ], blocks )( { select, dispatch } ); + replaceBlocks( + [ 'chicken' ], + blocks + )( { select, dispatch, registry } ); expect( dispatch ).toHaveBeenCalledWith( { type: 'REPLACE_BLOCKS', @@ -319,6 +324,7 @@ describe( 'actions', () => { }; const dispatch = jest.fn(); dispatch.ensureDefaultBlock = jest.fn(); + const registry = createRegistry(); replaceBlocks( [ 'chicken' ], @@ -326,7 +332,7 @@ describe( 'actions', () => { null, null, meta - )( { select, dispatch } ); + )( { select, dispatch, registry } ); expect( dispatch ).toHaveBeenCalledWith( { type: 'REPLACE_BLOCKS', @@ -628,8 +634,9 @@ describe( 'actions', () => { const dispatch = Object.assign( jest.fn(), { selectPreviousBlock: jest.fn(), } ); + const registry = createRegistry(); - removeBlocks( clientIds )( { select, dispatch } ); + removeBlocks( clientIds )( { select, dispatch, registry } ); expect( dispatch.selectPreviousBlock ).toHaveBeenCalledWith( clientId, @@ -739,8 +746,8 @@ describe( 'actions', () => { const dispatch = Object.assign( jest.fn(), { selectPreviousBlock: jest.fn(), } ); - - removeBlock( clientId )( { select, dispatch } ); + const registry = createRegistry(); + removeBlock( clientId )( { select, dispatch, registry } ); expect( dispatch.selectPreviousBlock ).toHaveBeenCalledWith( clientId, @@ -753,7 +760,7 @@ describe( 'actions', () => { } ); } ); - it( 'should dispatch REMOVE_BLOCKS action, opting out of select previous', () => { + it( 'should dispatch REMOVE_BLOCKS action, opting out of select previous', async () => { const clientId = 'myclientid'; const select = { @@ -765,7 +772,11 @@ describe( 'actions', () => { selectPreviousBlock: jest.fn(), } ); - removeBlocks( [ clientId ], false )( { select, dispatch } ); + const registry = createRegistry(); + removeBlocks( + [ clientId ], + false + )( { select, dispatch, registry } ); expect( dispatch.selectPreviousBlock ).not.toHaveBeenCalled(); From 8f3645eeb4085eef14cde30232efdb3f7097651e Mon Sep 17 00:00:00 2001 From: ntsekouras Date: Thu, 14 Sep 2023 14:01:18 +0300 Subject: [PATCH 2/2] add more comments --- packages/block-editor/src/store/actions.js | 4 ++++ packages/block-editor/src/store/private-actions.js | 2 ++ 2 files changed, 6 insertions(+) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 91434626f8d75..8b820de585daf 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -394,6 +394,8 @@ export const replaceBlocks = return; } } + // We're batching these two actions because an extra `undo/redo` step can + // be created, based on whether we insert a default block or not. registry.batch( () => { dispatch( { type: 'REPLACE_BLOCKS', @@ -404,6 +406,8 @@ export const replaceBlocks = initialPosition, meta, } ); + // 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. dispatch.ensureDefaultBlock(); } ); }; diff --git a/packages/block-editor/src/store/private-actions.js b/packages/block-editor/src/store/private-actions.js index d467aa791ebfb..0171b4f442d19 100644 --- a/packages/block-editor/src/store/private-actions.js +++ b/packages/block-editor/src/store/private-actions.js @@ -154,6 +154,8 @@ export const privateRemoveBlocks = dispatch.selectPreviousBlock( clientIds[ 0 ], selectPrevious ); } + // We're batching these two actions because an extra `undo/redo` step can + // be created, based on whether we insert a default block or not. registry.batch( () => { dispatch( { type: 'REMOVE_BLOCKS', clientIds } ); // To avoid a focus loss when removing the last block, assure there is