From cb8488099e5a196fbe79fda2c49dd7f5dae7216a Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 7 Nov 2018 21:30:57 +0100 Subject: [PATCH 1/6] memoize createRecord --- packages/editor/src/components/rich-text/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 688a5287052b6..c52d9a3943506 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -111,6 +111,7 @@ export class RichText extends Component { this.isActive = this.isActive.bind( this ); this.formatToValue = memize( this.formatToValue.bind( this ), { size: 1 } ); + this.createRecordFromRange = memize( this.createRecordFromRange.bind( this ), { size: 1 } ); this.savedContent = value; this.patterns = getPatterns( { @@ -175,8 +176,10 @@ export class RichText extends Component { } createRecord() { - const range = getSelection().getRangeAt( 0 ); + return this.createRecordFromRange( getSelection().getRangeAt( 0 ) ); + } + createRecordFromRange( range ) { return create( { element: this.editableRef, range, From 1056c3ae6b05694d5e5cd8c9d1307935677c4418 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 7 Nov 2018 22:31:32 +0100 Subject: [PATCH 2/6] Do own check --- .../editor/src/components/rich-text/index.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index c52d9a3943506..d4784484ec26e 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -111,7 +111,6 @@ export class RichText extends Component { this.isActive = this.isActive.bind( this ); this.formatToValue = memize( this.formatToValue.bind( this ), { size: 1 } ); - this.createRecordFromRange = memize( this.createRecordFromRange.bind( this ), { size: 1 } ); this.savedContent = value; this.patterns = getPatterns( { @@ -176,11 +175,13 @@ export class RichText extends Component { } createRecord() { - return this.createRecordFromRange( getSelection().getRangeAt( 0 ) ); - } + const range = getSelection().getRangeAt( 0 ); + + if ( range === this.createRecord.lastRange ) { + return this.createRecord.lastRecord; + } - createRecordFromRange( range ) { - return create( { + const record = create( { element: this.editableRef, range, multilineTag: this.multilineTag, @@ -191,6 +192,11 @@ export class RichText extends Component { filterString: ( string ) => string.replace( TINYMCE_ZWSP, '' ), prepareEditableTree: this.props.prepareEditableTree, } ); + + this.createRecord.lastRecord = record; + this.createRecord.lastRange = range; + + return record; } applyRecord( record ) { From 01f4c8da66a5792168694d1f4fa614d2f6193bfe Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 20 Nov 2018 00:29:54 +0100 Subject: [PATCH 3/6] Add comment --- packages/editor/src/components/rich-text/index.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index d4784484ec26e..b33182c02d080 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -177,6 +177,20 @@ export class RichText extends Component { createRecord() { const range = getSelection().getRangeAt( 0 ); + // If the range is shallowly equal to the last, return the last + // calculated value. This is useful when creating values on the + // `selectionchange` event, which fires more often than it should + // without a change in selection. + // + // Shallowly checking equality for ranges is fine: + // + // > Thus subsequent calls of this method returns the same range object + // > if nothing has removed the context object's range in the meantime. + // > In particular, getSelection().getRangeAt( 0 ) === + // > getSelection().getRangeAt( 0 ) evaluates to true if the selection + // > is not empty. + // > + // > https://w3c.github.io/selection-api/#dom-selection-getrangeat if ( range === this.createRecord.lastRange ) { return this.createRecord.lastRecord; } From 2f38940bd7c35abff1ac22f1ed04860e8b9319e8 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 30 Nov 2018 15:18:39 +0100 Subject: [PATCH 4/6] Don't cache on input --- packages/editor/src/components/rich-text/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index b33182c02d080..fac6b7efb039a 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -174,7 +174,7 @@ export class RichText extends Component { return { formats, text, start, end }; } - createRecord() { + createRecord( { cache } = {} ) { const range = getSelection().getRangeAt( 0 ); // If the range is shallowly equal to the last, return the last @@ -191,7 +191,7 @@ export class RichText extends Component { // > is not empty. // > // > https://w3c.github.io/selection-api/#dom-selection-getrangeat - if ( range === this.createRecord.lastRange ) { + if ( cache && range === this.createRecord.lastRange ) { return this.createRecord.lastRecord; } @@ -419,7 +419,7 @@ export class RichText extends Component { return; } - const { start, end, formats } = this.createRecord(); + const { start, end, formats } = this.createRecord( { cache: true } ); if ( start !== this.state.start || end !== this.state.end ) { const isCaretWithinFormattedText = this.props.isCaretWithinFormattedText; From d0899c5170697ab507d7721fc3d1d816bb2425d2 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 30 Nov 2018 15:38:28 +0100 Subject: [PATCH 5/6] Add doc --- packages/editor/src/components/rich-text/index.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index fac6b7efb039a..610db4ff48e7f 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -174,6 +174,15 @@ export class RichText extends Component { return { formats, text, start, end }; } + /** + * Creates a new record from the current editable DOM. + * + * @param {Object} $1 Named arguments. + * @param {Object} $1.cache Wether or not to cache the record based on the + * selection. + * + * @return {Object} A new record. + */ createRecord( { cache } = {} ) { const range = getSelection().getRangeAt( 0 ); From c8401d8c44306b73c0f3a553192f01f4f7c53a7c Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 30 Nov 2018 15:44:50 +0100 Subject: [PATCH 6/6] cache => useCache --- packages/editor/src/components/rich-text/index.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 610db4ff48e7f..b246f4e596180 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -177,13 +177,16 @@ export class RichText extends Component { /** * Creates a new record from the current editable DOM. * - * @param {Object} $1 Named arguments. - * @param {Object} $1.cache Wether or not to cache the record based on the - * selection. + * @param {Object} $1 Named arguments. + * @param {Object} $1.useCache Wether or not to use a cached record when the + * selection did not change. Recommended to use + * e.g. in the `selectionchange` event handler + * when nothing other than the selection could + * have changed. * * @return {Object} A new record. */ - createRecord( { cache } = {} ) { + createRecord( { useCache } = {} ) { const range = getSelection().getRangeAt( 0 ); // If the range is shallowly equal to the last, return the last @@ -200,7 +203,7 @@ export class RichText extends Component { // > is not empty. // > // > https://w3c.github.io/selection-api/#dom-selection-getrangeat - if ( cache && range === this.createRecord.lastRange ) { + if ( useCache && range === this.createRecord.lastRange ) { return this.createRecord.lastRecord; } @@ -428,7 +431,7 @@ export class RichText extends Component { return; } - const { start, end, formats } = this.createRecord( { cache: true } ); + const { start, end, formats } = this.createRecord( { useCache: true } ); if ( start !== this.state.start || end !== this.state.end ) { const isCaretWithinFormattedText = this.props.isCaretWithinFormattedText;