Skip to content

Commit

Permalink
Block Editor: Consider received blocks state change as ignored
Browse files Browse the repository at this point in the history
  • Loading branch information
aduth committed Apr 10, 2019
1 parent c382c79 commit a35fa82
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 27 deletions.
1 change: 1 addition & 0 deletions packages/block-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
### Internal

- Improved handling of blocks state references for unchanging states.
- Updated handling of blocks state to effectively ignored programmatically-received blocks data (e.g. reusable blocks received from editor).

## 1.0.0 (2019-03-06)

Expand Down
8 changes: 7 additions & 1 deletion packages/block-editor/src/components/provider/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class BlockEditorProvider extends Component {
const {
getBlocks,
isLastBlockChangePersistent,
__unstableIsLastBlockChangeIgnored,
} = registry.select( 'core/block-editor' );

let blocks = getBlocks();
Expand All @@ -81,7 +82,12 @@ class BlockEditorProvider extends Component {
} = this.props;
const newBlocks = getBlocks();
const newIsPersistent = isLastBlockChangePersistent();
if ( newBlocks !== blocks && this.isSyncingIncomingValue ) {
if (
newBlocks !== blocks && (
this.isSyncingIncomingValue ||
__unstableIsLastBlockChangeIgnored()
)
) {
this.isSyncingIncomingValue = false;
blocks = newBlocks;
isPersistent = newIsPersistent;
Expand Down
53 changes: 33 additions & 20 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,6 @@ 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 );

Expand All @@ -217,16 +207,6 @@ function withPersistentBlockChange( reducer ) {
};
}

// 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: false,
};
}

nextState = {
...nextState,
isPersistentChange: (
Expand All @@ -244,6 +224,38 @@ function withPersistentBlockChange( reducer ) {
};
}

/**
* Higher-order reducer intended to augment the blocks reducer, assigning an
* `isIgnoredChange` property value corresponding to whether a change in state
* can be considered as ignored. A change is considered ignored when the result
* of an action not incurred by direct user interaction.
*
* @param {Function} reducer Original reducer function.
*
* @return {Function} Enhanced reducer function.
*/
function withIgnoredBlockChange( reducer ) {
/**
* 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 ) => {
const nextState = reducer( state, action );

if ( nextState !== state ) {
nextState.isIgnoredChange = IGNORED_ACTION_TYPES.has( action.type );
}

return nextState;
};
}

/**
* Higher-order reducer targeting the combined blocks reducer, augmenting
* block client IDs in remove action to include cascade of inner blocks.
Expand Down Expand Up @@ -385,6 +397,7 @@ export const blocks = flow(
withBlockReset,
withSaveReusableBlock,
withPersistentBlockChange,
withIgnoredBlockChange,
)( {
byClientId( state = {}, action ) {
switch ( action.type ) {
Expand Down
19 changes: 19 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,25 @@ export function isLastBlockChangePersistent( state ) {
return state.blocks.isPersistentChange;
}

/**
* Returns true if the most recent block change is be considered ignored, or
* false otherwise. An ignored change is one not to be committed by
* BlockEditorProvider, neither via `onChange` nor `onInput`.
*
* TODO: Removal Plan: Changes incurred by RECEIVE_BLOCKS should not be ignored
* if in-fact they result in a change in blocks state. The current need to
* ignore changes incurred not as a result of user interaction should be
* accounted for in the refactoring of reusable blocks as occurring within
* their own separate block editor / state (#7119).
*
* @param {Object} state Block editor state.
*
* @return {boolean} Whether the most recent block change was ignored.
*/
export function __unstableIsLastBlockChangeIgnored( state ) {
return state.blocks.isIgnoredChange;
}

/**
* Returns the value of a post meta from the editor settings.
*
Expand Down
19 changes: 13 additions & 6 deletions packages/block-editor/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ describe( 'state', () => {
const state = blocks( existingState, action );

expect( state ).toEqual( {
isPersistentChange: expect.anything(),
isPersistentChange: true,
isIgnoredChange: false,
byClientId: {
clicken: {
clientId: 'chicken',
Expand Down Expand Up @@ -295,7 +296,8 @@ describe( 'state', () => {
const state = blocks( existingState, action );

expect( state ).toEqual( {
isPersistentChange: expect.anything(),
isPersistentChange: true,
isIgnoredChange: false,
byClientId: {
clicken: {
clientId: 'chicken',
Expand Down Expand Up @@ -386,7 +388,8 @@ describe( 'state', () => {
const state = blocks( existingState, action );

expect( state ).toEqual( {
isPersistentChange: expect.anything(),
isPersistentChange: true,
isIgnoredChange: false,
byClientId: {
clicken: {
clientId: 'chicken',
Expand Down Expand Up @@ -478,7 +481,8 @@ describe( 'state', () => {
const state = blocks( existingState, action );

expect( state ).toEqual( {
isPersistentChange: expect.anything(),
isPersistentChange: true,
isIgnoredChange: false,
byClientId: {
clicken: {
clientId: 'chicken',
Expand Down Expand Up @@ -512,6 +516,7 @@ describe( 'state', () => {
attributes: {},
order: {},
isPersistentChange: true,
isIgnoredChange: false,
} );
} );

Expand Down Expand Up @@ -1512,8 +1517,10 @@ describe( 'state', () => {

expect( state ).toBe( original );
} );
} );

it( 'should not consider received blocks as persistent change', () => {
describe( 'isIgnoredChange', () => {
it( 'should consider received blocks as ignored change', () => {
const state = blocks( undefined, {
type: 'RECEIVE_BLOCKS',
blocks: [ {
Expand All @@ -1523,7 +1530,7 @@ describe( 'state', () => {
} ],
} );

expect( state.isPersistentChange ).toBe( false );
expect( state.isIgnoredChange ).toBe( true );
} );
} );
} );
Expand Down
17 changes: 17 additions & 0 deletions packages/e2e-tests/specs/change-detection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,4 +288,21 @@ describe( 'Change detection', () => {

await assertIsDirty( true );
} );

it( 'should not prompt when receiving reusable blocks', async () => {
// Regression Test: Verify that non-modifying behaviors does not incur
// dirtiness. Previously, this could occur as a result of either (a)
// selecting a block, (b) opening the inserter, or (c) editing a post
// which contained a reusable block. The root issue was changes in
// block editor state as a result of reusable blocks data having been
// received, reflected here in this test.
//
// TODO: This should be considered a temporary test, existing only so
// long as the experimental reusable blocks fetching data flow exists.
//
// See: https://github.com/WordPress/gutenberg/issues/14766
await page.evaluate( () => window.wp.data.dispatch( 'core/editor' ).__experimentalReceiveReusableBlocks( [] ) );

await assertIsDirty( false );
} );
} );

0 comments on commit a35fa82

Please sign in to comment.