Proposal: history "buffer"/overwrite, sync RichText history records (#…

…4956)

* Overwrite undo + editable signal

* Prevent TinyMCE from accumulating history

* Setting content is no longer needed on split if state is in sync

* When setting editor content, do not create an undo level

* Add context.onRedo for RichText

* Fix failing test

* Simplify history reducer

* Rewrite shouldOverwriteState

* Rewrite tests

* Adjust hasEditorUndo

* Minimise diff

* Leave comment for backspace onChange

* Use change event instead of addundo which fires during init

* Remove dirty references

* Restore updateContent

* Avoid wastefully calculating nextPast

blocks/rich-text/index.js

@@ -1,7 +1,6 @@
 /**
  * External dependencies
  */
-import tinymce from 'tinymce';
 import classnames from 'classnames';
 import {
 	last,
@@ -131,22 +130,22 @@ export class RichText extends Component {
 		this.getSettings = this.getSettings.bind( this );
 		this.onSetup = this.onSetup.bind( this );
 		this.onChange = this.onChange.bind( this );
-		this.onInput = this.onChange.bind( this, false );
 		this.onNewBlock = this.onNewBlock.bind( this );
 		this.onNodeChange = this.onNodeChange.bind( this );
 		this.onKeyDown = this.onKeyDown.bind( this );
 		this.onKeyUp = this.onKeyUp.bind( this );
 		this.changeFormats = this.changeFormats.bind( this );
-		this.onSelectionChange = this.onSelectionChange.bind( this );
-		this.maybePropagateUndo = this.maybePropagateUndo.bind( this );
+		this.onPropagateUndo = this.onPropagateUndo.bind( this );
 		this.onPastePreProcess = this.onPastePreProcess.bind( this );
 		this.onPaste = this.onPaste.bind( this );
+		this.onCreateUndoLevel = this.onCreateUndoLevel.bind( this );
 
 		this.state = {
 			formats: {},
-			empty: ! value || ! value.length,
 			selectedNodeId: 0,
 		};
+
+		this.isEmpty = ! value || ! value.length;
 	}
 
 	/**
@@ -161,6 +160,9 @@ export class RichText extends Component {
 		return ( this.props.getSettings || identity )( {
 			...settings,
 			forced_root_block: this.props.multiline || false,
+			// Allow TinyMCE to keep one undo level for comparing changes.
+			// Prevent it otherwise from accumulating any history.
+			custom_undo_redo_levels: 1,
 		} );
 	}
 
@@ -180,16 +182,16 @@ export class RichText extends Component {
 		} );
 
 		editor.on( 'init', this.onInit );
-		editor.on( 'focusout', this.onChange );
 		editor.on( 'NewBlock', this.onNewBlock );
 		editor.on( 'nodechange', this.onNodeChange );
 		editor.on( 'keydown', this.onKeyDown );
 		editor.on( 'keyup', this.onKeyUp );
-		editor.on( 'selectionChange', this.onSelectionChange );
-		editor.on( 'BeforeExecCommand', this.maybePropagateUndo );
+		editor.on( 'BeforeExecCommand', this.onPropagateUndo );
 		editor.on( 'PastePreProcess', this.onPastePreProcess, true /* Add before core handlers */ );
 		editor.on( 'paste', this.onPaste, true /* Add before core handlers */ );
-		editor.on( 'input', this.onInput );
+		editor.on( 'input', this.onChange );
+		// The change event in TinyMCE fires every time an undo level is added.
+		editor.on( 'change', this.onCreateUndoLevel );
 
 		patterns.apply( this, [ editor ] );
 
@@ -242,42 +244,23 @@ export class RichText extends Component {
 		} );
 	}
 
-	/**
-	 * Handles the global selection change event.
-	 */
-	onSelectionChange() {
-		const isActive = document.activeElement === this.editor.getBody();
-		// We must check this because selectionChange is a global event.
-		if ( ! isActive ) {
-			return;
-		}
-
-		const isEmpty = tinymce.DOM.isEmpty( this.editor.getBody() );
-		if ( this.state.empty !== isEmpty ) {
-			this.setState( { empty: isEmpty } );
-		}
-	}
-
 	/**
 	 * Handles an undo event from tinyMCE.
 	 *
-	 * When user attempts Undo when empty Undo stack, propagate undo
-	 * action to context handler. The compromise here is that: TinyMCE
-	 * handles Undo until change, at which point `editor.save` resets
-	 * history. If no history exists, let context handler have a turn.
-	 * Defer in case an immediate undo causes TinyMCE to be destroyed,
-	 * if other undo behaviors test presence of an input field.
-	 *
-	 * @param {UndoEvent} event The undo event as triggered by tinyMCE.
+	 * @param {UndoEvent} event The undo event as triggered by TinyMCE.
 	 */
