RichText: own undo signalling (#10650)

* Own undo history

* Adjust e2e test

* Add undo level after input timeout

* Remove _withoutApply

packages/editor/src/components/rich-text/index.js

@@ -3,7 +3,6 @@
  */
 import classnames from 'classnames';
 import {
-	defer,
 	find,
 	isNil,
 	isEqual,
@@ -21,7 +20,7 @@ import {
 	getScrollContainer,
 } from '@wordpress/dom';
 import { createBlobURL } from '@wordpress/blob';
-import { BACKSPACE, DELETE, ENTER, rawShortcut } from '@wordpress/keycodes';
+import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes';
 import { withDispatch, withSelect } from '@wordpress/data';
 import { pasteHandler, children, getBlockTransforms, findTransform } from '@wordpress/blocks';
 import { withInstanceId, withSafeTimeout, compose } from '@wordpress/compose';
@@ -61,6 +60,7 @@ import { pickAriaProps } from './aria';
 import { getPatterns } from './patterns';
 import { withBlockEditContext } from '../block-edit/context';
 import { ListEdit } from './list-edit';
+import { RemoveBrowserShortcuts } from './remove-browser-shortcuts';
 
 /**
  * Browser dependencies
@@ -91,15 +91,13 @@ export class RichText extends Component {
 			this.onSplit = this.props.unstableOnSplit;
 		}
 
-		this.onInit = this.onInit.bind( this );
 		this.onSetup = this.onSetup.bind( this );
 		this.onFocus = this.onFocus.bind( this );
 		this.onChange = this.onChange.bind( this );
 		this.onNodeChange = this.onNodeChange.bind( this );
 		this.onDeleteKeyDown = this.onDeleteKeyDown.bind( this );
 		this.onKeyDown = this.onKeyDown.bind( this );
 		this.onKeyUp = this.onKeyUp.bind( this );
-		this.onPropagateUndo = this.onPropagateUndo.bind( this );
 		this.onPaste = this.onPaste.bind( this );
 		this.onCreateUndoLevel = this.onCreateUndoLevel.bind( this );
 		this.setFocusedElement = this.setFocusedElement.bind( this );
@@ -129,6 +127,7 @@ export class RichText extends Component {
 		this.state = {};
 
 		this.usedDeprecatedChildrenSource = Array.isArray( value );
+		this.lastHistoryValue = value;
 	}
 
 	componentDidMount() {
@@ -157,12 +156,7 @@ export class RichText extends Component {
 	 */
 	onSetup( editor ) {
 		this.editor = editor;
-
-		editor.on( 'init', this.onInit );
 		editor.on( 'nodechange', this.onNodeChange );
-		editor.on( 'BeforeExecCommand', this.onPropagateUndo );
-		// The change event in TinyMCE fires every time an undo level is added.
-		editor.on( 'change', this.onCreateUndoLevel );
 	}
 
 	setFocusedElement() {
@@ -171,35 +165,6 @@ export class RichText extends Component {
 		}
 	}
 
-	onInit() {
-		this.editor.shortcuts.add( rawShortcut.primary( 'z' ), '', 'Undo' );
-		this.editor.shortcuts.add( rawShortcut.primaryShift( 'z' ), '', 'Redo' );
-
-		// Remove TinyMCE Core shortcut for consistency with global editor
-		// shortcuts. Also clashes with Mac browsers.
-		this.editor.shortcuts.remove( 'meta+y', '', 'Redo' );
-	}
-
-	/**
-	 * Handles an undo event from TinyMCE.
-	 *
-	 * @param {UndoEvent} event The undo event as triggered by TinyMCE.
-	 */
-	onPropagateUndo( event ) {
-		const { onUndo, onRedo } = this.props;
-		const { command } = event;
-
-		if ( command === 'Undo' && onUndo ) {
-			defer( onUndo );
-			event.preventDefault();
-		}
-
-		if ( command === 'Redo' && onRedo ) {
-			defer( onRedo );
-			event.preventDefault();
-		}
-	}
-
 	/**
 	 * Get the current record (value and selection) from props and state.
 	 *
@@ -411,7 +376,13 @@ export class RichText extends Component {
 		const record = this.createRecord();
 		const transformed = this.patterns.reduce( ( accumlator, transform ) => transform( accumlator ), record );
 
-		this.onChange( transformed );
+		this.onChange( transformed, {
+			withoutHistory: true,
+		} );
+
+		// Create an undo level when input stops for over a second.
+		this.props.clearTimeout( this.onInput.timeout );
+		this.onInput.timeout = this.props.setTimeout( this.onCreateUndoLevel, 1000 );
 	}
 
 	onCompositionEnd() {
@@ -447,42 +418,33 @@ export class RichText extends Component {
 	 * Sync the value to global state. The node tree and selection will also be
 	 * updated if differences are found.
 	 *
-	 * @param {Object}  record        The record to sync and apply.
-	 * @param {boolean} _withoutApply If true, the record won't be applied to
-	 *                                the live DOM.
+	 * @param {Object}  record            The record to sync and apply.
+	 * @param {Object}  $2                Named options.
+	 * @param {boolean} $2.withoutHistory If true, no undo level will be
+	 *                                    created.
 	 */
-	onChange( record, _withoutApply ) {
-		if ( ! _withoutApply ) {
-			this.applyRecord( record );
-		}
+	onChange( record, { withoutHistory } = {} ) {
+		this.applyRecord( record );
 
 		const { start, end } = record;
 
 		this.savedContent = this.valueToFormat( record );
 		this.props.onChange( this.savedContent );
 		this.setState( { start, end } );
-	}
 
-	onCreateUndoLevel( event ) {
-		// TinyMCE fires a `change` event when the first letter in an instance
-		// is typed. This should not create a history record in Gutenberg.
-		// https://github.com/tinymce/tinymce/blob/4.7.11/src/core/main/ts/api/UndoManager.ts#L116-L125
-		// In other cases TinyMCE won't fire a `change` with at least a previous
-		// record present, so this is a reliable check.
-		// https://github.com/tinymce/tinymce/blob/4.7.11/src/core/main/ts/api/UndoManager.ts#L272-L275
-		if ( event && event.lastLevel === null ) {
-			return;
+		if ( ! withoutHistory ) {
+			this.onCreateUndoLevel();
 		}
+	}
 
-		// Always ensure the content is up-to-date. This is needed because e.g.
-		// making something bold will trigger a TinyMCE change event but no
-		// input event. Avoid dispatching an action if the original event is
-		// blur because the content will already be up-to-date.
-		if ( ! event || ! event.originalEvent || event.originalEvent.type !== 'blur' ) {
-			this.onChange( this.createRecord(), true );
+	onCreateUndoLevel() {
+		// If the content is the same, no level needs to be created.
+		if ( this.lastHistoryValue === this.savedContent ) {
+			return;
 		}
 
 		this.props.onCreateUndoLevel();
+		this.lastHistoryValue = this.savedContent;
 	}
 
 	/**
@@ -641,7 +603,7 @@ export class RichText extends Component {
 		// The input event does not fire when the whole field is selected and
 		// BACKSPACE is pressed.
 		if ( keyCode === BACKSPACE ) {
-			this.onChange( this.createRecord(), true );
+			this.onChange( this.createRecord() );
 		}
 
 		// `scrollToRect` is called on `nodechange`, whereas calling it on
@@ -963,6 +925,7 @@ export class RichText extends Component {
 						</Fragment>
 					) }
 				</Autocomplete>
+				{ isSelected && <RemoveBrowserShortcuts /> }
 			</div>
 		);
 	}

packages/editor/src/components/rich-text/remove-browser-shortcuts.js

@@ -0,0 +1,45 @@
+/**
+ * External dependencies
+ */
+import { fromPairs } from 'lodash';
+
+/**
+ * WordPress dependencies
+ */
+import { rawShortcut } from '@wordpress/keycodes';
+import { KeyboardShortcuts } from '@wordpress/components';
+
+/**
+ * Set of keyboard shortcuts handled internally by RichText.
+ *
+ * @type {Array}
+ */
+const HANDLED_SHORTCUTS = [
+	rawShortcut.primary( 'z' ),
+	rawShortcut.primaryShift( 'z' ),
+	rawShortcut.primary( 'y' ),
+];
+
+/**
+ * An instance of a KeyboardShortcuts element pre-bound for the handled
+ * shortcuts. Since shortcuts never change, the element can be considered
+ * static, and can be skipped in reconciliation.
+ *
+ * @type {WPElement}
+ */
+const SHORTCUTS_ELEMENT = (
+	<KeyboardShortcuts
+		bindGlobal
+		shortcuts={ fromPairs( HANDLED_SHORTCUTS.map( ( shortcut ) => {
+			return [ shortcut, ( event ) => event.preventDefault() ];
+		} ) ) }
+	/>
+);
+
+/**
+ * Component which registered keyboard event handlers to prevent default
+ * behaviors for key combinations otherwise handled internally by RichText.
+ *
+ * @return {WPElement} WordPress element.
+ */
+export const RemoveBrowserShortcuts = () => SHORTCUTS_ELEMENT;

packages/editor/src/components/rich-text/tinymce.js

@@ -221,14 +221,26 @@ export default class TinyMCE extends Component {
 				} );
 
 				editor.on( 'init', () => {
-					// See https://github.com/tinymce/tinymce/blob/master/src/core/main/ts/keyboard/FormatShortcuts.ts
+					// History is handled internally by RichText.
+					//
+					// See: https://github.com/tinymce/tinymce/blob/master/src/core/main/ts/api/UndoManager.ts
+					[ 'z', 'y' ].forEach( ( character ) => {
+						editor.shortcuts.remove( `meta+${ character }` );
+					} );
+					editor.shortcuts.remove( 'meta+shift+z' );
+
+					// Reset TinyMCE's default formatting shortcuts, since
+					// RichText supports only registered formats.
+					//
+					// See: https://github.com/tinymce/tinymce/blob/master/src/core/main/ts/keyboard/FormatShortcuts.ts
 					[ 'b', 'i', 'u' ].forEach( ( character ) => {
 						editor.shortcuts.remove( `meta+${ character }` );
 					} );
 					[ 1, 2, 3, 4, 5, 6, 7, 8, 9 ].forEach( ( number ) => {
 						editor.shortcuts.remove( `access+${ number }` );
 					} );
 
+					// Restore the original `setHTML` once initialized.
 					editor.dom.setHTML = setHTML;
 				} );
 

test/e2e/specs/__snapshots__/undo.test.js.snap

@@ -13,3 +13,27 @@ exports[`undo Should undo to expected level intervals 1`] = `
 <p>test</p>
 <!-- /wp:paragraph -->"
 `;
+
+exports[`undo should undo typing after a pause 1`] = `
+"<!-- wp:paragraph -->
+<p>before pause after pause</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`undo should undo typing after a pause 2`] = `
+"<!-- wp:paragraph -->
+<p>before pause</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`undo should undo typing after non input change 1`] = `
+"<!-- wp:paragraph -->
+<p>before keyboard <strong>after keyboard</strong></p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`undo should undo typing after non input change 2`] = `
+"<!-- wp:paragraph -->
+<p>before keyboard </p>
+<!-- /wp:paragraph -->"
+`;

test/e2e/specs/undo.test.js

@@ -10,10 +10,38 @@ import {
 } from '../support/utils';
 
 describe( 'undo', () => {
-	beforeAll( async () => {
+	beforeEach( async () => {
 		await newPost();
 	} );
 
+	it( 'should undo typing after a pause', async () => {
+		await clickBlockAppender();
+
+		await page.keyboard.type( 'before pause' );
+		await new Promise( ( resolve ) => setTimeout( resolve, 1000 ) );
+		await page.keyboard.type( ' after pause' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+
+		await pressWithModifier( META_KEY, 'z' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
+
+	it( 'should undo typing after non input change', async () => {
+		await clickBlockAppender();
+
+		await page.keyboard.type( 'before keyboard ' );
+		await pressWithModifier( META_KEY, 'b' );
+		await page.keyboard.type( 'after keyboard' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+
+		await pressWithModifier( META_KEY, 'z' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
+
 	it( 'Should undo to expected level intervals', async () => {
 		await clickBlockAppender();