diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index ef21a8aa4ab23..51a70677a5edc 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -325,10 +325,13 @@ export function RichTextWrapper( { ...props } { ...autocompleteProps } ref={ useMergeRefs( [ + // Rich text ref must be first because its focus listener + // must be set up before any other ref calls .focus() on + // mount. + richTextRef, forwardedRef, autocompleteProps.ref, props.ref, - richTextRef, useBeforeInputRules( { value, onChange } ), useInputRules( { getValue, @@ -387,6 +390,7 @@ export function RichTextWrapper( // tabIndex because Safari will focus the element. However, // Safari will correctly ignore nested contentEditable elements. tabIndex={ props.tabIndex === 0 ? null : props.tabIndex } + data-wp-block-attribute-key={ identifier } /> ); diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index f15c1dac0267f..c76ab74b03b77 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -47,7 +47,6 @@ export function useWritingFlow() { useRefEffect( ( node ) => { node.tabIndex = 0; - node.contentEditable = hasMultiSelection; if ( ! hasMultiSelection ) { return; diff --git a/packages/block-editor/src/components/writing-flow/use-drag-selection.js b/packages/block-editor/src/components/writing-flow/use-drag-selection.js index e013776e49c7a..8d8791afe5f09 100644 --- a/packages/block-editor/src/components/writing-flow/use-drag-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-drag-selection.js @@ -27,8 +27,12 @@ function setContentEditableWrapper( node, value ) { export default function useDragSelection() { const { startMultiSelect, stopMultiSelect } = useDispatch( blockEditorStore ); - const { isSelectionEnabled, hasMultiSelection, isDraggingBlocks } = - useSelect( blockEditorStore ); + const { + isSelectionEnabled, + hasSelectedBlock, + isDraggingBlocks, + isMultiSelecting, + } = useSelect( blockEditorStore ); return useRefEffect( ( node ) => { const { ownerDocument } = node; @@ -45,7 +49,7 @@ export default function useDragSelection() { // so wait until the next animation frame to get the browser // selection. rafId = defaultView.requestAnimationFrame( () => { - if ( hasMultiSelection() ) { + if ( ! hasSelectedBlock() ) { return; } @@ -84,6 +88,16 @@ export default function useDragSelection() { return; } + // Abort if we are already multi-selecting. + if ( isMultiSelecting() ) { + return; + } + + // Abort if selection is leaving writing flow. + if ( node === target ) { + return; + } + // Check the attribute, not the contentEditable attribute. All // child elements of the content editable wrapper are editable // and return true for this property. We only want to start @@ -127,7 +141,7 @@ export default function useDragSelection() { startMultiSelect, stopMultiSelect, isSelectionEnabled, - hasMultiSelection, + hasSelectedBlock, ] ); } diff --git a/packages/block-editor/src/components/writing-flow/use-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-selection-observer.js index b780950dae0ea..57082f90c055f 100644 --- a/packages/block-editor/src/components/writing-flow/use-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-selection-observer.js @@ -3,6 +3,7 @@ */ import { useSelect, useDispatch } from '@wordpress/data'; import { useRefEffect } from '@wordpress/compose'; +import { create } from '@wordpress/rich-text'; /** * Internal dependencies @@ -75,10 +76,20 @@ function setContentEditableWrapper( node, value ) { // Since we are calling this on every selection change, check if the value // needs to be updated first because it trigger the browser to recalculate // style. - if ( node.contentEditable !== String( value ) ) + if ( node.contentEditable !== String( value ) ) { node.contentEditable = value; - // Firefox doesn't automatically move focus. - if ( value ) node.focus(); + + // Firefox doesn't automatically move focus. + if ( value ) { + node.focus(); + } + } +} + +function getRichTextElement( node ) { + const element = + node.nodeType === node.ELEMENT_NODE ? node : node.parentElement; + return element?.closest( '[data-wp-block-attribute-key]' ); } /** @@ -87,7 +98,7 @@ function setContentEditableWrapper( node, value ) { export default function useSelectionObserver() { const { multiSelect, selectBlock, selectionChange } = useDispatch( blockEditorStore ); - const { getBlockParents, getBlockSelectionStart } = + const { getBlockParents, getBlockSelectionStart, isMultiSelecting } = useSelect( blockEditorStore ); return useRefEffect( ( node ) => { @@ -101,6 +112,16 @@ export default function useSelectionObserver() { return; } + const startNode = extractSelectionStartNode( selection ); + const endNode = extractSelectionEndNode( selection ); + + if ( + ! node.contains( startNode ) || + ! node.contains( endNode ) + ) { + return; + } + // If selection is collapsed and we haven't used `shift+click`, // end multi selection and disable the contentEditable wrapper. // We have to check about `shift+click` case because elements @@ -109,16 +130,24 @@ export default function useSelectionObserver() { // For now we check if the event is a `mouse` event. const isClickShift = event.shiftKey && event.type === 'mouseup'; if ( selection.isCollapsed && ! isClickShift ) { - setContentEditableWrapper( node, false ); + if ( + node.contentEditable === 'true' && + ! isMultiSelecting() + ) { + setContentEditableWrapper( node, false ); + let element = + startNode.nodeType === startNode.ELEMENT_NODE + ? startNode + : startNode.parentElement; + element = element?.closest( '[contenteditable]' ); + element?.focus(); + } return; } - let startClientId = getBlockClientId( - extractSelectionStartNode( selection ) - ); - let endClientId = getBlockClientId( - extractSelectionEndNode( selection ) - ); + let startClientId = getBlockClientId( startNode ); + let endClientId = getBlockClientId( endNode ); + // If the selection has changed and we had pressed `shift+click`, // we need to check if in an element that doesn't support // text selection has been clicked. @@ -155,7 +184,11 @@ export default function useSelectionObserver() { const isSingularSelection = startClientId === endClientId; if ( isSingularSelection ) { - selectBlock( startClientId ); + if ( ! isMultiSelecting() ) { + selectBlock( startClientId ); + } else { + multiSelect( startClientId, startClientId ); + } } else { const startPath = [ ...getBlockParents( startClientId ), @@ -167,39 +200,68 @@ export default function useSelectionObserver() { ]; const depth = findDepth( startPath, endPath ); - multiSelect( startPath[ depth ], endPath[ depth ] ); - } - } + if ( + startPath[ depth ] !== startClientId || + endPath[ depth ] !== endClientId + ) { + multiSelect( startPath[ depth ], endPath[ depth ] ); + return; + } - function addListeners() { - ownerDocument.addEventListener( - 'selectionchange', - onSelectionChange - ); - defaultView.addEventListener( 'mouseup', onSelectionChange ); + const richTextElementStart = + getRichTextElement( startNode ); + const richTextElementEnd = getRichTextElement( endNode ); + + if ( richTextElementStart && richTextElementEnd ) { + const range = selection.getRangeAt( 0 ); + const richTextDataStart = create( { + element: richTextElementStart, + range, + __unstableIsEditableTree: true, + } ); + const richTextDataEnd = create( { + element: richTextElementEnd, + range, + __unstableIsEditableTree: true, + } ); + + const startOffset = + richTextDataStart.start ?? richTextDataStart.end; + const endOffset = + richTextDataEnd.start ?? richTextDataEnd.end; + selectionChange( { + start: { + clientId: startClientId, + attributeKey: + richTextElementStart.dataset + .wpBlockAttributeKey, + offset: startOffset, + }, + end: { + clientId: endClientId, + attributeKey: + richTextElementEnd.dataset + .wpBlockAttributeKey, + offset: endOffset, + }, + } ); + } else { + multiSelect( startClientId, endClientId ); + } + } } - function removeListeners() { + ownerDocument.addEventListener( + 'selectionchange', + onSelectionChange + ); + defaultView.addEventListener( 'mouseup', onSelectionChange ); + return () => { ownerDocument.removeEventListener( 'selectionchange', onSelectionChange ); defaultView.removeEventListener( 'mouseup', onSelectionChange ); - } - - function resetListeners() { - removeListeners(); - addListeners(); - } - - addListeners(); - // We must allow rich text to set selection first. This ensures that - // our `selectionchange` listener is always reset to be called after - // the rich text one. - node.addEventListener( 'focusin', resetListeners ); - return () => { - removeListeners(); - node.removeEventListener( 'focusin', resetListeners ); }; }, [ multiSelect, selectBlock, selectionChange, getBlockParents ] diff --git a/packages/rich-text/src/component/use-input-and-selection.js b/packages/rich-text/src/component/use-input-and-selection.js index 870bfe170635c..ecfcc2151a989 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -126,48 +126,14 @@ export function useInputAndSelection( props ) { return; } - // If the selection changes where the active element is a parent of - // the rich text instance (writing flow), call `onSelectionChange` - // for the rich text instance that contains the start or end of the - // selection. + // Ensure the active element is the rich text element. if ( ownerDocument.activeElement !== element ) { - // Only process if the active elment is contentEditable, either - // this rich text instance or the writing flow parent. Fixes a - // bug in Firefox where it strangely selects the closest - // contentEditable element, even though the click was outside - // any contentEditable element. - if ( ownerDocument.activeElement.contentEditable !== 'true' ) { - return; - } - - if ( ! ownerDocument.activeElement.contains( element ) ) { - return; - } - - const selection = defaultView.getSelection(); - const { anchorNode, focusNode } = selection; - - if ( - element.contains( anchorNode ) && - element !== anchorNode && - element.contains( focusNode ) && - element !== focusNode - ) { - const { start, end } = createRecord(); - record.current.activeFormats = EMPTY_ACTIVE_FORMATS; - onSelectionChange( start, end ); - } else if ( - element.contains( anchorNode ) && - element !== anchorNode - ) { - const { start, end: offset = start } = createRecord(); - record.current.activeFormats = EMPTY_ACTIVE_FORMATS; - onSelectionChange( offset ); - } else if ( element.contains( focusNode ) ) { - const { start, end: offset = start } = createRecord(); - record.current.activeFormats = EMPTY_ACTIVE_FORMATS; - onSelectionChange( undefined, offset ); - } + // If it is not, we can stop listening for selection changes. + // We resume listening when the element is focused. + ownerDocument.removeEventListener( + 'selectionchange', + handleSelectionChange + ); return; } @@ -276,18 +242,21 @@ export function useInputAndSelection( props ) { }; } else { applyRecord( record.current ); - onSelectionChange( record.current.start, record.current.end ); } + + onSelectionChange( record.current.start, record.current.end ); + + ownerDocument.addEventListener( + 'selectionchange', + handleSelectionChange + ); } element.addEventListener( 'input', onInput ); element.addEventListener( 'compositionstart', onCompositionStart ); element.addEventListener( 'compositionend', onCompositionEnd ); element.addEventListener( 'focus', onFocus ); - ownerDocument.addEventListener( - 'selectionchange', - handleSelectionChange - ); + return () => { element.removeEventListener( 'input', onInput ); element.removeEventListener( @@ -296,10 +265,6 @@ export function useInputAndSelection( props ) { ); element.removeEventListener( 'compositionend', onCompositionEnd ); element.removeEventListener( 'focus', onFocus ); - ownerDocument.removeEventListener( - 'selectionchange', - handleSelectionChange - ); }; }, [] ); } diff --git a/test/e2e/specs/editor/various/multi-block-selection.spec.js b/test/e2e/specs/editor/various/multi-block-selection.spec.js index 585e4f1851373..a7d7444a223f7 100644 --- a/test/e2e/specs/editor/various/multi-block-selection.spec.js +++ b/test/e2e/specs/editor/various/multi-block-selection.spec.js @@ -693,6 +693,16 @@ test.describe( 'Multi-block selection', () => { force: true, } ); + await expect + .poll( multiBlockSelectionUtils.getSelectedFlatIndices ) + .toEqual( [ 1 ] ); + + await paragraph1.click( { + position: { x: -1, y: 0 }, + // Use force since it's outside the bounding box of the element. + force: true, + } ); + await expect .poll( () => page.evaluate( () => window.getSelection().rangeCount )