From a35fa8269343a20c451669cc2a31307db665ce9b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 10 Apr 2019 15:54:03 -0400 Subject: [PATCH] Block Editor: Consider received blocks state change as ignored --- packages/block-editor/CHANGELOG.md | 1 + .../src/components/provider/index.js | 8 ++- packages/block-editor/src/store/reducer.js | 53 ++++++++++++------- packages/block-editor/src/store/selectors.js | 19 +++++++ .../block-editor/src/store/test/reducer.js | 19 ++++--- .../e2e-tests/specs/change-detection.test.js | 17 ++++++ 6 files changed, 90 insertions(+), 27 deletions(-) diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md index bc6e7685a126f..c275c7319f9c4 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -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) diff --git a/packages/block-editor/src/components/provider/index.js b/packages/block-editor/src/components/provider/index.js index f321b19dc1de8..9841f79d509a9 100644 --- a/packages/block-editor/src/components/provider/index.js +++ b/packages/block-editor/src/components/provider/index.js @@ -69,6 +69,7 @@ class BlockEditorProvider extends Component { const { getBlocks, isLastBlockChangePersistent, + __unstableIsLastBlockChangeIgnored, } = registry.select( 'core/block-editor' ); let blocks = getBlocks(); @@ -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; diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index d643dca7cb65c..9813b9e93bd57 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -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 ); @@ -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: ( @@ -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. @@ -385,6 +397,7 @@ export const blocks = flow( withBlockReset, withSaveReusableBlock, withPersistentBlockChange, + withIgnoredBlockChange, )( { byClientId( state = {}, action ) { switch ( action.type ) { diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index fb431d1a48d0f..005744bc023fe 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -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. * diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index c1fff7d03deda..fe63fc34cd65b 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -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', @@ -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', @@ -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', @@ -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', @@ -512,6 +516,7 @@ describe( 'state', () => { attributes: {}, order: {}, isPersistentChange: true, + isIgnoredChange: false, } ); } ); @@ -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: [ { @@ -1523,7 +1530,7 @@ describe( 'state', () => { } ], } ); - expect( state.isPersistentChange ).toBe( false ); + expect( state.isIgnoredChange ).toBe( true ); } ); } ); } ); diff --git a/packages/e2e-tests/specs/change-detection.test.js b/packages/e2e-tests/specs/change-detection.test.js index 0e529a95ca512..7c9dab21d4d79 100644 --- a/packages/e2e-tests/specs/change-detection.test.js +++ b/packages/e2e-tests/specs/change-detection.test.js @@ -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 ); + } ); } );