Overwrite undo + editable signal

blocks/rich-text/index.js

@@ -13,7 +13,6 @@ import {
 	find,
 	defer,
 	noop,
-	throttle,
 } from 'lodash';
 import { nodeListToReact } from 'dom-react';
 import 'element-closest';
@@ -93,16 +92,17 @@ export class RichText extends Component {
 		this.getSettings = this.getSettings.bind( this );
 		this.onSetup = this.onSetup.bind( this );
 		this.onChange = this.onChange.bind( this );
-		this.throttledOnChange = throttle( this.onChange.bind( this ), 500 );
 		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.onAddUndo = this.onAddUndo.bind( this );
+		this.onCreateUndoLevel = this.onCreateUndoLevel.bind( this );
 
 		this.state = {
 			formats: {},
@@ -142,16 +142,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.throttledOnChange );
+		editor.on( 'input', this.onChange );
+		editor.on( 'addundo', this.onAddUndo );
 
 		patterns.apply( this, [ editor ] );
 
@@ -223,23 +223,15 @@ export class RichText extends Component {
 	/**
 	 * 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 ) {
+	onPropagateUndo( event ) {
 		const { onUndo } = this.context;
-		if ( onUndo && event.command === 'Undo' && ! this.editor.undoManager.hasUndo() ) {
-			defer( onUndo );
+		const { command } = event;
 
-			// 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 ( onUndo && ( command === 'Undo' || command === 'Redo' ) ) {
+			defer( onUndo );
+			event.preventDefault();
 		}
 	}
 
@@ -373,12 +365,22 @@ export class RichText extends Component {
 	 * Handles any case where the content of the tinyMCE instance has changed.
 	 */
 	onChange() {
-		if ( ! this.editor.isDirty() ) {
+		this.savedContent = this.getContent();
+		this.props.onChange( this.savedContent );
+	}
+
+	onAddUndo( { lastLevel } ) {
+		if ( ! lastLevel ) {
 			return;
 		}
-		this.savedContent = this.state.empty ? [] : this.getContent();
-		this.props.onChange( this.savedContent );
-		this.editor.save();
+
+		this.onCreateUndoLevel();
+	}
+
+	onCreateUndoLevel() {
+		// Always ensure the content is up-to-date.
+		this.onChange();
+		this.context.onCreateUndoLevel();
 	}
 
 	/**
@@ -506,6 +508,8 @@ export class RichText extends Component {
 				return;
 			}
 
+			this.onCreateUndoLevel();
+
 			const forward = event.keyCode === DELETE;
 
 			if ( this.props.onMerge ) {
@@ -543,6 +547,7 @@ export class RichText extends Component {
 				}
 
 				event.preventDefault();
+				this.onCreateUndoLevel();
 
 				const childNodes = Array.from( rootNode.childNodes );
 				const index = dom.nodeIndex( selectedNode );
@@ -555,6 +560,7 @@ export class RichText extends Component {
 				this.props.onSplit( beforeElement, afterElement );
 			} else {
 				event.preventDefault();
+				this.onCreateUndoLevel();
 
 				if ( event.shiftKey || ! this.props.onSplit ) {
 					this.editor.execCommand( 'InsertLineBreak', false, event );
@@ -683,28 +689,20 @@ export class RichText extends Component {
 		this.setState( { formats, focusPosition, selectedNodeId: this.state.selectedNodeId + 1 } );
 	}
 
-	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();
-	}
-
 	setContent( content = '' ) {
 		this.editor.setContent( renderToString( content ) );
 	}
 
 	getContent() {
+		if ( this.state.empty ) {
+			return [];
+		}
+
 		return nodeListToReact( this.editor.getBody().childNodes || [], createTinyMCEElement );
 	}
 
 	componentWillUnmount() {
 		this.onChange();
-		this.throttledOnChange.cancel();
 	}
 
 	componentDidUpdate( prevProps ) {
@@ -715,7 +713,11 @@ export class RichText extends Component {
 			this.props.value !== prevProps.value &&
 			this.props.value !== this.savedContent
 		) {
-			this.updateContent();
+			const bookmark = this.editor.selection.getBookmark( 2, true );
+
+			this.savedContent = this.props.value;
+			this.setContent( this.savedContent );
+			this.editor.selection.moveToBookmark( bookmark );
 		}
 	}
 	componentWillReceiveProps( nextProps ) {
@@ -848,6 +850,7 @@ export class RichText extends Component {
 RichText.contextTypes = {
 	onUndo: noop,
 	canUserUseUnfilteredHTML: noop,
+	onCreateUndoLevel: noop,
 };
 
 RichText.defaultProps = {

blocks/rich-text/provider.js

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

editor/components/provider/index.js

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

editor/store/actions.js

@@ -294,6 +294,10 @@ export function undo() {
 	return { type: 'UNDO' };
 }
 
+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

@@ -14,6 +14,9 @@ import {
 	mapValues,
 	findIndex,
 	reject,
+	includes,
+	keys,
+	isEqual,
 } from 'lodash';
 
 /**
@@ -114,7 +117,27 @@ 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( action, previousAction ) {
+			if ( ! includes( [ 'UPDATE_BLOCK_ATTRIBUTES', 'EDIT_POST', 'RESET_POST' ], action.type ) ) {
+				return false;
+			}
+
+			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 true;
+		},
+	} ),
 
 	// Track whether changes exist, resetting at each post save. Relies on
 	// editor initialization firing post reset as an effect.

editor/store/selectors.js

@@ -94,7 +94,8 @@ export function isSavingMetaBoxes( state ) {
  * @return {boolean} Whether undo history exists.
  */
 export function hasEditorUndo( state ) {
-	return state.editor.past.length > 0;
+	const { past, present } = state.editor;
+	return past.length > 1 || last( past ) !== present;
 }
 
 /**

editor/store/test/reducer.js

@@ -72,7 +72,7 @@ describe( 'state', () => {
 		it( 'should return history (empty edits, blocksByUid, blockOrder), dirty flag by default', () => {
 			const state = editor( undefined, {} );
 
-			expect( state.past ).toEqual( [] );
+			expect( state.past ).toEqual( [ state.present ] );
 			expect( state.future ).toEqual( [] );
 			expect( state.present.edits ).toEqual( {} );
 			expect( state.present.blocksByUid ).toEqual( {} );
@@ -819,6 +819,70 @@ describe( 'state', () => {
 				expect( state.present.blocksByUid ).toBe( state.present.blocksByUid );
 			} );
 		} );
+
+		describe( 'withHistory', () => {
+			it( 'should overwrite present history if updating same attributes', () => {
+				let state;
+
+				state = editor( state, {
+					type: 'RESET_BLOCKS',
+					blocks: [ {
+						uid: 'kumquat',
+						attributes: {},
+						innerBlocks: [],
+					} ],
+				} );
+
+				state = editor( state, {
+					type: 'UPDATE_BLOCK_ATTRIBUTES',
+					uid: 'kumquat',
+					attributes: {
+						test: 1,
+					},
+				} );
+
+				state = editor( state, {
+					type: 'UPDATE_BLOCK_ATTRIBUTES',
+					uid: 'kumquat',
+					attributes: {
+						test: 2,
+					},
+				} );
+
+				expect( state.past ).toHaveLength( 2 );
+			} );
+
+			it( 'should not overwrite present history if updating same attributes', () => {
+				let state;
+
+				state = editor( state, {
+					type: 'RESET_BLOCKS',
+					blocks: [ {
+						uid: 'kumquat',
+						attributes: {},
+						innerBlocks: [],
+					} ],
+				} );
+
+				state = editor( state, {
+					type: 'UPDATE_BLOCK_ATTRIBUTES',
+					uid: 'kumquat',
+					attributes: {
+						test: 1,
+					},
+				} );
+
+				state = editor( state, {
+					type: 'UPDATE_BLOCK_ATTRIBUTES',
+					uid: 'kumquat',
+					attributes: {
+						other: 1,
+					},
+				} );
+
+				expect( state.past ).toHaveLength( 3 );
+			} );
+		} );
 	} );
 
 	describe( 'currentPost()', () => {