Block Editor: Separate onChange from onInput as committing change

packages/block-editor/src/components/provider/index.js

@@ -3,13 +3,125 @@
  */
 import { Component } from '@wordpress/element';
 import { DropZoneProvider, SlotFillProvider } from '@wordpress/components';
-import { withDispatch, withSelect } from '@wordpress/data';
-import { compose } from '@wordpress/compose';
+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 ) => (
+		<RegistryConsumer>
+			{ ( registry ) => (
+				<OriginalComponent
+					{ ...props }
+					registry={ registry }
+				/>
+			) }
+		</RegistryConsumer>
+	),
+	'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.
+ */
+function hasSameKeys( a, b ) {
+	return 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.
+ */
+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.editor.blocks !== lastState.editor.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;
+}
 
 class BlockEditorProvider extends Component {
+	constructor() {
+		super( ...arguments );
+
+		this.onChange = this.onChange.bind( this );
+	}
+
 	componentDidMount() {
 		this.props.updateEditorSettings( this.props.settings );
 		this.props.resetBlocks( this.props.value );
+		this.attachChangeObserver( this.props.registry );
 
 		this.isSyncingBlockValue = true;
 	}
@@ -21,24 +133,61 @@ class BlockEditorProvider extends Component {
 			value,
 			resetBlocks,
 			blocks,
-			onChange,
+			onInput,
+			registry,
 		} = 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 ) {
-			onChange( blocks );
+			onInput( blocks );
 			this.isSyncingBlockValue = true;
 		} else if ( value !== prevProps.value ) {
 			resetBlocks( value );
 			this.isSyncingBlockValue = true;
 		}
 	}
 
+	/**
+	 * 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;
 
@@ -71,4 +220,5 @@ export default compose( [
 			resetBlocks,
 		};
 	} ),
+	withRegistry,
 ] )( BlockEditorProvider );

packages/block-editor/src/components/provider/test/index.js

@@ -0,0 +1,95 @@
+/**
+ * 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 );
+	} );
+} );

packages/editor/src/components/provider/index.js

@@ -86,6 +86,7 @@ class EditorProvider extends Component {
 			meta,
 			onMetaChange,
 			reusableBlocks,
+			createUndoLevel,
 		} = this.props;
 
 		if ( ! isReady ) {
@@ -99,7 +100,8 @@ class EditorProvider extends Component {
 		return (
 			<BlockEditorProvider
 				value={ blocks }
-				onChange={ resetEditorBlocks }
+				onInput={ resetEditorBlocks }
+				onChange={ createUndoLevel }
 				settings={ editorSettings }
 			>
 				{ children }
@@ -129,6 +131,7 @@ export default compose( [
 			updatePostLock,
 			resetEditorBlocks,
 			editPost,
+			createUndoLevel,
 		} = dispatch( 'core/editor' );
 		const { createWarningNotice } = dispatch( 'core/notices' );
 
@@ -137,6 +140,7 @@ export default compose( [
 			updatePostLock,
 			createWarningNotice,
 			resetEditorBlocks,
+			createUndoLevel,
 			onMetaChange( meta ) {
 				editPost( { meta } );
 			},

packages/editor/src/store/reducer.js

@@ -129,7 +129,15 @@ export const editor = flow( [
 
 	withHistory( {
 		resetTypes: [ 'SETUP_EDITOR_STATE' ],
-		ignoreTypes: [ 'RECEIVE_BLOCKS', 'RESET_POST', 'UPDATE_POST' ],
+		ignoreTypes: [
+			'RECEIVE_BLOCKS',
+			'RESET_POST',
+			'UPDATE_POST',
+
+			// Blocks history is managed by explicit createUndoLevel actions
+			// occurring via the rendered BlockEditor's onChange callback.
+			'RESET_BLOCKS',
+		],
 		shouldOverwriteState,
 	} ),
 ] )( {