-	maybePropagateUndo( event ) {
-		const { onUndo } = this.context;
-		if ( onUndo && event.command === 'Undo' && ! this.editor.undoManager.hasUndo() ) {
+	onPropagateUndo( event ) {
+		const { onUndo, onRedo } = this.context;
+		const { command } = event;
+
+		if ( command === 'Undo' && onUndo ) {
 			defer( onUndo );
+			event.preventDefault();
+		}
 
-			// We could return false here to stop other TinyMCE event handlers
-			// from running, but we assume TinyMCE won't do anything on an
-			// empty undo stack anyways.
+		if ( command === 'Redo' && onRedo ) {
+			defer( onRedo );
+			event.preventDefault();
 		}
 	}
 
@@ -409,17 +392,18 @@ export class RichText extends Component {
 
 	/**
 	 * Handles any case where the content of the tinyMCE instance has changed.
-	 *
-	 * @param {boolean} checkIfDirty Check whether the editor is dirty before calling onChange.
 	 */
-	onChange( checkIfDirty = true ) {
-		if ( checkIfDirty && ! this.editor.isDirty() ) {
-			return;
-		}
-		const isEmpty = tinymce.DOM.isEmpty( this.editor.getBody() );
-		this.savedContent = isEmpty ? [] : this.getContent();
+
+	onChange() {
+		this.isEmpty = this.editor.dom.isEmpty( this.editor.getBody() );
+		this.savedContent = this.isEmpty ? [] : this.getContent();
 		this.props.onChange( this.savedContent );
-		this.editor.save();
+	}
+
+	onCreateUndoLevel() {
+		// Always ensure the content is up-to-date.
+		this.onChange();
+		this.context.onCreateUndoLevel();
 	}
 
 	/**
@@ -547,7 +531,7 @@ export class RichText extends Component {
 				return;
 			}
 
-			this.onChange( false );
+			this.onCreateUndoLevel();
 
 			const forward = event.keyCode === DELETE;
 
@@ -586,6 +570,7 @@ export class RichText extends Component {
 				}
 
 				event.preventDefault();
+				this.onCreateUndoLevel();
 
 				const childNodes = Array.from( rootNode.childNodes );
 				const index = dom.nodeIndex( selectedNode );
@@ -594,10 +579,10 @@ export class RichText extends Component {
 				const beforeElement = nodeListToReact( beforeNodes, createTinyMCEElement );
 				const afterElement = nodeListToReact( afterNodes, createTinyMCEElement );
 
-				this.setContent( beforeElement );
 				this.props.onSplit( beforeElement, afterElement );
 			} else {
 				event.preventDefault();
+				this.onCreateUndoLevel();
 
 				if ( event.shiftKey || ! this.props.onSplit ) {
 					this.editor.execCommand( 'InsertLineBreak', false, event );
@@ -614,8 +599,10 @@ export class RichText extends Component {
 	 * @param {number} keyCode The key code that has been pressed on the keyboard.
 	 */
 	onKeyUp( { keyCode } ) {
+		// The input event does not fire when the whole field is selected and
+		// BACKSPACE is pressed.
 		if ( keyCode === BACKSPACE ) {
-			this.onSelectionChange();
+			this.onChange();
 		}
 	}
 
@@ -651,10 +638,8 @@ export class RichText extends Component {
 
 			const beforeElement = nodeListToReact( beforeFragment.childNodes, createTinyMCEElement );
 			const afterElement = nodeListToReact( filterEmptyNodes( afterFragment.childNodes ), createTinyMCEElement );
-			this.setContent( beforeElement );
 			this.props.onSplit( beforeElement, afterElement, ...blocks );
 		} else {
-			this.setContent( [] );
 			this.props.onSplit( [], [], ...blocks );
 		}
 	}
@@ -726,14 +711,15 @@ export class RichText extends Component {
 	}
 
 	updateContent() {
-		const bookmark = this.editor.selection.getBookmark( 2, true );
-		this.savedContent = this.props.value;
-		this.setContent( this.savedContent );
-		this.editor.selection.moveToBookmark( bookmark );
-
-		// Saving the editor on updates avoid unecessary onChanges calls
-		// These calls can make the focus jump
-		this.editor.save();
+		// Do not trigger a change event coming from the TinyMCE undo manager.
+		// Our global state is already up-to-date.
+		this.editor.undoManager.ignore( () => {
+			const bookmark = this.editor.selection.getBookmark( 2, true );
+
+			this.savedContent = this.props.value;
+			this.setContent( this.savedContent );
+			this.editor.selection.moveToBookmark( bookmark );
+		} );
 	}
 
 	setContent( content = '' ) {
@@ -808,8 +794,6 @@ export class RichText extends Component {
 		this.setState( ( state ) => ( {
 			formats: merge( {}, state.formats, formats ),
 		} ) );
-
-		this.editor.setDirty( true );
 	}
 
 	render() {
@@ -827,15 +811,14 @@ export class RichText extends Component {
 			isSelected = false,
 			formatters,
 		} = this.props;
-		const { empty } = this.state;
 
 		const ariaProps = pickAriaProps( this.props );
 
 		// Generating a key that includes `tagName` ensures that if the tag
 		// changes, we unmount and destroy the previous TinyMCE element, then
 		// mount and initialize a new child element in its place.
 		const key = [ 'editor', Tagname ].join();
-		const isPlaceholderVisible = placeholder && ( ! isSelected || keepPlaceholderOnFocus ) && empty;
+		const isPlaceholderVisible = placeholder && ( ! isSelected || keepPlaceholderOnFocus ) && this.isEmpty;
 		const classes = classnames( wrapperClassName, 'blocks-rich-text' );
 
 		const formatToolbar = (
@@ -889,7 +872,9 @@ export class RichText extends Component {
 
 RichText.contextTypes = {
 	onUndo: noop,
+	onRedo: noop,
 	canUserUseUnfilteredHTML: noop,
+	onCreateUndoLevel: noop,
 };
 
 RichText.defaultProps = {

blocks/rich-text/provider.js

@@ -29,6 +29,8 @@ class RichTextProvider extends Component {
 
 RichTextProvider.childContextTypes = {
 	onUndo: noop,
+	onRedo: noop,
+	onCreateUndoLevel: noop,
 };
 
 export default RichTextProvider;

blocks/rich-text/test/index.js

@@ -219,6 +219,7 @@ describe( 'RichText', () => {
 				expect( wrapper.instance().getSettings( settings ) ).toEqual( {
 					setting: 'hi',
 					forced_root_block: false,
+					custom_undo_redo_levels: 1,
 				} );
 			} );
 

editor/components/provider/index.js

@@ -19,7 +19,7 @@ import {
 /**
  * Internal Dependencies
  */
-import { setupEditor, undo, initializeMetaBoxState } from '../../store/actions';
+import { setupEditor, undo, redo, createUndoLevel, initializeMetaBoxState } from '../../store/actions';
 import store from '../../store';
 
 /**
@@ -105,10 +105,14 @@ class EditorProvider extends Component {
 			// RichText provider:
 			//
 			//  - context.onUndo
+			//  - context.onRedo
+			//  - context.onCreateUndoLevel
 			[
 				RichTextProvider,
 				bindActionCreators( {
 					onUndo: undo,
+					onRedo: redo,
+					onCreateUndoLevel: createUndoLevel,
 				}, this.store.dispatch ),
 			],
 

editor/store/actions.js

@@ -302,6 +302,16 @@ export function undo() {
 	return { type: 'UNDO' };
 }
 
+/**
+ * Returns an action object used in signalling that undo history record should
+ * be created.
+ *
+ * @return {Object} Action object.
+ */
+export function createUndoLevel() {
+	return { type: 'CREATE_UNDO_LEVEL' };
+}
+
 /**
  * Returns an action object used in signalling that the blocks
  * corresponding to the specified UID set are to be removed.

editor/store/reducer.js

@@ -15,6 +15,8 @@ import {
 	findIndex,
 	reject,
 	omitBy,
+	keys,
+	isEqual,
 } from 'lodash';
 
 /**
@@ -95,6 +97,31 @@ function getFlattenedBlocks( blocks ) {
 	return flattenedBlocks;
 }
 
+/**
+ * Option for the history reducer. When the block ID and updated attirbute keys
+ * are the same as previously, the history reducer should overwrite its present
+ * state.
+ *
+ * @param {Object} action         The currently dispatched action.
+ * @param {Object} previousAction The previously dispatched action.
+ *
+ * @return {boolean} Whether or not to overwrite present state.
+ */
+function shouldOverwriteState( action, previousAction ) {
+	if (
+		previousAction &&
+		action.type === 'UPDATE_BLOCK_ATTRIBUTES' &&
+		action.type === previousAction.type
+	) {
+		const attributes = keys( action.attributes );
+		const previousAttributes = keys( previousAction.attributes );
+
+		return action.uid === previousAction.uid && isEqual( attributes, previousAttributes );
+	}
+
+	return false;
+}
+
 /**
  * Undoable reducer returning the editor post state, including blocks parsed
  * from current HTML markup.
@@ -115,7 +142,10 @@ export const editor = flow( [
 	combineReducers,
 
 	// Track undo history, starting at editor initialization.
-	partialRight( withHistory, { resetTypes: [ 'SETUP_NEW_POST', 'SETUP_EDITOR' ] } ),
+	partialRight( withHistory, {
+		resetTypes: [ 'SETUP_NEW_POST', 'SETUP_EDITOR' ],
+		shouldOverwriteState,
+	} ),
 
 	// Track whether changes exist, resetting at each post save. Relies on
 	// editor initialization firing post reset as an effect.