diff --git a/packages/block-editor/src/components/provider/index.js b/packages/block-editor/src/components/provider/index.js index d13917b0cfb04..9303612e64358 100644 --- a/packages/block-editor/src/components/provider/index.js +++ b/packages/block-editor/src/components/provider/index.js @@ -3,126 +3,14 @@ */ import { Component } from '@wordpress/element'; import { DropZoneProvider, SlotFillProvider } from '@wordpress/components'; -import { withDispatch, withSelect, RegistryConsumer } from '@wordpress/data'; -import { compose, createHigherOrderComponent } from '@wordpress/compose'; -import isShallowEqual from '@wordpress/is-shallow-equal'; - -/** - * Higher-order component which renders the original component with the current - * registry context passed as its `registry` prop. - * - * @param {WPComponent} OriginalComponent Original component. - * - * @return {WPComponent} Enhanced component. - */ -const withRegistry = createHigherOrderComponent( - ( OriginalComponent ) => ( props ) => ( - - { ( registry ) => ( - - ) } - - ), - 'withRegistry' -); - -/** - * Returns true if the two object arguments have the same keys, or false - * otherwise. - * - * @param {Object} a First object. - * @param {Object} b Second object. - * - * @return {boolean} Whether the two objects have the same keys. - */ -export function hasSameKeys( a, b ) { - return !! a && !! b && isShallowEqual( Object.keys( a ), Object.keys( b ) ); -} - -/** - * Returns true if, given the currently dispatching action and the previously - * dispatched action, the two actions are updating the same block attribute, or - * false otherwise. - * - * @param {Object} action Currently dispatching action. - * @param {Object} lastAction Previously dispatched action. - * - * @return {boolean} Whether actions are updating the same block attribute. - */ -export function isUpdatingSameBlockAttribute( action, lastAction ) { - return ( - action.type === 'UPDATE_BLOCK_ATTRIBUTES' && - action.clientId === lastAction.clientId && - hasSameKeys( action.attributes, lastAction.attributes ) - ); -} - -/** - * Given a data namespace store and a callback, returns a substitute dispatch - * function which preserves the original dispatch behavior and invokes the - * callback when a blocks state change should be committed. - * - * @param {WPDataNamespaceStore} store Store for which to create replacement - * dispatch function. - * @param {Function} callback Function to call when blocks state - * should be committed. - * - * @return {Function} Enhanced store dispatch function. - */ -function createChangeObserver( store, callback ) { - let lastAction, lastState, isPendingCommit; - - function dispatch( action ) { - const result = dispatch._originalDispatch( action ); - const state = store.getState(); - - if ( action.type === 'RESET_BLOCKS' ) { - // Consider block reset as superseding any pending change commits, - // even if destructive to pending user commits. It should rarely be - // the case that blocks are suddenly reset while user interacts. - isPendingCommit = false; - } else if ( lastAction && lastState && state !== lastState ) { - if ( - state.blocks !== lastState.blocks && - isUpdatingSameBlockAttribute( action, lastAction ) - ) { - // So long as block updates occur as operating on the same - // attributes in the previous action, delay callback. - isPendingCommit = true; - } else if ( isPendingCommit ) { - // Once any other action occurs while pending commit, release - // the deferred callback as completed. - callback(); - isPendingCommit = false; - } - } - - lastAction = action; - lastState = state; - - return result; - } - - dispatch._originalDispatch = store.dispatch; - - return dispatch; -} +import { withDispatch, withSelect } from '@wordpress/data'; +import { compose } from '@wordpress/compose'; class BlockEditorProvider extends Component { - constructor() { - super( ...arguments ); - - this.onChange = this.onChange.bind( this ); - } - componentDidMount() { this.isSyncingBlockValue = true; this.props.updateEditorSettings( this.props.settings ); this.props.resetBlocks( this.props.value ); - this.attachChangeObserver( this.props.registry ); } componentDidUpdate( prevProps ) { @@ -133,60 +21,29 @@ class BlockEditorProvider extends Component { resetBlocks, blocks, onInput, - registry, + onChange, + isLastBlockChangePersistent, } = this.props; if ( settings !== prevProps.settings ) { updateEditorSettings( settings ); } - if ( registry !== prevProps.registry ) { - this.attachChangeObserver( registry, prevProps.registry ); - } - if ( this.isSyncingBlockValue ) { this.isSyncingBlockValue = false; } else if ( blocks !== prevProps.blocks ) { this.isSyncingBlockValue = true; onInput( blocks ); + + if ( isLastBlockChangePersistent ) { + onChange( blocks ); + } } else if ( value !== prevProps.value ) { this.isSyncingBlockValue = true; resetBlocks( value ); } } - /** - * Calls the mounted instance's `onChange` prop callback with the current - * blocks prop value. - */ - onChange() { - this.props.onChange( this.props.blocks ); - } - - /** - * Given a registry object, overrides the default dispatch behavior for the - * `core/block-editor` store to interpret a state change which should be - * considered as calling the mounted instance's `onChange` callback. Unlike - * `onInput` which is called for any change in block state, `onChange` is - * only called for meaningful commit interactions. If a second registry - * argument is passed, it is treated as the previous registry to which the - * dispatch behavior was overridden, and the original dispatch is restored. - * - * @param {WPDataRegistry} registry Registry from which block editor - * dispatch is to be overriden. - * @param {WPDataRegistry} prevRegistry Previous registry whose dispatch - * behavior should be restored. - */ - attachChangeObserver( registry, prevRegistry ) { - const { store } = registry.namespaces[ 'core/block-editor' ]; - store.dispatch = createChangeObserver( store, this.onChange ); - - if ( prevRegistry ) { - const { store: prevStore } = prevRegistry.namespaces[ 'core/block-editor' ]; - prevStore.dispatch = prevStore.dispatch._originalDispatch; - } - } - render() { const { children } = this.props; @@ -202,10 +59,14 @@ class BlockEditorProvider extends Component { export default compose( [ withSelect( ( select ) => { - const { getBlocks } = select( 'core/block-editor' ); + const { + getBlocks, + isLastBlockChangePersistent, + } = select( 'core/block-editor' ); return { blocks: getBlocks(), + isLastBlockChangePersistent: isLastBlockChangePersistent(), }; } ), withDispatch( ( dispatch ) => { @@ -219,5 +80,4 @@ export default compose( [ resetBlocks, }; } ), - withRegistry, ] )( BlockEditorProvider ); diff --git a/packages/block-editor/src/components/provider/test/index.js b/packages/block-editor/src/components/provider/test/index.js deleted file mode 100644 index 8dbc266fdceeb..0000000000000 --- a/packages/block-editor/src/components/provider/test/index.js +++ /dev/null @@ -1,95 +0,0 @@ -/** - * Internal dependencies - */ -import { - hasSameKeys, - isUpdatingSameBlockAttribute, -} from '../'; - -describe( 'hasSameKeys()', () => { - it( 'returns false if two objects do not have the same keys', () => { - const a = { foo: 10 }; - const b = { bar: 10 }; - - expect( hasSameKeys( a, b ) ).toBe( false ); - } ); - - it( 'returns false if two objects have the same keys', () => { - const a = { foo: 10 }; - const b = { foo: 20 }; - - expect( hasSameKeys( a, b ) ).toBe( true ); - } ); -} ); - -describe( 'isUpdatingSameBlockAttribute()', () => { - it( 'should return false if not updating block attributes', () => { - const action = { - type: 'START_TYPING', - edits: {}, - }; - const previousAction = { - type: 'START_TYPING', - edits: {}, - }; - - expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( false ); - } ); - - it( 'should return false if not updating the same block', () => { - const action = { - type: 'UPDATE_BLOCK_ATTRIBUTES', - clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', - attributes: { - foo: 10, - }, - }; - const previousAction = { - type: 'UPDATE_BLOCK_ATTRIBUTES', - clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', - attributes: { - foo: 20, - }, - }; - - expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( false ); - } ); - - it( 'should return false if not updating the same block attributes', () => { - const action = { - type: 'UPDATE_BLOCK_ATTRIBUTES', - clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', - attributes: { - foo: 10, - }, - }; - const previousAction = { - type: 'UPDATE_BLOCK_ATTRIBUTES', - clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', - attributes: { - bar: 20, - }, - }; - - expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( false ); - } ); - - it( 'should return true if updating the same block attributes', () => { - const action = { - type: 'UPDATE_BLOCK_ATTRIBUTES', - clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', - attributes: { - foo: 10, - }, - }; - const previousAction = { - type: 'UPDATE_BLOCK_ATTRIBUTES', - clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', - attributes: { - foo: 20, - }, - }; - - expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( true ); - } ); -} ); diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 20bc84618a1ec..0d871cf5b630d 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -141,7 +141,65 @@ function getMutateSafeObject( original, working ) { } /** - * Higher-order reducer targeting the combined editor reducer, augmenting + * Returns true if the two object arguments have the same keys, or false + * otherwise. + * + * @param {Object} a First object. + * @param {Object} b Second object. + * + * @return {boolean} Whether the two objects have the same keys. + */ +export function hasSameKeys( a, b ) { + return isEqual( Object.keys( a ), Object.keys( b ) ); +} + +/** + * Returns true if, given the currently dispatching action and the previously + * dispatched action, the two actions are updating the same block attribute, or + * false otherwise. + * + * @param {Object} action Currently dispatching action. + * @param {Object} lastAction Previously dispatched action. + * + * @return {boolean} Whether actions are updating the same block attribute. + */ +export function isUpdatingSameBlockAttribute( action, lastAction ) { + return ( + action.type === 'UPDATE_BLOCK_ATTRIBUTES' && + action.clientId === lastAction.clientId && + hasSameKeys( action.attributes, lastAction.attributes ) + ); +} + +/** + * Higher-order reducer intended to augment the blocks reducer, assigning an + * `isPersistentChange` property value corresponding to whether a change in + * state can be considered as persistent. All changes are considered persistent + * except when updating the same block attribute as in the previous action. + * + * @param {Function} reducer Original reducer function. + * + * @return {Function} Enhanced reducer function. + */ +function withPersistentBlockChange( reducer ) { + let lastAction; + + return ( state, action ) => { + const nextState = reducer( state, action ); + if ( state !== nextState ) { + nextState.isPersistentChange = ( + ! isUpdatingSameBlockAttribute( action, lastAction ) + ); + } + + lastAction = action; + + return nextState; + }; +} + +/** + * Higher-order reducer targeting the combined blocks reducer, augmenting * block client IDs in remove action to include cascade of inner blocks. * * @param {Function} reducer Original reducer function. @@ -235,8 +293,7 @@ const withSaveReusableBlock = ( reducer ) => ( state, action ) => { }; /** - * Reducer returning the editor post state, including blocks parsed from - * current HTML markup. + * Reducer returning the blocks state. * * @param {Object} state Current state. * @param {Object} action Dispatched action. @@ -248,6 +305,7 @@ export const blocks = flow( withInnerBlocksRemoveCascade, withBlockReset, withSaveReusableBlock, + withPersistentBlockChange, )( { 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 dbca183a9e7fa..b3c6f7bd6701d 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -1346,6 +1346,19 @@ export function getEditorSettings( state ) { return state.settings; } +/** + * Returns true if the most recent block change is be considered persistent, or + * false otherwise. A persistent change is one committed by BlockEditorProvider + * via its `onChange` callback, in addition to `onInput`. + * + * @param {Object} state Block editor state. + * + * @return {boolean} Whether the most recent block change was persistent. + */ +export function isLastBlockChangePersistent( state ) { + return state.blocks.isPersistentChange; +} + /** * 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 17da02f344d9f..282314eb9c804 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -17,6 +17,8 @@ import { * Internal dependencies */ import { + hasSameKeys, + isUpdatingSameBlockAttribute, blocks, isTyping, isCaretWithinFormattedText, @@ -29,6 +31,94 @@ import { } from '../reducer'; describe( 'state', () => { + describe( 'hasSameKeys()', () => { + it( 'returns false if two objects do not have the same keys', () => { + const a = { foo: 10 }; + const b = { bar: 10 }; + + expect( hasSameKeys( a, b ) ).toBe( false ); + } ); + + it( 'returns false if two objects have the same keys', () => { + const a = { foo: 10 }; + const b = { foo: 20 }; + + expect( hasSameKeys( a, b ) ).toBe( true ); + } ); + } ); + + describe( 'isUpdatingSameBlockAttribute()', () => { + it( 'should return false if not updating block attributes', () => { + const action = { + type: 'EDIT_POST', + edits: {}, + }; + const previousAction = { + type: 'EDIT_POST', + edits: {}, + }; + + expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( false ); + } ); + + it( 'should return false if not updating the same block', () => { + const action = { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', + attributes: { + foo: 10, + }, + }; + const previousAction = { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', + attributes: { + foo: 20, + }, + }; + + expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( false ); + } ); + + it( 'should return false if not updating the same block attributes', () => { + const action = { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', + attributes: { + foo: 10, + }, + }; + const previousAction = { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', + attributes: { + bar: 20, + }, + }; + + expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( false ); + } ); + + it( 'should return true if updating the same block attributes', () => { + const action = { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', + attributes: { + foo: 10, + }, + }; + const previousAction = { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189', + attributes: { + foo: 20, + }, + }; + + expect( isUpdatingSameBlockAttribute( action, previousAction ) ).toBe( true ); + } ); + } ); + describe( 'blocks()', () => { beforeAll( () => { registerBlockType( 'core/test-block', { @@ -43,11 +133,15 @@ describe( 'state', () => { unregisterBlockType( 'core/test-block' ); } ); - it( 'should return empty edits, blocks by default', () => { + it( 'should return empty byClientId, attributes, order by default', () => { const state = blocks( undefined, {} ); - expect( state.byClientId ).toEqual( {} ); - expect( state.order ).toEqual( {} ); + expect( state ).toEqual( { + byClientId: {}, + attributes: {}, + order: {}, + isPersistentChange: true, + } ); } ); it( 'should key by reset blocks clientId', () => { @@ -921,6 +1015,53 @@ describe( 'state', () => { expect( state.attributes ).toBe( state.attributes ); } ); } ); + + describe( 'isPersistentChange', () => { + it( 'should consider any non-exempt block change as persistent', () => { + const original = deepFreeze( blocks( undefined, { + type: 'RESET_BLOCKS', + blocks: [], + } ) ); + + const state = blocks( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: true, + }, + } ); + + expect( state.isPersistentChange ).toBe( true ); + } ); + + it( 'should consider same block attribute update as exempt', () => { + const original = deepFreeze( blocks( undefined, { + type: 'RESET_BLOCKS', + blocks: [ { + clientId: 'kumquat', + attributes: {}, + innerBlocks: [], + } ], + } ) ); + let state = blocks( original, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: false, + }, + } ); + + state = blocks( state, { + type: 'UPDATE_BLOCK_ATTRIBUTES', + clientId: 'kumquat', + attributes: { + updated: true, + }, + } ); + + expect( state.isPersistentChange ).toBe( false ); + } ); + } ); } ); } );