From f66b4aa8b093a296544b26baee29b8ec4953afc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 11 May 2021 23:15:12 +0300 Subject: [PATCH 1/5] Rich text: move key handlers to ref callbacks --- .../src/components/rich-text/index.js | 118 +++------------- .../rich-text/use-enter-delete-handler.js | 131 ++++++++++++++++++ packages/components/src/autocomplete/index.js | 24 +++- 3 files changed, 168 insertions(+), 105 deletions(-) create mode 100644 packages/block-editor/src/components/rich-text/use-enter-delete-handler.js diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 042082fb19fbe..e745c322d1a68 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -9,26 +9,17 @@ import { omit } from 'lodash'; */ import { RawHTML, useRef, useCallback, forwardRef } from '@wordpress/element'; import { useDispatch, useSelect } from '@wordpress/data'; -import { - children as childrenSource, - getBlockTransforms, - findTransform, -} from '@wordpress/blocks'; +import { children as childrenSource } from '@wordpress/blocks'; import { useInstanceId, useMergeRefs } from '@wordpress/compose'; import { __unstableUseRichText as useRichText, __unstableCreateElement, isEmpty, - __unstableIsEmptyLine as isEmptyLine, - insert, - __unstableInsertLineSeparator as insertLineSeparator, split, toHTMLString, - isCollapsed, removeFormat, } from '@wordpress/rich-text'; import deprecated from '@wordpress/deprecated'; -import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes'; /** * Internal dependencies @@ -44,6 +35,7 @@ import { useMarkPersistent } from './use-mark-persistent'; import { usePasteHandler } from './use-paste-handler'; import { useInputRules } from './use-input-rules'; import { useFormatTypes } from './use-format-types'; +import { useEnterDeleteHandler } from './use-enter-delete-handler'; import FormatEdit from './format-edit'; import { getMultilineTag, getAllowedFormats } from './utils'; @@ -312,93 +304,6 @@ function RichTextWrapper( useCaretInFormat( hasActiveFormats ); useMarkPersistent( { hasActiveFormats, html: adjustedValue, value } ); - function onKeyDown( event ) { - const { keyCode } = event; - - if ( event.defaultPrevented ) { - return; - } - - if ( event.keyCode === ENTER ) { - event.preventDefault(); - - const _value = { ...value }; - _value.formats = 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' ) ); - } - } 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' ) ); - } - } 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 ) || - hasActiveFormats || - ( 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(); - } - } - const TagName = tagName; const content = ( <> @@ -439,6 +344,21 @@ function RichTextWrapper( onReplace, } ), useUndoAutomaticChange(), + useEnterDeleteHandler( { + removeEditorOnlyFormats, + value, + onReplace, + onSplit, + __unstableMarkAutomaticChange, + multiline, + onChange, + disableLineBreaks, + splitValue, + onSplitAtEnd, + hasActiveFormats, + onMerge, + onRemove, + } ), usePasteHandler( { isSelected, disableFormats, @@ -466,10 +386,6 @@ function RichTextWrapper( 'rich-text' ) } onFocus={ unstableOnFocus } - onKeyDown={ ( event ) => { - autocompleteProps.onKeyDown( event ); - onKeyDown( event ); - } } /> ); diff --git a/packages/block-editor/src/components/rich-text/use-enter-delete-handler.js b/packages/block-editor/src/components/rich-text/use-enter-delete-handler.js new file mode 100644 index 0000000000000..fe11636afd2d2 --- /dev/null +++ b/packages/block-editor/src/components/rich-text/use-enter-delete-handler.js @@ -0,0 +1,131 @@ +/** + * WordPress dependencies + */ +import { useRef } from '@wordpress/element'; +import { useRefEffect } from '@wordpress/compose'; +import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes'; +import { + isCollapsed, + isEmpty, + insert, + __unstableIsEmptyLine as isEmptyLine, + __unstableInsertLineSeparator as insertLineSeparator, +} from '@wordpress/rich-text'; +import { getBlockTransforms, findTransform } from '@wordpress/blocks'; + +export function useEnterDeleteHandler( props ) { + const propsRef = useRef( props ); + propsRef.current = props; + return useRefEffect( ( element ) => { + function onKeyDown( event ) { + const { keyCode } = event; + + if ( event.defaultPrevented ) { + return; + } + + const { + removeEditorOnlyFormats, + value, + onReplace, + onSplit, + __unstableMarkAutomaticChange, + multiline, + onChange, + disableLineBreaks, + splitValue, + onSplitAtEnd, + hasActiveFormats, + onMerge, + onRemove, + } = propsRef.current; + + if ( event.keyCode === ENTER ) { + event.preventDefault(); + + const _value = { ...value }; + _value.formats = 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' ) ); + } + } 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' ) ); + } + } 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 ) || + hasActiveFormats || + ( 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(); + } + } + + element.addEventListener( 'keydown', onKeyDown ); + return () => { + element.removeEventListener( 'keydown', onKeyDown ); + }; + }, [] ); +} diff --git a/packages/components/src/autocomplete/index.js b/packages/components/src/autocomplete/index.js index 100672f811d44..87dd92eac2633 100644 --- a/packages/components/src/autocomplete/index.js +++ b/packages/components/src/autocomplete/index.js @@ -24,7 +24,12 @@ import { BACKSPACE, } from '@wordpress/keycodes'; import { __, _n, sprintf } from '@wordpress/i18n'; -import { useInstanceId, useDebounce } from '@wordpress/compose'; +import { + useInstanceId, + useDebounce, + useMergeRefs, + useRefEffect, +} from '@wordpress/compose'; import { create, slice, @@ -565,15 +570,26 @@ function useAutocomplete( { export function useAutocompleteProps( options ) { const ref = useRef(); + const onKeyDownRef = useRef(); const { popover, listBoxId, activeId, onKeyDown } = useAutocomplete( { ...options, contentRef: ref, } ); - + onKeyDownRef.current = onKeyDown; return { - ref, + ref: useMergeRefs( [ + ref, + useRefEffect( ( element ) => { + function _onKeyDown( event ) { + onKeyDownRef.current( event ); + } + element.addEventListener( 'keydown', _onKeyDown ); + return () => { + element.removeEventListener( 'keydown', _onKeyDown ); + }; + }, [] ), + ] ), children: popover, - onKeyDown, 'aria-autocomplete': listBoxId ? 'list' : undefined, 'aria-owns': listBoxId, 'aria-activedescendant': activeId, From 773895682d8866c26a7b6629d5ee14921c47732a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 12 May 2021 00:29:37 +0300 Subject: [PATCH 2/5] Fix selection after merge --- packages/block-editor/src/store/actions.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 69da6c75860ed..9c8767f4c5c0e 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -802,8 +802,10 @@ export function* mergeBlocks( firstBlockClientId, secondBlockClientId ) { blocksWithTheSameType[ 0 ].attributes ); + let newAttributeKey, newOffset; + if ( canRestoreTextSelection ) { - const newAttributeKey = findKey( + newAttributeKey = findKey( updatedAttributes, ( v ) => typeof v === 'string' && @@ -821,7 +823,7 @@ export function* mergeBlocks( firstBlockClientId, secondBlockClientId ) { multilineWrapperTags, preserveWhiteSpace, } ); - const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); + newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); const newValue = remove( convertedValue, newOffset, newOffset + 1 ); const newHtml = toHTMLString( { value: newValue, @@ -830,13 +832,6 @@ export function* mergeBlocks( firstBlockClientId, secondBlockClientId ) { } ); updatedAttributes[ newAttributeKey ] = newHtml; - - yield selectionChange( - blockA.clientId, - newAttributeKey, - newOffset, - newOffset - ); } yield* replaceBlocks( @@ -852,6 +847,15 @@ export function* mergeBlocks( firstBlockClientId, secondBlockClientId ) { ...blocksWithTheSameType.slice( 1 ), ] ); + + if ( canRestoreTextSelection ) { + yield selectionChange( + blockA.clientId, + newAttributeKey, + newOffset, + newOffset + ); + } } /** From 3bc07daaa00ba2193c940adb2dafd4a46e78200d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 12 May 2021 12:56:58 +0300 Subject: [PATCH 3/5] Fix selection --- packages/block-editor/src/store/actions.js | 2 +- packages/rich-text/src/component/index.js | 24 ++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 9c8767f4c5c0e..e7a31b72f201b 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -711,7 +711,7 @@ export function* mergeBlocks( firstBlockClientId, secondBlockClientId ) { // Only focus the previous block if it's not mergeable if ( ! blockAType.merge ) { - yield selectBlock( blockA.clientId ); + yield selectBlock( blockA.clientId, -1 ); return; } diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index c671633d54c4e..1dc39487e3c0f 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -74,6 +74,11 @@ export function useRichText( { // Internal values are updated synchronously, unlike props and state. const _value = useRef( value ); const record = useRef(); + const previousRecord = useRef( {} ); + + useLayoutEffect( () => { + previousRecord.current = record.current; + }, [ record.current ] ); function setRecordFromProps() { _value.current = value; @@ -95,6 +100,15 @@ export function useRichText( { if ( ! record.current ) { setRecordFromProps(); + } else if ( + selectionStart !== record.current.start || + selectionEnd !== record.current.end + ) { + record.current = { + ...record.current, + start: selectionStart, + end: selectionEnd, + }; } /** @@ -161,16 +175,10 @@ export function useRichText( { if ( isSelected && - ( selectionStart !== record.current.start || - selectionEnd !== record.current.end ) + ( selectionStart !== previousRecord.current.start || + selectionEnd !== previousRecord.current.end ) ) { applyFromProps(); - } else { - record.current = { - ...record.current, - start: selectionStart, - end: selectionEnd, - }; } }, [ selectionStart, selectionEnd, isSelected ] ); From a36222004d51f92b10a5c2e3a5b65ede8f67b25d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 12 May 2021 13:18:42 +0300 Subject: [PATCH 4/5] Fix unit tests --- .../block-editor/src/store/test/actions.js | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index 6433f15e6523e..0fc931602e738 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -1187,7 +1187,7 @@ describe( 'actions', () => { fulfillment.next(); expect( fulfillment.next( blockA ) ).toEqual( { done: false, - value: selectBlock( 'chicken' ), + value: selectBlock( 'chicken', -1 ), } ); expect( fulfillment.next( blockA ).done ).toEqual( true ); } ); @@ -1236,15 +1236,6 @@ describe( 'actions', () => { attributeKey: 'content', offset: 0, } ); - // selectionChange - fulfillment.next( - selectionChange( - blockA.clientId, - 'content', - 'chicken'.length + 1, - 'chicken'.length + 1 - ) - ); fulfillment.next(); fulfillment.next(); expect( fulfillment.next( blockA ).value ).toMatchObject( { @@ -1258,6 +1249,15 @@ describe( 'actions', () => { }, ], } ); + fulfillment.next(); + expect( fulfillment.next().value ).toEqual( + selectionChange( + blockA.clientId, + 'content', + 'chicken'.length + 1, + 'chicken'.length + 1 + ) + ); } ); it( 'should not merge the blocks have different types without transformation', () => { @@ -1382,22 +1382,11 @@ describe( 'actions', () => { storeKey: blockEditorStoreName, type: '@@data/SELECT', } ); - expect( - fulfillment.next( { - clientId: blockB.clientId, - attributeKey: 'content2', - offset: 0, - } ).value - ).toEqual( - selectionChange( - blockA.clientId, - 'content', - 'chicken'.length + 1, - 'chicken'.length + 1 - ) - ); - - fulfillment.next(); + fulfillment.next( { + clientId: blockB.clientId, + attributeKey: 'content2', + offset: 0, + } ); fulfillment.next(); fulfillment.next(); expect( fulfillment.next( blockA ).value ).toMatchObject( { @@ -1411,6 +1400,15 @@ describe( 'actions', () => { }, ], } ); + fulfillment.next(); + expect( fulfillment.next().value ).toEqual( + selectionChange( + blockA.clientId, + 'content', + 'chicken'.length + 1, + 'chicken'.length + 1 + ) + ); } ); } ); From 24d33525b297ad9d7cbd8ac4f6b6cb03f9c8a429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 12 May 2021 13:31:26 +0300 Subject: [PATCH 5/5] wip --- packages/rich-text/src/component/index.js | 35 +++++++---------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 1dc39487e3c0f..047e669b4b11d 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -98,17 +98,22 @@ export function useRichText( { record.current.end = selectionEnd; } + function applyFromProps( { domOnly } = {} ) { + setRecordFromProps(); + applyRecord( record.current, { domOnly } ); + } + if ( ! record.current ) { setRecordFromProps(); } else if ( selectionStart !== record.current.start || selectionEnd !== record.current.end ) { - record.current = { - ...record.current, - start: selectionStart, - end: selectionEnd, - }; + if ( isSelected ) { + applyFromProps(); + } else { + setRecordFromProps(); + } } /** @@ -153,11 +158,6 @@ export function useRichText( { setActiveFormats( newActiveFormats ); } - function applyFromProps( { domOnly } = {} ) { - setRecordFromProps(); - applyRecord( record.current, { domOnly } ); - } - const didMount = useRef( false ); // Value updates must happen synchonously to avoid overwriting newer values. @@ -167,21 +167,6 @@ export function useRichText( { } }, [ value ] ); - // Value updates must happen synchonously to avoid overwriting newer values. - useLayoutEffect( () => { - if ( ! didMount.current ) { - return; - } - - if ( - isSelected && - ( selectionStart !== previousRecord.current.start || - selectionEnd !== previousRecord.current.end ) - ) { - applyFromProps(); - } - }, [ selectionStart, selectionEnd, isSelected ] ); - function focus() { ref.current.focus(); applyRecord( record.current );