RichText: Memoize createRecord

[ellatrix]

Description

It very often occurs that on selection change, the range stays the same and no selection calculation needs to be made. E.g. on entering a character onSelectionChange will be called twice, while the selection actually stays the same. We can memoize createRecord, as range object will be strict equal through getSelection().getRangeAt( 0 ) calls.

How has this been tested?

Screenshots

Types of changes

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[youknowriad]

I'm moving this out of 4.3, we still need to check whether it creates bugs or not.

[ellatrix]

Sounds good.

…

On Fri, 9 Nov 2018 at 10:02, Riad Benguella ***@***.***> wrote: I'm moving this out of 4.3, we still need to check whether it creates bugs or not. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11602 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEfg6w0119bqnzrd6pRqTpUYCPdFydn4ks5utTaIgaJpZM4YTL_m> .

[ellatrix]

@youknowriad Do you mean something like this? With this button, the range still stays the same.

wp.richText.registerFormatType( 'plugin/test', {
	title: 'test',
	tagName: 'test',
	className: 'test',
	edit: function( props ) {
		return wp.element.createElement( wp.element.Fragment, null,
			wp.element.createElement( wp.editor.RichTextToolbarButton, {
				icon: 'editor-textcolor',
				title: 'test',
				onClick: function() {
					props.onChange( {
						...props.value,
						text: props.value.text.toUpperCase(),
					} );
				}
			} )
		);
	},
} );

[youknowriad]

yes

[ellatrix]

Cool. So all seems fine to me then.

[mtias]

Should we add a brief comment about the intention of this for posterity?

[mtias]

Left a comment about adding a comment.

[youknowriad]

The e2e test failures seems legit.

[ellatrix]

@youknowriad Previous ones were passing, and the last commit is just a comment. May only need a rebase.

[ellatrix]

No idea why e2e are failing suddenly. Needs investigation.

[ellatrix]

I'm going to close this for now as #12547 addresses the source of the issue: the selection change event emitting too many times with the same selection. The browser might still emit the event in other cases with the same selection, so maybe this fix should still be included, but it would be good to see where it could happen.