diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 182242589abb3d..bcd8b3295c4bc9 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'; /** @@ -202,20 +203,36 @@ function withPersistentBlockChange( reducer ) { const isExplicitPersistentChange = action.type === 'MARK_LAST_CHANGE_AS_PERSISTENT'; - if ( state !== nextState || isExplicitPersistentChange ) { - // Some state changes should not be considered persistent, namely - // those which are not a direct result of user interaction. - const isPersistentStateChange = ! IGNORED_ACTION_TYPES.has( action.type ); + // 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 ), + }; + } - nextState = { + // 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 || ( - isPersistentStateChange && - ! 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 f466889def5797..ad5d5cf5747854 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -1046,6 +1046,19 @@ describe( 'state', () => { } ); describe( 'isPersistentChange', () => { + it( 'should default a changing state to true', () => { + const state = deepFreeze( blocks( undefined, {} ) ); + + expect( state.isPersistentChange ).toBe( true ); + } ); + + it( 'should default an unchanging state to false', () => { + const original = {}; + const state = deepFreeze( blocks( original, {} ) ); + + 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 +1076,33 @@ 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: '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 +1110,7 @@ describe( 'state', () => { innerBlocks: [], } ], } ) ); - let state = blocks( original, { + original = blocks( original, { type: 'UPDATE_BLOCK_ATTRIBUTES', clientId: 'kumquat', attributes: { @@ -1080,7 +1118,7 @@ describe( 'state', () => { }, } ); - state = blocks( state, { + const state = blocks( original, { type: 'UPDATE_BLOCK_ATTRIBUTES', clientId: 'kumquat', attributes: { @@ -1091,6 +1129,37 @@ describe( 'state', () => { 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: { + updated: true, + }, + } ); + + 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',