Skip to content

Commit

Permalink
Block Editor: Consider RECEIVE_BLOCKS as non-persistent change (#14108)
Browse files Browse the repository at this point in the history
* Block Editor: Consider RECEIVE_BLOCKS as non-persistent change

* Block Editor: Compare last action in reducer enhancer only if non-ignored
  • Loading branch information
aduth authored Feb 27, 2019
1 parent 05570ff commit 845185b
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 10 deletions.
44 changes: 38 additions & 6 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
keys,
isEqual,
isEmpty,
get,
} from 'lodash';

/**
Expand Down Expand Up @@ -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;
Expand Down
86 changes: 83 additions & 3 deletions packages/block-editor/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -1063,31 +1069,105 @@ 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',
attributes: {},
innerBlocks: [],
} ],
} ) );
let state = blocks( original, {
original = blocks( original, {
type: 'UPDATE_BLOCK_ATTRIBUTES',
clientId: 'kumquat',
attributes: {
updated: false,
},
} );

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: {
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',
blocks: [ {
clientId: 'kumquat',
attributes: {},
innerBlocks: [],
} ],
} );

expect( state.isPersistentChange ).toBe( false );
} );
} );
Expand Down
1 change: 0 additions & 1 deletion packages/editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ export const editor = flow( [
withHistory( {
resetTypes: [ 'SETUP_EDITOR_STATE' ],
ignoreTypes: [
'RECEIVE_BLOCKS',
'RESET_POST',
'UPDATE_POST',
],
Expand Down

0 comments on commit 845185b

Please sign in to comment.