From 84132f8e51fda98877e919b5ef04a1781e65dc2c Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 8 Feb 2018 15:00:25 +0100 Subject: [PATCH 1/2] WIP: try editable onChange on input --- blocks/rich-text/index.js | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index 8efe4996dd490..a8234fb4c82ea 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -149,6 +149,7 @@ export default class RichText extends Component { editor.on( 'BeforeExecCommand', this.maybePropagateUndo ); editor.on( 'PastePreProcess', this.onPastePreProcess, true /* Add before core handlers */ ); editor.on( 'paste', this.onPaste, true /* Add before core handlers */ ); + editor.on( 'input', this.onChange ); patterns.apply( this, [ editor ] ); @@ -358,21 +359,12 @@ export default class RichText extends Component { } } - fireChange() { - this.savedContent = this.getContent(); - this.editor.save(); - this.props.onChange( this.state.empty ? [] : this.savedContent ); - } - /** * Handles any case where the content of the tinyMCE instance has changed. */ onChange() { - // Note that due to efficiency, speed and low cost requirements isDirty may - // not reflect reality for a brief period immediately after a change. - if ( this.editor.isDirty() ) { - this.fireChange(); - } + this.savedContent = this.state.empty ? [] : this.getContent(); + this.props.onChange( this.savedContent ); } /** @@ -500,8 +492,6 @@ export default class RichText extends Component { return; } - this.fireChange(); - const forward = event.keyCode === DELETE; if ( this.props.onMerge ) { @@ -680,19 +670,10 @@ export default class RichText extends Component { 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 ) { - if ( ! content ) { - content = ''; - } - - content = renderToString( content ); - this.editor.setContent( content ); + setContent( content = '' ) { + this.editor.setContent( renderToString( content ) ); } getContent() { From b5764cd0a33cb041ae58edad28bb5a4114817bfb Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 8 Feb 2018 19:01:51 +0100 Subject: [PATCH 2/2] Try a generic undo handler --- editor/store/reducer.js | 76 +++++++++++++++++++++++++++++- editor/utils/with-history/index.js | 26 ++++++++-- package-lock.json | 5 ++ package.json | 1 + 4 files changed, 102 insertions(+), 6 deletions(-) diff --git a/editor/store/reducer.js b/editor/store/reducer.js index e0dd31667c274..3e34293f4c2f2 100644 --- a/editor/store/reducer.js +++ b/editor/store/reducer.js @@ -14,7 +14,13 @@ import { mapValues, findIndex, reject, + keys, + isString, + isArray, + isPlainObject, + every, } from 'lodash'; +import diff from 'fast-diff'; /** * WordPress dependencies @@ -94,6 +100,55 @@ function getFlattenedBlocks( blocks ) { return flattenedBlocks; } +/** + * Check whether two values are considered equivalent + * Two values are considered equivalent if the only + * possible difference between these two values is a character in a string. + * + * @param {*} value1 + * @param {*} value2 + * + * @return {boolean} Whether the values are equivalent. + */ +function areValuesEquivalent( value1, value2 ) { + if ( isString( value1 ) && isString( value2 ) ) { + const diffResult = diff( value1, value2 ).filter( ( [ type ] ) => type !== 0 ); + if ( + diffResult.length === 0 || + ( diffResult.length === 1 && diffResult[ 0 ][ 1 ].match( /\S/ ) !== null ) + ) { + return true; + } + + // For some reason comparing text adding a char after a space is creating two diffs, + // let's run the diff again for more accurate results + if ( diffResult.length === 2 ) { + const diffResult2 = diff( diffResult[ 0 ][ 1 ], diffResult[ 1 ][ 1 ] ).filter( ( [ type ] ) => type !== 0 ); + return diffResult2.length === 0 || + ( diffResult2.length === 1 && diffResult2[ 0 ][ 1 ].match( /\S/ ) !== null ); + } + + return false; + } + + if ( isArray( value1 ) && isArray( value2 ) && value1.length === value2.length ) { + return every( value1, ( subvalue, key ) => areValuesEquivalent( subvalue, value2[ key ] ) ); + } + + if ( isPlainObject( value1 ) && isPlainObject( value2 ) ) { + const keys1 = keys( value1 ); + const keys2 = keys( value2 ); + return ( + keys1.length === keys2.length && + every( keys1, ( key, index ) => + keys2[ index ] === key && areValuesEquivalent( value1[ key ], value2[ key ] ) + ) + ); + } + + return value1 === value2; +} + /** * Undoable reducer returning the editor post state, including blocks parsed * from current HTML markup. @@ -114,7 +169,26 @@ 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: ( previous, next ) => { + if ( previous.type !== next.type || previous.type !== 'UPDATE_BLOCK_ATTRIBUTES' ) { + return false; + } + + const previousAttributes = keys( previous.attributes ); + const nextAttributes = keys( next.attributes ); + if ( + previousAttributes.length !== 1 || + nextAttributes.length !== 1 || + previousAttributes[ 0 ] !== nextAttributes[ 0 ] + ) { + return false; + } + + return areValuesEquivalent( previous.attributes[ previousAttributes[ 0 ] ], next.attributes[ nextAttributes[ 0 ] ] ); + }, + } ), // Track whether changes exist, resetting at each post save. Relies on // editor initialization firing post reset as an effect. diff --git a/editor/utils/with-history/index.js b/editor/utils/with-history/index.js index f1af9e1f20573..8b1e5bc0ba77f 100644 --- a/editor/utils/with-history/index.js +++ b/editor/utils/with-history/index.js @@ -7,9 +7,12 @@ import { includes } from 'lodash'; * Reducer enhancer which transforms the result of the original reducer into an * object tracking its own history (past, present, future). * - * @param {Function} reducer Original reducer. - * @param {?Object} options Optional options. - * @param {?Array} options.resetTypes Action types upon which to clear past. + * @param {Function} reducer Original reducer. + * @param {?Object} options Optional options. + * @param {?Array} options.resetTypes Action types upon which to clear past. + * @param {Function} options.shouldOverwriteState A function passed the previous and current action. + * Returning true means we don't create a new undo level + * and replace the previous last item. * * @return {Function} Enhanced reducer. */ @@ -18,10 +21,11 @@ export default function withHistory( reducer, options = {} ) { past: [], present: reducer( undefined, {} ), future: [], + previousAction: null, }; return ( state = initialState, action ) => { - const { past, present, future } = state; + const { past, present, future, previousAction } = state; switch ( action.type ) { case 'UNDO': @@ -34,6 +38,7 @@ export default function withHistory( reducer, options = {} ) { past: past.slice( 0, past.length - 1 ), present: past[ past.length - 1 ], future: [ present, ...future ], + previousAction: null, }; case 'REDO': @@ -46,6 +51,7 @@ export default function withHistory( reducer, options = {} ) { past: [ ...past, present ], present: future[ 0 ], future: future.slice( 1 ), + previousAction: null, }; } @@ -56,6 +62,7 @@ export default function withHistory( reducer, options = {} ) { past: [], present: nextPresent, future: [], + previousAction: null, }; } @@ -63,10 +70,19 @@ export default function withHistory( reducer, options = {} ) { return state; } + let newPast; + const { shouldOverwriteState = () => false } = options; + if ( past.length && previousAction && shouldOverwriteState( previousAction, action ) ) { + newPast = [ ...past.slice( 0, -1 ), present ]; + } else { + newPast = [ ...past, present ]; + } + return { - past: [ ...past, present ], + past: newPast, present: nextPresent, future: [], + previousAction: action, }; }; } diff --git a/package-lock.json b/package-lock.json index 3af6d0a9c6834..12ff1f0051afa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3848,6 +3848,11 @@ "integrity": "sha1-liVqO8l1WV6zbYLpkp0GDYk0Of8=", "dev": true }, + "fast-diff": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/fast-diff/-/fast-diff-1.1.2.tgz", + "integrity": "sha512-KaJUt+M9t1qaIteSvjc6P3RbMdXsNhK61GRftR6SNxqmhthcd9MGIi4T+o0jD8LUSpSnSKXE20nLtJ3fOHxQig==" + }, "fast-json-stable-stringify": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.0.0.tgz", diff --git a/package.json b/package.json index 97a45a36710d1..e0d1d34c9a962 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "element-closest": "2.0.2", "escape-string-regexp": "1.0.5", "eslint-plugin-wordpress": "git://github.com/WordPress-Coding-Standards/eslint-plugin-wordpress.git#327b6bdec434177a6e841bd3210e87627ccfcecb", + "fast-diff": "1.1.2", "hpq": "1.2.0", "is-equal-shallow": "0.1.3", "jed": "1.1.1",