From e95e7e540c77d1545771b7cae2612c780b235198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 10 May 2021 11:01:05 +0300 Subject: [PATCH 1/4] Rich text: remove implementation specific delete and enter handling --- .../src/components/rich-text/index.js | 261 ++++++++++-------- packages/rich-text/src/component/index.js | 43 +-- 2 files changed, 143 insertions(+), 161 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index efa42c67204115..1b68c1b2a4cc0b 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -36,10 +36,12 @@ import { __UNSTABLE_LINE_SEPARATOR as LINE_SEPARATOR, toHTMLString, slice, + isCollapsed, } from '@wordpress/rich-text'; import deprecated from '@wordpress/deprecated'; import { isURL } from '@wordpress/url'; import { regexp } from '@wordpress/shortcode'; +import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes'; /** * Internal dependencies @@ -270,23 +272,6 @@ function RichTextWrapper( [ clientId, identifier ] ); - const onDelete = useCallback( - ( { value, isReverse } ) => { - if ( onMerge ) { - onMerge( ! isReverse ); - } - - // Only handle remove on Backspace. This serves dual-purpose of being - // an intentional user interaction distinguishing between Backspace and - // Delete to remove the empty field, but also to avoid merge & remove - // causing destruction of two fields (merge, then removed merged). - if ( onRemove && isEmpty( value ) && isReverse ) { - onRemove( ! isReverse ); - } - }, - [ onMerge, onRemove ] - ); - /** * Signals to the RichText owner that the block can be replaced with two * blocks as a result of splitting the block by pressing enter, or with @@ -368,62 +353,6 @@ function RichTextWrapper( [ onReplace, onSplit, multilineTag, onSplitMiddle ] ); - const onEnter = useCallback( - ( { value, onChange, shiftKey } ) => { - const canSplit = onReplace && onSplit; - - if ( onReplace ) { - const transforms = getBlockTransforms( 'from' ).filter( - ( { type } ) => type === 'enter' - ); - const transformation = findTransform( transforms, ( item ) => { - return item.regExp.test( value.text ); - } ); - - if ( transformation ) { - onReplace( [ - transformation.transform( { content: value.text } ), - ] ); - __unstableMarkAutomaticChange(); - } - } - - if ( multiline ) { - if ( shiftKey ) { - if ( ! disableLineBreaks ) { - onChange( insert( value, '\n' ) ); - } - } else if ( canSplit && isEmptyLine( value ) ) { - splitValue( value ); - } else { - onChange( insertLineSeparator( value ) ); - } - } else { - const { text, start, end } = value; - const canSplitAtEnd = - onSplitAtEnd && start === end && end === text.length; - - if ( shiftKey || ( ! canSplit && ! canSplitAtEnd ) ) { - if ( ! disableLineBreaks ) { - onChange( insert( value, '\n' ) ); - } - } else if ( ! canSplit && canSplitAtEnd ) { - onSplitAtEnd(); - } else if ( canSplit ) { - splitValue( value ); - } - } - }, - [ - onReplace, - onSplit, - __unstableMarkAutomaticChange, - multiline, - splitValue, - onSplitAtEnd, - ] - ); - const onPaste = useCallback( ( { value, @@ -598,8 +527,6 @@ function RichTextWrapper( placeholder={ placeholder } allowedFormats={ adjustedAllowedFormats } withoutInteractiveFormatting={ withoutInteractiveFormatting } - onEnter={ onEnter } - onDelete={ onDelete } onPaste={ onPaste } __unstableIsSelected={ isSelected } __unstableInputRule={ inputRule } @@ -656,55 +583,149 @@ function RichTextWrapper( onFocus, editableProps, editableTagName: TagName, - } ) => ( - <> - { children && children( { value, onChange, onFocus } ) } - { nestedIsSelected && hasFormats && ( - - ) } - { nestedIsSelected && } - - { ( { listBoxId, activeId, onKeyDown } ) => ( - { + function _onKeyDown( event ) { + const { keyCode } = event; + + if ( event.keyCode === ENTER ) { + event.preventDefault(); + + const _value = removeEditorOnlyFormats( value ); + const canSplit = onReplace && onSplit; + + if ( onReplace ) { + const transforms = getBlockTransforms( + 'from' + ).filter( ( { type } ) => type === 'enter' ); + const transformation = findTransform( + transforms, + ( item ) => { + return item.regExp.test( _value.text ); + } + ); + + if ( transformation ) { + onReplace( [ + transformation.transform( { + content: _value.text, + } ), + ] ); + __unstableMarkAutomaticChange(); + } + } + + if ( multiline ) { + if ( event.shiftKey ) { + if ( ! disableLineBreaks ) { + onChange( insert( _value, '\n' ) ); } - className={ classnames( - classes, - props.className, - editableProps.className - ) } - aria-autocomplete={ - listBoxId ? 'list' : undefined + } else if ( canSplit && isEmptyLine( _value ) ) { + splitValue( _value ); + } else { + onChange( insertLineSeparator( _value ) ); + } + } else { + const { text, start, end } = _value; + const canSplitAtEnd = + onSplitAtEnd && + start === end && + end === text.length; + + if ( + event.shiftKey || + ( ! canSplit && ! canSplitAtEnd ) + ) { + if ( ! disableLineBreaks ) { + onChange( insert( _value, '\n' ) ); } - aria-owns={ listBoxId } - aria-activedescendant={ activeId } - onKeyDown={ ( event ) => { - onKeyDown( event ); - editableProps.onKeyDown( event ); - } } + } else if ( ! canSplit && canSplitAtEnd ) { + onSplitAtEnd(); + } else if ( canSplit ) { + splitValue( _value ); + } + } + } else if ( keyCode === DELETE || keyCode === BACKSPACE ) { + const { start, end, text } = value; + const isReverse = keyCode === BACKSPACE; + + // Only process delete if the key press occurs at an uncollapsed edge. + if ( + ! isCollapsed( value ) || + activeFormats.length || + ( isReverse && start !== 0 ) || + ( ! isReverse && end !== text.length ) + ) { + return; + } + + if ( onMerge ) { + onMerge( ! isReverse ); + } + + // Only handle remove on Backspace. This serves dual-purpose of being + // an intentional user interaction distinguishing between Backspace and + // Delete to remove the empty field, but also to avoid merge & remove + // causing destruction of two fields (merge, then removed merged). + if ( onRemove && isEmpty( value ) && isReverse ) { + onRemove( ! isReverse ); + } + + event.preventDefault(); + } + } + + return ( + <> + { children && children( { value, onChange, onFocus } ) } + { nestedIsSelected && hasFormats && ( + ) } - - - ) } + { nestedIsSelected && } + + { ( { listBoxId, activeId, onKeyDown } ) => ( + { + onKeyDown( event ); + _onKeyDown( event ); + } } + /> + ) } + + + ); + } } ); diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 2b99a4020c9018..a01c777c5dce1b 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -8,7 +8,6 @@ import { useState, useLayoutEffect, } from '@wordpress/element'; -import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes'; import { useMergeRefs } from '@wordpress/compose'; /** @@ -19,7 +18,6 @@ import { create } from '../create'; import { apply } from '../to-dom'; import { toHTMLString } from '../to-html-string'; import { removeFormat } from '../remove-format'; -import { isCollapsed } from '../is-collapsed'; import { useFormatTypes } from './use-format-types'; import { useDefaultStyle } from './use-default-style'; import { useBoundaryStyle } from './use-boundary-style'; @@ -57,8 +55,6 @@ function RichText( preserveWhiteSpace, onPaste, format = 'string', - onDelete, - onEnter, onSelectionChange, onChange, unstableOnFocus: onFocus, @@ -210,42 +206,6 @@ function RichText( } ); } - function handleKeyDown( event ) { - if ( event.defaultPrevented ) { - return; - } - - const { keyCode } = event; - - if ( event.keyCode === ENTER ) { - event.preventDefault(); - if ( onEnter ) { - onEnter( { - value: removeEditorOnlyFormats( record.current ), - onChange: handleChange, - shiftKey: event.shiftKey, - } ); - } - } else if ( keyCode === DELETE || keyCode === BACKSPACE ) { - const { start, end, text } = record.current; - const isReverse = keyCode === BACKSPACE; - - // Only process delete if the key press occurs at an uncollapsed edge. - if ( - ! onDelete || - ! isCollapsed( record.current ) || - activeFormats.length || - ( isReverse && start !== 0 ) || - ( ! isReverse && end !== text.length ) - ) { - return; - } - - onDelete( { isReverse, value: record.current } ); - event.preventDefault(); - } - } - // Internal values are updated synchronously, unlike props and state. const _value = useRef( value ); const record = useRef(); @@ -414,7 +374,6 @@ function RichText( } ), ] ), className: 'rich-text', - onKeyDown: handleKeyDown, onFocus, // Do not set the attribute if disabled. contentEditable: disabled ? undefined : true, @@ -440,6 +399,8 @@ function RichText( onFocus: focus, editableProps, editableTagName: TagName, + activeFormats, + removeEditorOnlyFormats, } ) } { ! children && } From 1934cc93110b10257a344ececf3c6f9c4d4e757e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 10 May 2021 11:24:24 +0300 Subject: [PATCH 2/4] Restore defaultPrevented check --- packages/block-editor/src/components/rich-text/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 1b68c1b2a4cc0b..4c69023cb70e78 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -589,6 +589,10 @@ function RichTextWrapper( function _onKeyDown( event ) { const { keyCode } = event; + if ( event.defaultPrevented ) { + return; + } + if ( event.keyCode === ENTER ) { event.preventDefault(); From c879499778688ab35331d744d1376604913d930c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 10 May 2021 13:25:44 +0300 Subject: [PATCH 3/4] useLayoutEffect --- packages/rich-text/src/component/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index a01c777c5dce1b..1a09e420cd6823 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -292,7 +292,7 @@ function RichText( } }, [ value ] ); - useEffect( () => { + useLayoutEffect( () => { if ( ! didMount.current ) { return; } From 4a675bc079d15ed7f2ad6a1e02a605188efdd8dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 10 May 2021 17:26:12 +0300 Subject: [PATCH 4/4] use synchronous effect to avoid overwriting --- packages/rich-text/src/component/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 1a09e420cd6823..80df45f96de54c 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -3,7 +3,6 @@ */ import { forwardRef, - useEffect, useRef, useState, useLayoutEffect, @@ -276,6 +275,7 @@ function RichText( const didMount = useRef( false ); + // Value updates must happen synchonously to avoid overwriting newer values. useLayoutEffect( () => { if ( didMount.current ) { applyFromProps(); @@ -286,12 +286,14 @@ function RichText( didMount.current = true; }, [ TagName, placeholder, ...dependencies ] ); - useEffect( () => { + // Value updates must happen synchonously to avoid overwriting newer values. + useLayoutEffect( () => { if ( didMount.current && value !== _value.current ) { applyFromProps(); } }, [ value ] ); + // Value updates must happen synchonously to avoid overwriting newer values. useLayoutEffect( () => { if ( ! didMount.current ) { return;