diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index d6cc2444af050..bcd8b3295c4bc 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -12,6 +12,7 @@ import { keys, isEqual, isEmpty, + get, } from 'lodash'; /** @@ -187,20 +188,51 @@ export function isUpdatingSameBlockAttribute( action, lastAction ) { function withPersistentBlockChange( reducer ) { let lastAction; + /** + * Set of action types for which a blocks state change should be considered + * non-persistent. + * + * @type {Set} + */ + const IGNORED_ACTION_TYPES = new Set( [ + 'RECEIVE_BLOCKS', + ] ); + return ( state, action ) => { let nextState = reducer( state, action ); + const isExplicitPersistentChange = action.type === 'MARK_LAST_CHANGE_AS_PERSISTENT'; - if ( state !== nextState || isExplicitPersistentChange ) { - nextState = { + // Defer to previous state value (or default) unless changing or + // explicitly marking as persistent. + if ( state === nextState && ! isExplicitPersistentChange ) { + return { + ...nextState, + isPersistentChange: get( state, [ 'isPersistentChange' ], true ), + }; + } + + // Some state changes should not be considered persistent, namely those + // which are not a direct result of user interaction. + const isIgnoredActionType = IGNORED_ACTION_TYPES.has( action.type ); + if ( isIgnoredActionType ) { + return { ...nextState, - isPersistentChange: ( - isExplicitPersistentChange || - ! isUpdatingSameBlockAttribute( action, lastAction ) - ), + isPersistentChange: false, }; } + nextState = { + ...nextState, + isPersistentChange: ( + isExplicitPersistentChange || + ! isUpdatingSameBlockAttribute( action, lastAction ) + ), + }; + + // In comparing against the previous action, consider only those which + // would have qualified as one which would have been ignored or not + // have resulted in a changed state. lastAction = action; return nextState; diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 6f1fae9dc0f03..778875b99ef59 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -1046,6 +1046,12 @@ describe( 'state', () => { } ); describe( 'isPersistentChange', () => { + it( 'should default a changing state to true', () => { + const state = deepFreeze( blocks( undefined, {} ) ); + + expect( state.isPersistentChange ).toBe( true ); + } ); + it( 'should consider any non-exempt block change as persistent', () => { const original = deepFreeze( blocks( undefined, { type: 'RESET_BLOCKS', @@ -1063,8 +1069,38 @@ describe( 'state', () => { expect( state.isPersistentChange ).toBe( true ); } ); + it( 'should consider any non-exempt block change as persistent across unchanging actions', () => { + let original = deepFreeze( blocks( undefined, { + type: 'RESET_BLOCKS', + blocks: [ { + clientId: 'kumquat', + attributes: {}, + innerBlocks: [], + } ], + } ) ); + original = blocks( original, { + type: 'NOOP', + } ); + original = blocks( original, { + // While RECEIVE_BLOCKS changes state, it's considered + // as ignored, confirmed by this test. + type: 'RECEIVE_BLOCKS', + blocks: [], + } ); + + const state = blocks( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: false, + }, + } ); + + expect( state.isPersistentChange ).toBe( true ); + } ); + it( 'should consider same block attribute update as exempt', () => { - const original = deepFreeze( blocks( undefined, { + let original = deepFreeze( blocks( undefined, { type: 'RESET_BLOCKS', blocks: [ { clientId: 'kumquat', @@ -1072,7 +1108,7 @@ describe( 'state', () => { innerBlocks: [], } ], } ) ); - let state = blocks( original, { + original = blocks( original, { type: 'UPDATE_BLOCK_ATTRIBUTES', clientId: 'kumquat', attributes: { @@ -1080,7 +1116,34 @@ describe( 'state', () => { }, } ); - state = blocks( state, { + const state = blocks( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: true, + }, + } ); + + expect( state.isPersistentChange ).toBe( false ); + } ); + + it( 'should flag an explicitly marked persistent change', () => { + let original = deepFreeze( blocks( undefined, { + type: 'RESET_BLOCKS', + blocks: [ { + clientId: 'kumquat', + attributes: {}, + innerBlocks: [], + } ], + } ) ); + original = blocks( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: false, + }, + } ); + original = blocks( original, { type: 'UPDATE_BLOCK_ATTRIBUTES', clientId: 'kumquat', attributes: { @@ -1088,6 +1151,23 @@ describe( 'state', () => { }, } ); + const state = blocks( original, { + type: 'MARK_LAST_CHANGE_AS_PERSISTENT', + } ); + + expect( state.isPersistentChange ).toBe( true ); + } ); + + it( 'should not consider received blocks as persistent change', () => { + const state = blocks( undefined, { + type: 'RECEIVE_BLOCKS', + blocks: [ { + clientId: 'kumquat', + attributes: {}, + innerBlocks: [], + } ], + } ); + expect( state.isPersistentChange ).toBe( false ); } ); } ); diff --git a/packages/editor/src/store/reducer.js b/packages/editor/src/store/reducer.js index 2cc1791a0a4a0..68a6624163ae0 100644 --- a/packages/editor/src/store/reducer.js +++ b/packages/editor/src/store/reducer.js @@ -134,7 +134,6 @@ export const editor = flow( [ withHistory( { resetTypes: [ 'SETUP_EDITOR_STATE' ], ignoreTypes: [ - 'RECEIVE_BLOCKS', 'RESET_POST', 'UPDATE_POST', ],