From d765a154de167c52bc791845d5f9127c15a4b705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 17 Feb 2022 21:58:02 +0200 Subject: [PATCH 01/67] Initial attempt --- .../data/data-core-block-editor.md | 3 +- .../use-block-props/use-multi-selection.js | 2 +- .../src/components/block-tools/index.js | 38 ++- .../components/keyboard-shortcuts/index.js | 2 +- .../src/components/rich-text/index.js | 38 ++- .../writing-flow/use-multi-selection.js | 29 ++- packages/block-editor/src/store/actions.js | 229 ++++++++++++++---- packages/block-editor/src/store/reducer.js | 35 ++- packages/block-editor/src/utils/dom.js | 23 +- packages/rich-text/src/component/index.js | 5 + .../src/component/use-input-and-selection.js | 40 ++- 11 files changed, 345 insertions(+), 99 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 798871d54b1e8..2e7ecf3ea5cd9 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1218,6 +1218,7 @@ _Parameters_ - _firstBlockClientId_ `string`: Client ID of the first block to merge. - _secondBlockClientId_ `string`: Client ID of the second block to merge. +- _toRemove_ `Array?`: Array of block client IDs to remove. ### moveBlocksDown @@ -1390,7 +1391,7 @@ Action that changes the position of the user caret. _Parameters_ -- _clientId_ `string`: The selected block client ID. +- _clientId_ `string|Object`: The selected block client ID. - _attributeKey_ `string`: The selected block attribute key. - _startOffset_ `number`: The start offset. - _endOffset_ `number`: The end offset. diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js b/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js index 23fd567bb738d..a0271b8e09dec 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js @@ -97,7 +97,7 @@ export function useMultiSelection( clientId ) { ]; const depth = Math.min( startPath.length, endPath.length ) - 1; - + // Check if selection is already set by rich text. multiSelect( startPath[ depth ], endPath[ depth ] ); } } diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index e4d4c589a160b..51b2cd99f6db6 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -19,6 +19,17 @@ import BlockPopover from './block-popover'; import { store as blockEditorStore } from '../../store'; import BlockContextualToolbar from './block-contextual-toolbar'; import { usePopoverScroll } from './use-popover-scroll'; +import { getBlockNode } from '../../utils/dom'; + +function isSimpleContentEditable( node ) { + return ( + node.getAttribute( 'contenteditable' ) === 'true' && + ( ! node.firstElementChild || + node.ownerDocument.defaultView.getComputedStyle( + node.firstElementChild + ).display === 'inline' ) + ); +} /** * Renders block tools (the block toolbar, select/navigation mode toolbar, the @@ -51,6 +62,7 @@ export default function BlockTools( { clearSelectedBlock, moveBlocksUp, moveBlocksDown, + mergeBlocks, } = useDispatch( blockEditorStore ); function onKeyDown( event ) { @@ -106,11 +118,33 @@ export default function BlockTools( { ) { return; } + const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length > 1 ) { + + if ( clientIds.length < 2 ) { + return; + } + + const { ownerDocument } = event.target; + const { defaultView } = ownerDocument; + const { anchorNode, focusNode } = defaultView.getSelection(); + + // To do: find other way to check if blocks are mergeable. + if ( + isSimpleContentEditable( getBlockNode( anchorNode ) ) && + isSimpleContentEditable( getBlockNode( focusNode ) ) + ) { + mergeBlocks( + first( clientIds ), + last( clientIds ), + clientIds.slice( 1, -1 ) + ); event.preventDefault(); - removeBlocks( clientIds ); + return; } + + event.preventDefault(); + removeBlocks( clientIds ); } else if ( isMatch( 'core/block-editor/unselect', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length > 1 ) { diff --git a/packages/block-editor/src/components/keyboard-shortcuts/index.js b/packages/block-editor/src/components/keyboard-shortcuts/index.js index 4c88b7675f877..77a5e7852f0de 100644 --- a/packages/block-editor/src/components/keyboard-shortcuts/index.js +++ b/packages/block-editor/src/components/keyboard-shortcuts/index.js @@ -61,7 +61,7 @@ function KeyboardShortcutsRegister() { registerShortcut( { name: 'core/block-editor/delete-multi-selection', category: 'block', - description: __( 'Remove multiple selected blocks.' ), + description: __( 'Merge or remove multiple selected blocks.' ), keyCombination: { character: 'del', }, diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 3a44454616ef6..d0c86dbe47189 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -131,7 +131,10 @@ function RichTextWrapper( if ( originalIsSelected === undefined ) { isSelected = selectionStart.clientId === clientId && - selectionStart.attributeKey === identifier; + selectionEnd.clientId === clientId && + selectionStart.attributeKey === identifier && + typeof selectionStart.offset === 'number' && + typeof selectionEnd.offset === 'number'; } else if ( originalIsSelected ) { isSelected = selectionStart.clientId === clientId; } @@ -171,7 +174,26 @@ function RichTextWrapper( const onSelectionChange = useCallback( ( start, end ) => { - selectionChange( clientId, identifier, start, end ); + const selection = {}; + const unset = start === undefined && end === undefined; + + if ( typeof start === 'number' || unset ) { + selection.start = { + clientId, + attributeKey: identifier, + offset: start, + }; + } + + if ( typeof end === 'number' || unset ) { + selection.end = { + clientId, + attributeKey: identifier, + offset: end, + }; + } + + selectionChange( selection ); }, [ clientId, identifier ] ); @@ -227,8 +249,16 @@ function RichTextWrapper( changeHandler( __unstableFormats, __unstableText ); } ); }, - selectionStart, - selectionEnd, + selectionStart: + typeof selectionStart === 'number' && + typeof selectionEnd === 'number' + ? selectionStart + : undefined, + selectionEnd: + typeof selectionStart === 'number' && + typeof selectionEnd === 'number' + ? selectionEnd + : undefined, onSelectionChange, placeholder, __unstableIsSelected: isSelected, diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index b4e7ed6bd0c52..eb2a19eaad76c 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -41,6 +41,16 @@ function getDeepestNode( node, type ) { return node; } +function isSimpleContentEditable( node ) { + return ( + node.getAttribute( 'contenteditable' ) === 'true' && + ( ! node.firstElementChild || + node.ownerDocument.defaultView.getComputedStyle( + node.firstElementChild + ).display === 'inline' ) + ); +} + function selector( select ) { const { isMultiSelecting, @@ -120,12 +130,6 @@ export default function useMultiSelection() { return; } - // The block refs might not be immediately available - // when dragging blocks into another block. - if ( ! startRef.current || ! endRef.current ) { - return; - } - // Allow cross contentEditable selection by temporarily making // all content editable. We can't rely on using the store and // React because re-rending happens too slowly. We need to be @@ -136,6 +140,19 @@ export default function useMultiSelection() { // BEFORE selection. node.focus(); + // The block refs might not be immediately available + // when dragging blocks into another block. + if ( ! startRef.current || ! endRef.current ) { + return; + } + + if ( + isSimpleContentEditable( startRef.current ) && + isSimpleContentEditable( endRef.current ) + ) { + return; + } + const selection = defaultView.getSelection(); const range = ownerDocument.createRange(); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index bc5bf2133045b..51cacec785011 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -658,20 +658,170 @@ export const synchronizeTemplate = () => ( { select, dispatch } ) => { dispatch.resetBlocks( updatedBlockList ); }; +function mapRichTextSettings( attributeDefinition ) { + const { + multiline: multilineTag, + __unstableMultilineWrapperTags: multilineWrapperTags, + __unstablePreserveWhiteSpace: preserveWhiteSpace, + } = attributeDefinition; + return { + multilineTag, + multilineWrapperTags, + preserveWhiteSpace, + }; +} + /** * Action that merges two blocks. * * @param {string} firstBlockClientId Client ID of the first block to merge. * @param {string} secondBlockClientId Client ID of the second block to merge. + * @param {Array?} toRemove Array of block client IDs to remove. */ -export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { - select, - dispatch, -} ) => { +export const mergeBlocks = ( + firstBlockClientId, + secondBlockClientId, + toRemove +) => ( { registry, select, dispatch } ) => { const blocks = [ firstBlockClientId, secondBlockClientId ]; + const [ clientIdA, clientIdB ] = blocks; + + const selectionStart = select.getSelectionStart(); + const selectionEnd = select.getSelectionEnd(); + + // To do: merge logic with existing mergeBlocks logic. + if ( selectionStart.clientId !== selectionEnd.clientId ) { + let selectionA, selectionB; + + if ( selectionStart.clientId === clientIdA ) { + selectionA = selectionStart; + } else if ( selectionStart.clientId === clientIdB ) { + selectionB = selectionStart; + } + + if ( selectionEnd.clientId === clientIdA ) { + selectionA = selectionEnd; + } else if ( selectionEnd.clientId === clientIdB ) { + selectionB = selectionEnd; + } + // Abort if the selection doesn't match with blocks to merge. + if ( ! selectionA || ! selectionB ) { + return; + } + + const blockA = select.getBlock( clientIdA ); + const blockAType = getBlockType( blockA.name ); + const blockB = select.getBlock( clientIdB ); + const blockBType = getBlockType( blockB.name ); + + // A robust way to retain selection position through various transforms + // is to insert a special character at the position and then recover it. + const START_OF_SELECTED_AREA = '\u0086'; + + // Clone the blocks so we don't insert the character in a "live" block. + const cloneA = cloneBlock( blockA ); + const cloneB = cloneBlock( blockB ); + + const attributeDefinitionA = + blockAType.attributes[ selectionA.attributeKey ]; + const attributeDefinitionB = + blockBType.attributes[ selectionB.attributeKey ]; + + const htmlA = cloneA.attributes[ selectionA.attributeKey ]; + const htmlB = cloneB.attributes[ selectionB.attributeKey ]; + + let valueA = create( { + html: htmlA, + ...mapRichTextSettings( attributeDefinitionA ), + } ); + let valueB = create( { + html: htmlB, + ...mapRichTextSettings( attributeDefinitionB ), + } ); + + valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); + valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); + + cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { + value: valueA, + ...mapRichTextSettings( attributeDefinitionA ), + } ); + cloneB.attributes[ selectionB.attributeKey ] = toHTMLString( { + value: valueB, + ...mapRichTextSettings( attributeDefinitionB ), + } ); + + // We can only merge blocks with similar types + // thus, we transform the block to merge first + const blocksWithTheSameType = + blockA.name === blockB.name + ? [ cloneB ] + : switchToBlockType( cloneB, blockA.name ); + + // If the block types can not match, do nothing + if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { + return; + } + + // Calling the merge to update the attributes and remove the block to be merged + const updatedAttributes = blockAType.merge( + cloneA.attributes, + blocksWithTheSameType[ 0 ].attributes + ); + + const newAttributeKey = findKey( + updatedAttributes, + ( v ) => + typeof v === 'string' && + v.indexOf( START_OF_SELECTED_AREA ) !== -1 + ); + const convertedHtml = updatedAttributes[ newAttributeKey ]; + const convertedValue = create( { + html: convertedHtml, + ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), + } ); + const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); + const newValue = remove( convertedValue, newOffset, newOffset + 1 ); + const newHtml = toHTMLString( { + value: newValue, + ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), + } ); + + updatedAttributes[ newAttributeKey ] = newHtml; + + registry.batch( () => { + dispatch.selectionChange( + blockA.clientId, + newAttributeKey, + newOffset, + newOffset + ); + + if ( toRemove && toRemove.length ) + dispatch.removeBlocks( toRemove ); + + dispatch.replaceBlocks( + [ blockA.clientId, blockB.clientId ], + [ + { + ...blockA, + attributes: { + ...blockA.attributes, + ...updatedAttributes, + }, + }, + ...blocksWithTheSameType.slice( 1 ), + ], + 0, // If we don't pass the `indexToSelect` it will default to the last block. + select.getSelectedBlocksInitialCaretPosition() + ); + } ); + + return; + } + dispatch( { type: 'MERGE_BLOCKS', blocks } ); - const [ clientIdA, clientIdB ] = blocks; const blockA = select.getBlock( clientIdA ); const blockAType = getBlockType( blockA.name ); @@ -683,7 +833,16 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { const blockB = select.getBlock( clientIdB ); const blockBType = getBlockType( blockB.name ); - const { clientId, attributeKey, offset } = select.getSelectionStart(); + + // A robust way to retain selection position through various transforms + // is to insert a special character at the position and then recover it. + const START_OF_SELECTED_AREA = '\u0086'; + + // Clone the blocks so we don't insert the character in a "live" block. + const cloneA = cloneBlock( blockA ); + const cloneB = cloneBlock( blockB ); + + const { clientId, attributeKey, offset } = selectionStart; const selectedBlockType = clientId === clientIdA ? blockAType : blockBType; const attributeDefinition = selectedBlockType.attributes[ attributeKey ]; const canRestoreTextSelection = @@ -708,28 +867,13 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { } } - // A robust way to retain selection position through various transforms - // is to insert a special character at the position and then recover it. - const START_OF_SELECTED_AREA = '\u0086'; - - // Clone the blocks so we don't insert the character in a "live" block. - const cloneA = cloneBlock( blockA ); - const cloneB = cloneBlock( blockB ); - if ( canRestoreTextSelection ) { const selectedBlock = clientId === clientIdA ? cloneA : cloneB; const html = selectedBlock.attributes[ attributeKey ]; - const { - multiline: multilineTag, - __unstableMultilineWrapperTags: multilineWrapperTags, - __unstablePreserveWhiteSpace: preserveWhiteSpace, - } = attributeDefinition; const value = insert( create( { html, - multilineTag, - multilineWrapperTags, - preserveWhiteSpace, + ...mapRichTextSettings( attributeDefinition ), } ), START_OF_SELECTED_AREA, offset, @@ -738,8 +882,7 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { selectedBlock.attributes[ attributeKey ] = toHTMLString( { value, - multilineTag, - preserveWhiteSpace, + ...mapRichTextSettings( attributeDefinition ), } ); } @@ -769,23 +912,15 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { v.indexOf( START_OF_SELECTED_AREA ) !== -1 ); const convertedHtml = updatedAttributes[ newAttributeKey ]; - const { - multiline: multilineTag, - __unstableMultilineWrapperTags: multilineWrapperTags, - __unstablePreserveWhiteSpace: preserveWhiteSpace, - } = blockAType.attributes[ newAttributeKey ]; const convertedValue = create( { html: convertedHtml, - multilineTag, - multilineWrapperTags, - preserveWhiteSpace, + ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), } ); const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); const newValue = remove( convertedValue, newOffset, newOffset + 1 ); const newHtml = toHTMLString( { value: newValue, - multilineTag, - preserveWhiteSpace, + ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), } ); updatedAttributes[ newAttributeKey ] = newHtml; @@ -978,10 +1113,10 @@ export function exitFormattedText() { /** * Action that changes the position of the user caret. * - * @param {string} clientId The selected block client ID. - * @param {string} attributeKey The selected block attribute key. - * @param {number} startOffset The start offset. - * @param {number} endOffset The end offset. + * @param {string|Object} clientId The selected block client ID. + * @param {string} attributeKey The selected block attribute key. + * @param {number} startOffset The start offset. + * @param {number} endOffset The end offset. * * @return {Object} Action object. */ @@ -991,13 +1126,17 @@ export function selectionChange( startOffset, endOffset ) { - return { - type: 'SELECTION_CHANGE', - clientId, - attributeKey, - startOffset, - endOffset, - }; + if ( typeof clientId === 'string' ) { + return { + type: 'SELECTION_CHANGE', + clientId, + attributeKey, + startOffset, + endOffset, + }; + } + + return { type: 'SELECTION_CHANGE', ...clientId }; } /** diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 76e015b796128..1a388d117d860 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -1278,17 +1278,24 @@ function selectionHelper( state = {}, action ) { export function selection( state = {}, action ) { switch ( action.type ) { case 'SELECTION_CHANGE': + if ( action.clientId ) { + return { + selectionStart: { + clientId: action.clientId, + attributeKey: action.attributeKey, + offset: action.startOffset, + }, + selectionEnd: { + clientId: action.clientId, + attributeKey: action.attributeKey, + offset: action.endOffset, + }, + }; + } + return { - selectionStart: { - clientId: action.clientId, - attributeKey: action.attributeKey, - offset: action.startOffset, - }, - selectionEnd: { - clientId: action.clientId, - attributeKey: action.attributeKey, - offset: action.endOffset, - }, + selectionStart: action.start || state.selectionStart, + selectionEnd: action.end || state.selectionEnd, }; case 'RESET_SELECTION': const { selectionStart, selectionEnd } = action; @@ -1298,6 +1305,14 @@ export function selection( state = {}, action ) { }; case 'MULTI_SELECT': const { start, end } = action; + + if ( + start === state.selectionStart.clientId && + end === state.selectionEnd.clientId + ) { + return state; + } + return { selectionStart: { clientId: start }, selectionEnd: { clientId: end }, diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 6de80b24dac4b..8a51fa7469b62 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -31,24 +31,35 @@ export function isInsideRootBlock( blockElement, element ) { } /** - * Finds the block client ID given any DOM node inside the block. + * Finds the block node given any DOM node inside the block. * * @param {Node?} node DOM node. * - * @return {string|undefined} Client ID or undefined if the node is not part of + * @return {Element|null} Client ID or undefined if the node is not part of * a block. */ -export function getBlockClientId( node ) { +export function getBlockNode( node ) { while ( node && node.nodeType !== node.ELEMENT_NODE ) { node = node.parentNode; } if ( ! node ) { - return; + return null; } - const elementNode = /** @type {Element} */ ( node ); - const blockNode = elementNode.closest( BLOCK_SELECTOR ); + return /** @type {Element} */ ( node ).closest( BLOCK_SELECTOR ); +} + +/** + * Finds the block client ID given any DOM node inside the block. + * + * @param {Node?} node DOM node. + * + * @return {string|undefined} Client ID or undefined if the node is not part of + * a block. + */ +export function getBlockClientId( node ) { + const blockNode = getBlockNode( node ); if ( ! blockNode ) { return; diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 3f6c9c1260374..93f7cef03d2c0 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -214,6 +214,11 @@ export function useRichText( { return; } + // For merging multi selection, focus is on writing flow, so it should + // move to the selection after merge. This will probably break some + // things. + if ( ref.current.ownerDocument.activeElement !== ref.current ) + ref.current.focus(); applyFromProps(); hadSelectionUpdate.current = false; }, [ hadSelectionUpdate.current ] ); 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 2e0eebea69911..e36d00bfcf93b 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -124,10 +124,6 @@ export function useInputAndSelection( props ) { * @param {Event|DOMHighResTimeStamp} event */ function handleSelectionChange( event ) { - if ( ownerDocument.activeElement !== element ) { - return; - } - const { record, applyRecord, @@ -136,14 +132,22 @@ export function useInputAndSelection( props ) { onSelectionChange, } = propsRef.current; - if ( event.type !== 'selectionchange' && ! isSelected ) { + if ( ownerDocument.activeElement !== element ) { + const selection = defaultView.getSelection(); + const { anchorNode, focusNode } = selection; + + if ( + element.contains( anchorNode ) || + element.contains( focusNode ) + ) { + const { start, end } = createRecord(); + record.current.activeFormats = EMPTY_ACTIVE_FORMATS; + onSelectionChange( start, end ); + } return; } - // Check if the implementor disabled editing. `contentEditable` - // does disable input, but not text selection, so we must ignore - // selection changes. - if ( element.contentEditable !== 'true' ) { + if ( event.type !== 'selectionchange' && ! isSelected ) { return; } @@ -254,25 +258,12 @@ export function useInputAndSelection( props ) { // at this point, but this focus event is still too early to calculate // the selection. rafId = defaultView.requestAnimationFrame( handleSelectionChange ); - - ownerDocument.addEventListener( - 'selectionchange', - handleSelectionChange - ); - } - - function onBlur() { - ownerDocument.removeEventListener( - 'selectionchange', - handleSelectionChange - ); } element.addEventListener( 'input', onInput ); element.addEventListener( 'compositionstart', onCompositionStart ); element.addEventListener( 'compositionend', onCompositionEnd ); element.addEventListener( 'focus', onFocus ); - element.addEventListener( 'blur', onBlur ); // Selection updates must be done at these events as they // happen before the `selectionchange` event. In some cases, // the `selectionchange` event may not even fire, for @@ -280,6 +271,10 @@ export function useInputAndSelection( props ) { element.addEventListener( 'keyup', handleSelectionChange ); element.addEventListener( 'mouseup', handleSelectionChange ); element.addEventListener( 'touchend', handleSelectionChange ); + ownerDocument.addEventListener( + 'selectionchange', + handleSelectionChange + ); return () => { element.removeEventListener( 'input', onInput ); element.removeEventListener( @@ -288,7 +283,6 @@ export function useInputAndSelection( props ) { ); element.removeEventListener( 'compositionend', onCompositionEnd ); element.removeEventListener( 'focus', onFocus ); - element.removeEventListener( 'blur', onBlur ); element.removeEventListener( 'keyup', handleSelectionChange ); element.removeEventListener( 'mouseup', handleSelectionChange ); element.removeEventListener( 'touchend', handleSelectionChange ); From 60457c37dbe7b16915c117fb9afe0d50e206304d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Feb 2022 15:29:00 +0200 Subject: [PATCH 02/67] Fix selection from bottom to top --- .../src/component/use-input-and-selection.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 e36d00bfcf93b..b5eca0259ce9f 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -136,13 +136,14 @@ export function useInputAndSelection( props ) { const selection = defaultView.getSelection(); const { anchorNode, focusNode } = selection; - if ( - element.contains( anchorNode ) || - element.contains( focusNode ) - ) { - const { start, end } = createRecord(); + if ( element.contains( anchorNode ) ) { + const { start, end: offset = start } = createRecord(); record.current.activeFormats = EMPTY_ACTIVE_FORMATS; - onSelectionChange( start, end ); + onSelectionChange( offset ); + } else if ( element.contains( focusNode ) ) { + const { start, end: offset = start } = createRecord(); + record.current.activeFormats = EMPTY_ACTIVE_FORMATS; + onSelectionChange( undefined, offset ); } return; } From 06cc562922b2bd265603413bbe6ead449a9675da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Feb 2022 15:36:08 +0200 Subject: [PATCH 03/67] Ensure native selection spans multiple blocks --- packages/block-editor/src/components/block-tools/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 51b2cd99f6db6..4020547dbeb60 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -128,11 +128,14 @@ export default function BlockTools( { const { ownerDocument } = event.target; const { defaultView } = ownerDocument; const { anchorNode, focusNode } = defaultView.getSelection(); + const blockA = getBlockNode( anchorNode ); + const blockB = getBlockNode( focusNode ); // To do: find other way to check if blocks are mergeable. if ( - isSimpleContentEditable( getBlockNode( anchorNode ) ) && - isSimpleContentEditable( getBlockNode( focusNode ) ) + isSimpleContentEditable( blockA ) && + isSimpleContentEditable( blockB ) && + blockA !== blockB ) { mergeBlocks( first( clientIds ), From 4a8433932ce13ac22871e15dfd3fc60c026b88e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Feb 2022 20:24:29 +0200 Subject: [PATCH 04/67] Fix isSelected --- packages/block-editor/src/components/rich-text/index.js | 4 +--- .../block-editor/src/components/rich-text/split-value.js | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index d0c86dbe47189..0477927e5967a 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -132,9 +132,7 @@ function RichTextWrapper( isSelected = selectionStart.clientId === clientId && selectionEnd.clientId === clientId && - selectionStart.attributeKey === identifier && - typeof selectionStart.offset === 'number' && - typeof selectionEnd.offset === 'number'; + selectionStart.attributeKey === identifier; } else if ( originalIsSelected ) { isSelected = selectionStart.clientId === clientId; } diff --git a/packages/block-editor/src/components/rich-text/split-value.js b/packages/block-editor/src/components/rich-text/split-value.js index 86e801882dcd2..7ea2b87bc3953 100644 --- a/packages/block-editor/src/components/rich-text/split-value.js +++ b/packages/block-editor/src/components/rich-text/split-value.js @@ -20,8 +20,12 @@ export function splitValue( { return; } + // Ensure the value has a selection. This might happen when trying to split + // an empty value before there was a `selectionchange` event. + const { start = 0, end = 0 } = value; + const valueWithSelection = { ...value, start, end }; const blocks = []; - const [ before, after ] = split( value ); + const [ before, after ] = split( valueWithSelection ); const hasPastedBlocks = pastedBlocks.length > 0; let lastPastedBlockIndex = -1; From b4e52457b74537621707037771b10c51ab0b2962 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Feb 2022 21:32:07 +0200 Subject: [PATCH 05/67] Fix ctrl+A --- .../src/components/writing-flow/use-multi-selection.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index eb2a19eaad76c..ec4168fe9401f 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -146,9 +146,15 @@ export default function useMultiSelection() { return; } + const { anchorNode, focusNode } = defaultView.getSelection(); + if ( isSimpleContentEditable( startRef.current ) && - isSimpleContentEditable( endRef.current ) + ( startRef.current.contains( anchorNode ) || + startRef.current.contains( focusNode ) ) && + isSimpleContentEditable( endRef.current ) && + ( endRef.current.contains( anchorNode ) || + endRef.current.contains( focusNode ) ) ) { return; } From 5d59308d85cc6fc99eeba18d6dabd731beb794bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Feb 2022 00:32:09 +0200 Subject: [PATCH 06/67] Fix e2e tests --- .../src/components/block-tools/index.js | 17 +++++++++++------ packages/block-editor/src/store/actions.js | 4 ++++ .../src/component/use-input-and-selection.js | 10 ++++++++-- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 4020547dbeb60..2b25b94d37229 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -51,9 +51,11 @@ export default function BlockTools( { [] ); const isMatch = useShortcutEventMatch(); - const { getSelectedBlockClientIds, getBlockRootClientId } = useSelect( - blockEditorStore - ); + const { + getSelectedBlockClientIds, + getBlockRootClientId, + getSelectionStart, + } = useSelect( blockEditorStore ); const { duplicateBlocks, removeBlocks, @@ -131,22 +133,25 @@ export default function BlockTools( { const blockA = getBlockNode( anchorNode ); const blockB = getBlockNode( focusNode ); + event.preventDefault(); + + const selectionStart = getSelectionStart(); + // To do: find other way to check if blocks are mergeable. if ( isSimpleContentEditable( blockA ) && isSimpleContentEditable( blockB ) && - blockA !== blockB + blockA !== blockB && + selectionStart.attributeKey ) { mergeBlocks( first( clientIds ), last( clientIds ), clientIds.slice( 1, -1 ) ); - event.preventDefault(); return; } - event.preventDefault(); removeBlocks( clientIds ); } else if ( isMatch( 'core/block-editor/unselect', event ) ) { const clientIds = getSelectedBlockClientIds(); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 51cacec785011..085636c1248cc 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -691,6 +691,10 @@ export const mergeBlocks = ( // To do: merge logic with existing mergeBlocks logic. if ( selectionStart.clientId !== selectionEnd.clientId ) { + if ( ! selectionStart.attributeKey || ! selectionEnd.attributeKey ) { + return; + } + let selectionA, selectionB; if ( selectionStart.clientId === clientIdA ) { 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 b5eca0259ce9f..1d836868ce01c 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -136,11 +136,17 @@ export function useInputAndSelection( props ) { const selection = defaultView.getSelection(); const { anchorNode, focusNode } = selection; - if ( element.contains( anchorNode ) ) { + 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 ) ) { + } else if ( + element.contains( focusNode ) && + element !== focusNode + ) { const { start, end: offset = start } = createRecord(); record.current.activeFormats = EMPTY_ACTIVE_FORMATS; onSelectionChange( undefined, offset ); From a56eef61cce968fcca47f186b867de7a59573214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Feb 2022 18:36:35 +0200 Subject: [PATCH 07/67] Fix text color button --- packages/rich-text/src/component/use-input-and-selection.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 1d836868ce01c..91d139a02f208 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -132,7 +132,10 @@ export function useInputAndSelection( props ) { onSelectionChange, } = propsRef.current; - if ( ownerDocument.activeElement !== element ) { + if ( + ownerDocument.activeElement !== element && + ownerDocument.activeElement.contains( element ) + ) { const selection = defaultView.getSelection(); const { anchorNode, focusNode } = selection; From a467c2d45dda3710604951fc1e946576a5e71ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Feb 2022 18:58:16 +0200 Subject: [PATCH 08/67] Fix early return --- .../src/component/use-input-and-selection.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 91d139a02f208..adb5f9258b171 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -132,10 +132,15 @@ export function useInputAndSelection( props ) { onSelectionChange, } = propsRef.current; - if ( - ownerDocument.activeElement !== element && - ownerDocument.activeElement.contains( element ) - ) { + // 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. + if ( ownerDocument.activeElement !== element ) { + if ( ! ownerDocument.activeElement.contains( element ) ) { + return; + } + const selection = defaultView.getSelection(); const { anchorNode, focusNode } = selection; From e0b5a9ad53e3c8f5cfc574d7d01d66649264c126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Feb 2022 19:11:14 +0200 Subject: [PATCH 09/67] Prevent default behaviour for everything except Backspace and Delete --- .../src/components/block-tools/index.js | 59 ++++++++----------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 2b25b94d37229..ffe4cf511e815 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -68,44 +68,30 @@ export default function BlockTools( { } = useDispatch( blockEditorStore ); function onKeyDown( event ) { + const clientIds = getSelectedBlockClientIds(); + + if ( ! clientIds.length ) return; + if ( isMatch( 'core/block-editor/move-up', event ) ) { - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length ) { - event.preventDefault(); - const rootClientId = getBlockRootClientId( first( clientIds ) ); - moveBlocksUp( clientIds, rootClientId ); - } + event.preventDefault(); + const rootClientId = getBlockRootClientId( first( clientIds ) ); + moveBlocksUp( clientIds, rootClientId ); } else if ( isMatch( 'core/block-editor/move-down', event ) ) { - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length ) { - event.preventDefault(); - const rootClientId = getBlockRootClientId( first( clientIds ) ); - moveBlocksDown( clientIds, rootClientId ); - } + event.preventDefault(); + const rootClientId = getBlockRootClientId( first( clientIds ) ); + moveBlocksDown( clientIds, rootClientId ); } else if ( isMatch( 'core/block-editor/duplicate', event ) ) { - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length ) { - event.preventDefault(); - duplicateBlocks( clientIds ); - } + event.preventDefault(); + duplicateBlocks( clientIds ); } else if ( isMatch( 'core/block-editor/remove', event ) ) { - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length ) { - event.preventDefault(); - removeBlocks( clientIds ); - } + event.preventDefault(); + removeBlocks( clientIds ); } else if ( isMatch( 'core/block-editor/insert-after', event ) ) { - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length ) { - event.preventDefault(); - insertAfterBlock( last( clientIds ) ); - } + event.preventDefault(); + insertAfterBlock( last( clientIds ) ); } else if ( isMatch( 'core/block-editor/insert-before', event ) ) { - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length ) { - event.preventDefault(); - insertBeforeBlock( first( clientIds ) ); - } + event.preventDefault(); + insertBeforeBlock( first( clientIds ) ); } else if ( isMatch( 'core/block-editor/delete-multi-selection', event ) ) { @@ -121,8 +107,6 @@ export default function BlockTools( { return; } - const clientIds = getSelectedBlockClientIds(); - if ( clientIds.length < 2 ) { return; } @@ -154,7 +138,6 @@ export default function BlockTools( { removeBlocks( clientIds ); } else if ( isMatch( 'core/block-editor/unselect', event ) ) { - const clientIds = getSelectedBlockClientIds(); if ( clientIds.length > 1 ) { event.preventDefault(); clearSelectedBlock(); @@ -163,6 +146,12 @@ export default function BlockTools( { .removeAllRanges(); } } + + if ( clientIds.length === 1 ) { + return; + } + + event.preventDefault(); } return ( From 2eabf1954a3949cf9b1366e3ceaffad840f26f23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Feb 2022 20:25:25 +0200 Subject: [PATCH 10/67] Polish --- .../data/data-core-block-editor.md | 5 +- .../src/components/block-tools/index.js | 8 +- packages/block-editor/src/store/actions.js | 258 +++++++++--------- 3 files changed, 137 insertions(+), 134 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 2e7ecf3ea5cd9..70737f615845e 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1110,6 +1110,10 @@ _Returns_ - `Object`: Action object. +### deleteSelection + +Delete the current selection. + ### duplicateBlocks Action that duplicates a list of blocks. @@ -1218,7 +1222,6 @@ _Parameters_ - _firstBlockClientId_ `string`: Client ID of the first block to merge. - _secondBlockClientId_ `string`: Client ID of the second block to merge. -- _toRemove_ `Array?`: Array of block client IDs to remove. ### moveBlocksDown diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index ffe4cf511e815..185249c95247c 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -64,7 +64,7 @@ export default function BlockTools( { clearSelectedBlock, moveBlocksUp, moveBlocksDown, - mergeBlocks, + deleteSelection, } = useDispatch( blockEditorStore ); function onKeyDown( event ) { @@ -128,11 +128,7 @@ export default function BlockTools( { blockA !== blockB && selectionStart.attributeKey ) { - mergeBlocks( - first( clientIds ), - last( clientIds ), - clientIds.slice( 1, -1 ) - ); + deleteSelection(); return; } diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 085636c1248cc..2240e57701caa 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -672,157 +672,160 @@ function mapRichTextSettings( attributeDefinition ) { } /** - * Action that merges two blocks. - * - * @param {string} firstBlockClientId Client ID of the first block to merge. - * @param {string} secondBlockClientId Client ID of the second block to merge. - * @param {Array?} toRemove Array of block client IDs to remove. + * Delete the current selection. */ -export const mergeBlocks = ( - firstBlockClientId, - secondBlockClientId, - toRemove -) => ( { registry, select, dispatch } ) => { - const blocks = [ firstBlockClientId, secondBlockClientId ]; - const [ clientIdA, clientIdB ] = blocks; - +export const deleteSelection = () => ( { registry, select, dispatch } ) => { const selectionStart = select.getSelectionStart(); const selectionEnd = select.getSelectionEnd(); - // To do: merge logic with existing mergeBlocks logic. - if ( selectionStart.clientId !== selectionEnd.clientId ) { - if ( ! selectionStart.attributeKey || ! selectionEnd.attributeKey ) { - return; - } + // Abort if the selection is contained in one block. + if ( selectionStart.clientId === selectionEnd.clientId ) return; - let selectionA, selectionB; + // Abort if the blocks are selected as a whole. + if ( ! selectionStart.attributeKey || ! selectionEnd.attributeKey ) return; - if ( selectionStart.clientId === clientIdA ) { - selectionA = selectionStart; - } else if ( selectionStart.clientId === clientIdB ) { - selectionB = selectionStart; - } + const selectedBlockClientIds = select.getSelectedBlockClientIds(); + const clientIdA = first( selectedBlockClientIds ); + const clientIdB = last( selectedBlockClientIds ); + let selectionA, selectionB; - if ( selectionEnd.clientId === clientIdA ) { - selectionA = selectionEnd; - } else if ( selectionEnd.clientId === clientIdB ) { - selectionB = selectionEnd; - } - // Abort if the selection doesn't match with blocks to merge. - if ( ! selectionA || ! selectionB ) { - return; - } + if ( selectionStart.clientId === clientIdA ) { + selectionA = selectionStart; + } else if ( selectionStart.clientId === clientIdB ) { + selectionB = selectionStart; + } - const blockA = select.getBlock( clientIdA ); - const blockAType = getBlockType( blockA.name ); - const blockB = select.getBlock( clientIdB ); - const blockBType = getBlockType( blockB.name ); + if ( selectionEnd.clientId === clientIdA ) { + selectionA = selectionEnd; + } else if ( selectionEnd.clientId === clientIdB ) { + selectionB = selectionEnd; + } - // A robust way to retain selection position through various transforms - // is to insert a special character at the position and then recover it. - const START_OF_SELECTED_AREA = '\u0086'; + // Abort if the selection doesn't match with blocks to merge. + if ( ! selectionA || ! selectionB ) { + return; + } - // Clone the blocks so we don't insert the character in a "live" block. - const cloneA = cloneBlock( blockA ); - const cloneB = cloneBlock( blockB ); + const blockA = select.getBlock( clientIdA ); + const blockAType = getBlockType( blockA.name ); + const blockB = select.getBlock( clientIdB ); + const blockBType = getBlockType( blockB.name ); - const attributeDefinitionA = - blockAType.attributes[ selectionA.attributeKey ]; - const attributeDefinitionB = - blockBType.attributes[ selectionB.attributeKey ]; + // A robust way to retain selection position through various transforms + // is to insert a special character at the position and then recover it. + const START_OF_SELECTED_AREA = '\u0086'; - const htmlA = cloneA.attributes[ selectionA.attributeKey ]; - const htmlB = cloneB.attributes[ selectionB.attributeKey ]; + // Clone the blocks so we don't insert the character in a "live" block. + const cloneA = cloneBlock( blockA ); + const cloneB = cloneBlock( blockB ); - let valueA = create( { - html: htmlA, - ...mapRichTextSettings( attributeDefinitionA ), - } ); - let valueB = create( { - html: htmlB, - ...mapRichTextSettings( attributeDefinitionB ), - } ); + const attributeDefinitionA = + blockAType.attributes[ selectionA.attributeKey ]; + const attributeDefinitionB = + blockBType.attributes[ selectionB.attributeKey ]; - valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); - valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); + const htmlA = cloneA.attributes[ selectionA.attributeKey ]; + const htmlB = cloneB.attributes[ selectionB.attributeKey ]; - cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { - value: valueA, - ...mapRichTextSettings( attributeDefinitionA ), - } ); - cloneB.attributes[ selectionB.attributeKey ] = toHTMLString( { - value: valueB, - ...mapRichTextSettings( attributeDefinitionB ), - } ); + let valueA = create( { + html: htmlA, + ...mapRichTextSettings( attributeDefinitionA ), + } ); + let valueB = create( { + html: htmlB, + ...mapRichTextSettings( attributeDefinitionB ), + } ); - // We can only merge blocks with similar types - // thus, we transform the block to merge first - const blocksWithTheSameType = - blockA.name === blockB.name - ? [ cloneB ] - : switchToBlockType( cloneB, blockA.name ); + valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); + valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); - // If the block types can not match, do nothing - if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { - return; - } + cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { + value: valueA, + ...mapRichTextSettings( attributeDefinitionA ), + } ); + cloneB.attributes[ selectionB.attributeKey ] = toHTMLString( { + value: valueB, + ...mapRichTextSettings( attributeDefinitionB ), + } ); - // Calling the merge to update the attributes and remove the block to be merged - const updatedAttributes = blockAType.merge( - cloneA.attributes, - blocksWithTheSameType[ 0 ].attributes - ); + // We can only merge blocks with similar types + // thus, we transform the block to merge first + const blocksWithTheSameType = + blockA.name === blockB.name + ? [ cloneB ] + : switchToBlockType( cloneB, blockA.name ); - const newAttributeKey = findKey( - updatedAttributes, - ( v ) => - typeof v === 'string' && - v.indexOf( START_OF_SELECTED_AREA ) !== -1 - ); - const convertedHtml = updatedAttributes[ newAttributeKey ]; - const convertedValue = create( { - html: convertedHtml, - ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), - } ); - const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); - const newValue = remove( convertedValue, newOffset, newOffset + 1 ); - const newHtml = toHTMLString( { - value: newValue, - ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), - } ); + // If the block types can not match, do nothing + if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { + return; + } - updatedAttributes[ newAttributeKey ] = newHtml; + // Calling the merge to update the attributes and remove the block to be merged + const updatedAttributes = blockAType.merge( + cloneA.attributes, + blocksWithTheSameType[ 0 ].attributes + ); - registry.batch( () => { - dispatch.selectionChange( - blockA.clientId, - newAttributeKey, - newOffset, - newOffset - ); + const newAttributeKey = findKey( + updatedAttributes, + ( v ) => + typeof v === 'string' && v.indexOf( START_OF_SELECTED_AREA ) !== -1 + ); + const convertedHtml = updatedAttributes[ newAttributeKey ]; + const convertedValue = create( { + html: convertedHtml, + ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), + } ); + const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); + const newValue = remove( convertedValue, newOffset, newOffset + 1 ); + const newHtml = toHTMLString( { + value: newValue, + ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), + } ); + + updatedAttributes[ newAttributeKey ] = newHtml; + + registry.batch( () => { + dispatch.selectionChange( + blockA.clientId, + newAttributeKey, + newOffset, + newOffset + ); - if ( toRemove && toRemove.length ) - dispatch.removeBlocks( toRemove ); - - dispatch.replaceBlocks( - [ blockA.clientId, blockB.clientId ], - [ - { - ...blockA, - attributes: { - ...blockA.attributes, - ...updatedAttributes, - }, + if ( selectedBlockClientIds.length > 2 ) + dispatch.removeBlocks( selectedBlockClientIds.slice( 1, -1 ) ); + + dispatch.replaceBlocks( + [ blockA.clientId, blockB.clientId ], + [ + { + ...blockA, + attributes: { + ...blockA.attributes, + ...updatedAttributes, }, - ...blocksWithTheSameType.slice( 1 ), - ], - 0, // If we don't pass the `indexToSelect` it will default to the last block. - select.getSelectedBlocksInitialCaretPosition() - ); - } ); + }, + ...blocksWithTheSameType.slice( 1 ), + ], + 0, // If we don't pass the `indexToSelect` it will default to the last block. + select.getSelectedBlocksInitialCaretPosition() + ); + } ); +}; - return; - } +/** + * Action that merges two blocks. + * + * @param {string} firstBlockClientId Client ID of the first block to merge. + * @param {string} secondBlockClientId Client ID of the second block to merge. + */ +export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { + select, + dispatch, +} ) => { + const blocks = [ firstBlockClientId, secondBlockClientId ]; + const [ clientIdA, clientIdB ] = blocks; dispatch( { type: 'MERGE_BLOCKS', blocks } ); @@ -846,6 +849,7 @@ export const mergeBlocks = ( const cloneA = cloneBlock( blockA ); const cloneB = cloneBlock( blockB ); + const selectionStart = select.getSelectionStart(); const { clientId, attributeKey, offset } = selectionStart; const selectedBlockType = clientId === clientIdA ? blockAType : blockBType; const attributeDefinition = selectedBlockType.attributes[ attributeKey ]; From 3f6a30281940cf90e46224bd4dffa4010a41e87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Feb 2022 23:09:30 +0200 Subject: [PATCH 11/67] Fix forward delete --- .../data/data-core-block-editor.md | 4 + .../src/components/block-tools/index.js | 43 +------ packages/block-editor/src/store/actions.js | 119 +++++++++++------- packages/block-editor/src/utils/dom.js | 23 +--- 4 files changed, 87 insertions(+), 102 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 70737f615845e..177ea9a7b04e5 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1114,6 +1114,10 @@ _Returns_ Delete the current selection. +_Parameters_ + +- _isForward_ `boolean`: + ### duplicateBlocks Action that duplicates a list of blocks. diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 185249c95247c..8cb007d0e3f52 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -10,6 +10,7 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; +import { DELETE } from '@wordpress/keycodes'; /** * Internal dependencies @@ -19,17 +20,6 @@ import BlockPopover from './block-popover'; import { store as blockEditorStore } from '../../store'; import BlockContextualToolbar from './block-contextual-toolbar'; import { usePopoverScroll } from './use-popover-scroll'; -import { getBlockNode } from '../../utils/dom'; - -function isSimpleContentEditable( node ) { - return ( - node.getAttribute( 'contenteditable' ) === 'true' && - ( ! node.firstElementChild || - node.ownerDocument.defaultView.getComputedStyle( - node.firstElementChild - ).display === 'inline' ) - ); -} /** * Renders block tools (the block toolbar, select/navigation mode toolbar, the @@ -51,11 +41,9 @@ export default function BlockTools( { [] ); const isMatch = useShortcutEventMatch(); - const { - getSelectedBlockClientIds, - getBlockRootClientId, - getSelectionStart, - } = useSelect( blockEditorStore ); + const { getSelectedBlockClientIds, getBlockRootClientId } = useSelect( + blockEditorStore + ); const { duplicateBlocks, removeBlocks, @@ -111,28 +99,9 @@ export default function BlockTools( { return; } - const { ownerDocument } = event.target; - const { defaultView } = ownerDocument; - const { anchorNode, focusNode } = defaultView.getSelection(); - const blockA = getBlockNode( anchorNode ); - const blockB = getBlockNode( focusNode ); - + const isForward = event.keyCode === DELETE; + deleteSelection( isForward ); event.preventDefault(); - - const selectionStart = getSelectionStart(); - - // To do: find other way to check if blocks are mergeable. - if ( - isSimpleContentEditable( blockA ) && - isSimpleContentEditable( blockB ) && - blockA !== blockB && - selectionStart.attributeKey - ) { - deleteSelection(); - return; - } - - removeBlocks( clientIds ); } else if ( isMatch( 'core/block-editor/unselect', event ) ) { if ( clientIds.length > 1 ) { event.preventDefault(); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 2240e57701caa..a3cea2faf97b4 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -673,60 +673,82 @@ function mapRichTextSettings( attributeDefinition ) { /** * Delete the current selection. + * + * @param {boolean} isForward */ -export const deleteSelection = () => ( { registry, select, dispatch } ) => { - const selectionStart = select.getSelectionStart(); - const selectionEnd = select.getSelectionEnd(); - - // Abort if the selection is contained in one block. - if ( selectionStart.clientId === selectionEnd.clientId ) return; +export const deleteSelection = ( isForward ) => ( { + registry, + select, + dispatch, +} ) => { + const selectionAnchor = select.getSelectionStart(); + const selectionFocus = select.getSelectionEnd(); - // Abort if the blocks are selected as a whole. - if ( ! selectionStart.attributeKey || ! selectionEnd.attributeKey ) return; + if ( selectionAnchor.clientId === selectionFocus.clientId ) return; - const selectedBlockClientIds = select.getSelectedBlockClientIds(); - const clientIdA = first( selectedBlockClientIds ); - const clientIdB = last( selectedBlockClientIds ); - let selectionA, selectionB; - - if ( selectionStart.clientId === clientIdA ) { - selectionA = selectionStart; - } else if ( selectionStart.clientId === clientIdB ) { - selectionB = selectionStart; + // Remove the blocks if the whole blocks are selected. + if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) { + removeBlocks( select.getSelectedBlockClientIds() ); + return; } - if ( selectionEnd.clientId === clientIdA ) { - selectionA = selectionEnd; - } else if ( selectionEnd.clientId === clientIdB ) { - selectionB = selectionEnd; - } + if ( + typeof selectionAnchor.offset === 'undefined' || + typeof selectionFocus.offset === 'undefined' + ) + return; - // Abort if the selection doesn't match with blocks to merge. - if ( ! selectionA || ! selectionB ) { + const anchorRootClientId = select.getBlockRootClientId( + selectionAnchor.clientId + ); + const focusRootClientId = select.getBlockRootClientId( + selectionFocus.clientId + ); + + // It's not mergeable if the selection doesn't start and end in the same + // block list. Maybe in the future it should be allowed. + if ( anchorRootClientId !== focusRootClientId ) { return; } - const blockA = select.getBlock( clientIdA ); + const blockOrder = select.getBlockOrder( anchorRootClientId ); + const anchorIndex = blockOrder.indexOf( selectionAnchor.clientId ); + const focusIndex = blockOrder.indexOf( selectionFocus.clientId ); + + // Reassign selection start and end based on order. + let selectionStart, selectionEnd; + + if ( anchorIndex > focusIndex ) { + selectionStart = selectionFocus; + selectionEnd = selectionAnchor; + } else { + selectionStart = selectionAnchor; + selectionEnd = selectionFocus; + } + + const selectionA = isForward ? selectionEnd : selectionStart; + const selectionB = isForward ? selectionStart : selectionEnd; + + const blockA = select.getBlock( selectionA.clientId ); const blockAType = getBlockType( blockA.name ); - const blockB = select.getBlock( clientIdB ); - const blockBType = getBlockType( blockB.name ); - // A robust way to retain selection position through various transforms - // is to insert a special character at the position and then recover it. - const START_OF_SELECTED_AREA = '\u0086'; + if ( ! blockAType.merge ) return; + + const blockB = select.getBlock( selectionB.clientId ); + const blockBType = getBlockType( blockB.name ); // Clone the blocks so we don't insert the character in a "live" block. const cloneA = cloneBlock( blockA ); const cloneB = cloneBlock( blockB ); + const htmlA = cloneA.attributes[ selectionA.attributeKey ]; + const htmlB = cloneB.attributes[ selectionB.attributeKey ]; + const attributeDefinitionA = blockAType.attributes[ selectionA.attributeKey ]; const attributeDefinitionB = blockBType.attributes[ selectionB.attributeKey ]; - const htmlA = cloneA.attributes[ selectionA.attributeKey ]; - const htmlB = cloneB.attributes[ selectionB.attributeKey ]; - let valueA = create( { html: htmlA, ...mapRichTextSettings( attributeDefinitionA ), @@ -736,6 +758,10 @@ export const deleteSelection = () => ( { registry, select, dispatch } ) => { ...mapRichTextSettings( attributeDefinitionB ), } ); + // A robust way to retain selection position through various transforms + // is to insert a special character at the position and then recover it. + const START_OF_SELECTED_AREA = '\u0086'; + valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); @@ -785,6 +811,8 @@ export const deleteSelection = () => ( { registry, select, dispatch } ) => { updatedAttributes[ newAttributeKey ] = newHtml; + const selectedBlockClientIds = select.getSelectedBlockClientIds(); + registry.batch( () => { dispatch.selectionChange( blockA.clientId, @@ -793,11 +821,8 @@ export const deleteSelection = () => ( { registry, select, dispatch } ) => { newOffset ); - if ( selectedBlockClientIds.length > 2 ) - dispatch.removeBlocks( selectedBlockClientIds.slice( 1, -1 ) ); - dispatch.replaceBlocks( - [ blockA.clientId, blockB.clientId ], + selectedBlockClientIds, [ { ...blockA, @@ -840,17 +865,7 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { const blockB = select.getBlock( clientIdB ); const blockBType = getBlockType( blockB.name ); - - // A robust way to retain selection position through various transforms - // is to insert a special character at the position and then recover it. - const START_OF_SELECTED_AREA = '\u0086'; - - // Clone the blocks so we don't insert the character in a "live" block. - const cloneA = cloneBlock( blockA ); - const cloneB = cloneBlock( blockB ); - - const selectionStart = select.getSelectionStart(); - const { clientId, attributeKey, offset } = selectionStart; + const { clientId, attributeKey, offset } = select.getSelectionStart(); const selectedBlockType = clientId === clientIdA ? blockAType : blockBType; const attributeDefinition = selectedBlockType.attributes[ attributeKey ]; const canRestoreTextSelection = @@ -875,6 +890,14 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { } } + // A robust way to retain selection position through various transforms + // is to insert a special character at the position and then recover it. + const START_OF_SELECTED_AREA = '\u0086'; + + // Clone the blocks so we don't insert the character in a "live" block. + const cloneA = cloneBlock( blockA ); + const cloneB = cloneBlock( blockB ); + if ( canRestoreTextSelection ) { const selectedBlock = clientId === clientIdA ? cloneA : cloneB; const html = selectedBlock.attributes[ attributeKey ]; diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 8a51fa7469b62..6de80b24dac4b 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -31,35 +31,24 @@ export function isInsideRootBlock( blockElement, element ) { } /** - * Finds the block node given any DOM node inside the block. + * Finds the block client ID given any DOM node inside the block. * * @param {Node?} node DOM node. * - * @return {Element|null} Client ID or undefined if the node is not part of + * @return {string|undefined} Client ID or undefined if the node is not part of * a block. */ -export function getBlockNode( node ) { +export function getBlockClientId( node ) { while ( node && node.nodeType !== node.ELEMENT_NODE ) { node = node.parentNode; } if ( ! node ) { - return null; + return; } - return /** @type {Element} */ ( node ).closest( BLOCK_SELECTOR ); -} - -/** - * Finds the block client ID given any DOM node inside the block. - * - * @param {Node?} node DOM node. - * - * @return {string|undefined} Client ID or undefined if the node is not part of - * a block. - */ -export function getBlockClientId( node ) { - const blockNode = getBlockNode( node ); + const elementNode = /** @type {Element} */ ( node ); + const blockNode = elementNode.closest( BLOCK_SELECTOR ); if ( ! blockNode ) { return; From 05bf622829cb9ef6543a229b9d862edb45cbfee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Feb 2022 00:04:56 +0200 Subject: [PATCH 12/67] Fix typo --- packages/block-editor/src/store/actions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index a3cea2faf97b4..a180567a0d170 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -688,7 +688,7 @@ export const deleteSelection = ( isForward ) => ( { // Remove the blocks if the whole blocks are selected. if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) { - removeBlocks( select.getSelectedBlockClientIds() ); + dispatch.removeBlocks( select.getSelectedBlockClientIds() ); return; } @@ -850,10 +850,10 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { dispatch, } ) => { const blocks = [ firstBlockClientId, secondBlockClientId ]; - const [ clientIdA, clientIdB ] = blocks; dispatch( { type: 'MERGE_BLOCKS', blocks } ); + const [ clientIdA, clientIdB ] = blocks; const blockA = select.getBlock( clientIdA ); const blockAType = getBlockType( blockA.name ); From 19cc8d9a4ba0bc59297b52fb3ad35e4945aa8e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Feb 2022 16:08:20 +0200 Subject: [PATCH 13/67] Avoid DOM checks --- .../data/data-core-block-editor.md | 13 +++ .../use-block-props/use-multi-selection.js | 14 ++++ .../components/keyboard-shortcuts/index.js | 2 +- .../writing-flow/use-multi-selection.js | 31 ++------ packages/block-editor/src/store/selectors.js | 79 +++++++++++++++++++ 5 files changed, 115 insertions(+), 24 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 177ea9a7b04e5..8c39803639539 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1058,6 +1058,19 @@ _Returns_ - `boolean`: True if it should be possible to multi-select blocks, false if multi-selection is disabled. +### isSelectionMergeable + +Check wether the selection is mergeable. + +_Parameters_ + +- _state_ `Object`: Editor state. +- _isForward_ `boolean`: Wether to merge forwards. + +_Returns_ + +- `boolean`: Wether the selection is mergeable. + ### isTyping Returns true if the user is typing, or false otherwise. diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js b/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js index a0271b8e09dec..973111a6f8b2f 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js @@ -32,6 +32,7 @@ export function useMultiSelection( clientId ) { stopMultiSelect, multiSelect, selectBlock, + selectionChange, } = useDispatch( blockEditorStore ); const { isSelectionEnabled, @@ -39,6 +40,7 @@ export function useMultiSelection( clientId ) { getBlockParents, getBlockSelectionStart, hasMultiSelection, + isSelectionMergeable, } = useSelect( blockEditorStore ); return useRefEffect( ( node ) => { @@ -97,8 +99,20 @@ export function useMultiSelection( clientId ) { ]; const depth = Math.min( startPath.length, endPath.length ) - 1; + // Check if selection is already set by rich text. multiSelect( startPath[ depth ], endPath[ depth ] ); + + if ( ! isSelectionMergeable() ) { + selectionChange( { + start: { + clientId: startPath[ depth ], + }, + end: { + clientId: endPath[ depth ], + }, + } ); + } } } diff --git a/packages/block-editor/src/components/keyboard-shortcuts/index.js b/packages/block-editor/src/components/keyboard-shortcuts/index.js index 77a5e7852f0de..ae41f75e473fc 100644 --- a/packages/block-editor/src/components/keyboard-shortcuts/index.js +++ b/packages/block-editor/src/components/keyboard-shortcuts/index.js @@ -61,7 +61,7 @@ function KeyboardShortcutsRegister() { registerShortcut( { name: 'core/block-editor/delete-multi-selection', category: 'block', - description: __( 'Merge or remove multiple selected blocks.' ), + description: __( 'Delete selection.' ), keyCombination: { character: 'del', }, diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index ec4168fe9401f..640075b26ad0a 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -41,16 +41,6 @@ function getDeepestNode( node, type ) { return node; } -function isSimpleContentEditable( node ) { - return ( - node.getAttribute( 'contenteditable' ) === 'true' && - ( ! node.firstElementChild || - node.ownerDocument.defaultView.getComputedStyle( - node.firstElementChild - ).display === 'inline' ) - ); -} - function selector( select ) { const { isMultiSelecting, @@ -58,6 +48,7 @@ function selector( select ) { hasMultiSelection, getSelectedBlockClientId, getSelectedBlocksInitialCaretPosition, + getSelectionStart, } = select( blockEditorStore ); return { @@ -66,6 +57,7 @@ function selector( select ) { hasMultiSelection: hasMultiSelection(), selectedBlockClientId: getSelectedBlockClientId(), initialPosition: getSelectedBlocksInitialCaretPosition(), + isFullSelection: ! getSelectionStart().attributeKey, }; } @@ -76,6 +68,7 @@ export default function useMultiSelection() { multiSelectedBlockClientIds, hasMultiSelection, selectedBlockClientId, + isFullSelection, } = useSelect( selector, [] ); const selectedRef = useBlockRef( selectedBlockClientId ); // These must be in the right DOM order. @@ -130,6 +123,10 @@ export default function useMultiSelection() { return; } + if ( ! isFullSelection ) { + return; + } + // Allow cross contentEditable selection by temporarily making // all content editable. We can't rely on using the store and // React because re-rending happens too slowly. We need to be @@ -146,19 +143,6 @@ export default function useMultiSelection() { return; } - const { anchorNode, focusNode } = defaultView.getSelection(); - - if ( - isSimpleContentEditable( startRef.current ) && - ( startRef.current.contains( anchorNode ) || - startRef.current.contains( focusNode ) ) && - isSimpleContentEditable( endRef.current ) && - ( endRef.current.contains( anchorNode ) || - endRef.current.contains( focusNode ) ) - ) { - return; - } - const selection = defaultView.getSelection(); const range = ownerDocument.createRange(); @@ -180,6 +164,7 @@ export default function useMultiSelection() { multiSelectedBlockClientIds, selectedBlockClientId, initialPosition, + isFullSelection, ] ); } diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 7ded791161943..acdea46c5b2c6 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -28,6 +28,7 @@ import { hasBlockSupport, getPossibleBlockTransformations, parse, + switchToBlockType, } from '@wordpress/blocks'; import { Platform } from '@wordpress/element'; import { applyFilters } from '@wordpress/hooks'; @@ -895,6 +896,84 @@ export function getMultiSelectedBlocksEndClientId( state ) { return selectionEnd.clientId || null; } +/** + * Check wether the selection is mergeable. + * + * @param {Object} state Editor state. + * @param {boolean} isForward Wether to merge forwards. + * + * @return {boolean} Wether the selection is mergeable. + */ +export function isSelectionMergeable( state, isForward ) { + const selectionAnchor = getSelectionStart( state ); + const selectionFocus = getSelectionEnd( state ); + + // It's not mergeable if the start and end are within the same block. + if ( selectionAnchor.clientId === selectionFocus.clientId ) return; + + // It's not mergeable if there's no rich text selection. + if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) + return; + if ( + typeof selectionAnchor.offset === 'undefined' || + typeof selectionFocus.offset === 'undefined' + ) + return; + + const anchorRootClientId = getBlockRootClientId( + state, + selectionAnchor.clientId + ); + const focusRootClientId = getBlockRootClientId( + state, + selectionFocus.clientId + ); + + // It's not mergeable if the selection doesn't start and end in the same + // block list. Maybe in the future it should be allowed. + if ( anchorRootClientId !== focusRootClientId ) { + return; + } + + const blockOrder = getBlockOrder( state, anchorRootClientId ); + const anchorIndex = blockOrder.indexOf( selectionAnchor.clientId ); + const focusIndex = blockOrder.indexOf( selectionFocus.clientId ); + + // Reassign selection start and end based on order. + let selectionStart, selectionEnd; + + if ( anchorIndex > focusIndex ) { + selectionStart = selectionFocus; + selectionEnd = selectionAnchor; + } else { + selectionStart = selectionAnchor; + selectionEnd = selectionFocus; + } + + const targetBlockClientId = isForward + ? selectionEnd.clientId + : selectionStart.clientId; + const blockToMergeClientId = isForward + ? selectionStart.clientId + : selectionEnd.clientId; + + const targetBlock = getBlock( state, targetBlockClientId ); + const targetBlockType = getBlockType( targetBlock.name ); + + if ( ! targetBlockType.merge ) return; + + const blockToMerge = getBlock( state, blockToMergeClientId ); + + // It's mergeable if the blocks are of the same type. + if ( blockToMerge.name === targetBlock.name ) return true; + + // If the blocks are of a different type, try to transform the block being + // merged into the same type of block. + const blocksToMerge = switchToBlockType( blockToMerge, targetBlock.name ); + + return blocksToMerge && blocksToMerge.length; +} + /** * Returns an array containing all block client IDs in the editor in the order * they appear. Optionally accepts a root client ID of the block list for which From 55166572019702dbbeb5be49bcba3f281f16aca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Feb 2022 16:14:04 +0200 Subject: [PATCH 14/67] Fix unit test --- packages/block-editor/src/store/reducer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 1a388d117d860..8691d04cef79b 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -1307,8 +1307,8 @@ export function selection( state = {}, action ) { const { start, end } = action; if ( - start === state.selectionStart.clientId && - end === state.selectionEnd.clientId + start === state.selectionStart?.clientId && + end === state.selectionEnd?.clientId ) { return state; } From fb8db5a8df822e1be4a47e744d5eb601f096f86f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Feb 2022 17:58:33 +0200 Subject: [PATCH 15/67] Allow partial selection with shift + arrow keys --- .../block-editor/src/components/block-tools/index.js | 11 ++++++++++- .../src/components/writing-flow/use-arrow-nav.js | 8 +++----- .../src/component/use-input-and-selection.js | 9 +++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 8cb007d0e3f52..34e54e388dea7 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -10,7 +10,7 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; -import { DELETE } from '@wordpress/keycodes'; +import { DELETE, LEFT, RIGHT, UP, DOWN } from '@wordpress/keycodes'; /** * Internal dependencies @@ -116,6 +116,15 @@ export default function BlockTools( { return; } + if ( + event.keyCode === UP || + event.keyCode === DOWN || + event.keyCode === LEFT || + event.keyCode === RIGHT + ) { + return; + } + event.preventDefault(); } diff --git a/packages/block-editor/src/components/writing-flow/use-arrow-nav.js b/packages/block-editor/src/components/writing-flow/use-arrow-nav.js index 2bf957722f5e0..b3e462bb7eeaf 100644 --- a/packages/block-editor/src/components/writing-flow/use-arrow-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-arrow-nav.js @@ -128,6 +128,7 @@ export default function useArrowNav() { getLastMultiSelectedBlockClientId, getSettings, hasMultiSelection, + getSelectionStart, } = useSelect( blockEditorStore ); const { multiSelect, selectBlock } = useDispatch( blockEditorStore ); return useRefEffect( ( node ) => { @@ -218,7 +219,7 @@ export default function useArrowNav() { const { defaultView } = ownerDocument; if ( hasMultiSelection() ) { - if ( isNav ) { + if ( isNav && ! getSelectionStart().attributeKey ) { const action = isShift ? expandSelection : moveSelection; action( isReverse ); event.preventDefault(); @@ -278,10 +279,7 @@ export default function useArrowNav() { isTabbableEdge( target, isReverse ) && isNavEdge( target, isReverse ) ) { - // Shift key is down, and there is multi selection or we're - // at the end of the current block. - expandSelection( isReverse ); - event.preventDefault(); + node.contentEditable = true; } } else if ( isVertical && 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 adb5f9258b171..7ec56b2c47a9c 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -145,6 +145,15 @@ export function useInputAndSelection( props ) { 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 ) { From f0047f3fd737d1edf0540edee5a3726cb26604aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Feb 2022 18:48:02 +0200 Subject: [PATCH 16/67] Fix e2e tests --- .../src/components/block-tools/index.js | 53 +++++++++---------- .../__snapshots__/block-deletion.test.js.snap | 32 ++++++----- .../various/multi-block-selection.test.js | 4 +- .../src/component/use-input-and-selection.js | 1 + 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 34e54e388dea7..8853aef8178f2 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -10,7 +10,7 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; -import { DELETE, LEFT, RIGHT, UP, DOWN } from '@wordpress/keycodes'; +import { DELETE, LEFT, RIGHT, UP, DOWN, TAB } from '@wordpress/keycodes'; /** * Internal dependencies @@ -80,47 +80,42 @@ export default function BlockTools( { } else if ( isMatch( 'core/block-editor/insert-before', event ) ) { event.preventDefault(); insertBeforeBlock( first( clientIds ) ); - } else if ( - isMatch( 'core/block-editor/delete-multi-selection', event ) + } + + /** + * Check if the target element is a text area, input or + * event.defaultPrevented and return early. In all these + * cases backspace could be handled elsewhere. + */ + if ( + [ 'INPUT', 'TEXTAREA' ].includes( event.target.nodeName ) || + event.defaultPrevented ) { - /** - * Check if the target element is a text area, input or - * event.defaultPrevented and return early. In all these - * cases backspace could be handled elsewhere. - */ - if ( - [ 'INPUT', 'TEXTAREA' ].includes( event.target.nodeName ) || - event.defaultPrevented - ) { - return; - } + return; + } - if ( clientIds.length < 2 ) { - return; - } + if ( clientIds.length === 1 ) { + return; + } + if ( isMatch( 'core/block-editor/delete-multi-selection', event ) ) { const isForward = event.keyCode === DELETE; deleteSelection( isForward ); event.preventDefault(); } else if ( isMatch( 'core/block-editor/unselect', event ) ) { - if ( clientIds.length > 1 ) { - event.preventDefault(); - clearSelectedBlock(); - event.target.ownerDocument.defaultView - .getSelection() - .removeAllRanges(); - } - } - - if ( clientIds.length === 1 ) { - return; + event.preventDefault(); + clearSelectedBlock(); + event.target.ownerDocument.defaultView + .getSelection() + .removeAllRanges(); } if ( event.keyCode === UP || event.keyCode === DOWN || event.keyCode === LEFT || - event.keyCode === RIGHT + event.keyCode === RIGHT || + event.keyCode === TAB ) { return; } diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-deletion.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-deletion.test.js.snap index 0eadd8df91767..a433a325d2a92 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-deletion.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-deletion.test.js.snap @@ -1,26 +1,34 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`block deletion - deleting the third block using backspace in an empty block results in two remaining blocks and positions the caret at the end of the second block 1`] = ` +exports[`block deletion - deleting the third and fourth blocks using backspace with multi-block selection results in two remaining blocks and positions the caret at the end of the second block 1`] = ` "

First paragraph

Second paragraph

+ + + +

" `; -exports[`block deletion - deleting the third block using backspace in an empty block results in two remaining blocks and positions the caret at the end of the second block 2`] = ` +exports[`block deletion - deleting the third and fourth blocks using backspace with multi-block selection results in two remaining blocks and positions the caret at the end of the second block 2`] = ` "

First paragraph

-

Second paragraph - caret was here

-" +

Second paragraph

+ + + +
  • caret was here
+" `; -exports[`block deletion - deleting the third block using backspace with block wrapper selection results in three remaining blocks and positions the caret at the end of the third block 1`] = ` +exports[`block deletion - deleting the third block using backspace in an empty block results in two remaining blocks and positions the caret at the end of the second block 1`] = ` "

First paragraph

@@ -30,7 +38,7 @@ exports[`block deletion - deleting the third block using backspace with block wr " `; -exports[`block deletion - deleting the third block using backspace with block wrapper selection results in three remaining blocks and positions the caret at the end of the third block 2`] = ` +exports[`block deletion - deleting the third block using backspace in an empty block results in two remaining blocks and positions the caret at the end of the second block 2`] = ` "

First paragraph

@@ -40,7 +48,7 @@ exports[`block deletion - deleting the third block using backspace with block wr " `; -exports[`block deletion - deleting the third block using the Remove Block menu item results in two remaining blocks and positions the caret at the end of the second block 1`] = ` +exports[`block deletion - deleting the third block using backspace with block wrapper selection results in three remaining blocks and positions the caret at the end of the third block 1`] = ` "

First paragraph

@@ -50,7 +58,7 @@ exports[`block deletion - deleting the third block using the Remove Block menu i " `; -exports[`block deletion - deleting the third block using the Remove Block menu item results in two remaining blocks and positions the caret at the end of the second block 2`] = ` +exports[`block deletion - deleting the third block using backspace with block wrapper selection results in three remaining blocks and positions the caret at the end of the third block 2`] = ` "

First paragraph

@@ -60,7 +68,7 @@ exports[`block deletion - deleting the third block using the Remove Block menu i " `; -exports[`block deletion - deleting the third block using the Remove Block shortcut results in two remaining blocks and positions the caret at the end of the second block 1`] = ` +exports[`block deletion - deleting the third block using the Remove Block menu item results in two remaining blocks and positions the caret at the end of the second block 1`] = ` "

First paragraph

@@ -70,7 +78,7 @@ exports[`block deletion - deleting the third block using the Remove Block shortc " `; -exports[`block deletion - deleting the third block using the Remove Block shortcut results in two remaining blocks and positions the caret at the end of the second block 2`] = ` +exports[`block deletion - deleting the third block using the Remove Block menu item results in two remaining blocks and positions the caret at the end of the second block 2`] = ` "

First paragraph

@@ -80,7 +88,7 @@ exports[`block deletion - deleting the third block using the Remove Block shortc " `; -exports[`block deletion - deleting the third and fourth blocks using backspace with multi-block selection results in two remaining blocks and positions the caret at the end of the second block 1`] = ` +exports[`block deletion - deleting the third block using the Remove Block shortcut results in two remaining blocks and positions the caret at the end of the second block 1`] = ` "

First paragraph

@@ -90,7 +98,7 @@ exports[`block deletion - deleting the third and fourth blocks using backspace w " `; -exports[`block deletion - deleting the third and fourth blocks using backspace with multi-block selection results in two remaining blocks and positions the caret at the end of the second block 2`] = ` +exports[`block deletion - deleting the third block using the Remove Block shortcut results in two remaining blocks and positions the caret at the end of the second block 2`] = ` "

First paragraph

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index b242912d1c1c4..0813cc2810802 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -172,10 +172,10 @@ describe( 'Multi-block selection', () => { await page.keyboard.press( 'Enter' ); await page.keyboard.type( '12' ); await page.keyboard.press( 'ArrowLeft' ); - await pressKeyWithModifier( 'shift', 'ArrowRight' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowUp' ); await testNativeSelection(); - // This delete all blocks. + // This deletes all blocks. await page.keyboard.press( 'Backspace' ); expect( await getEditedPostContent() ).toMatchSnapshot(); 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 7ec56b2c47a9c..50508035b2f08 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -152,6 +152,7 @@ export function useInputAndSelection( props ) { ) { const { start, end } = createRecord(); record.current.activeFormats = EMPTY_ACTIVE_FORMATS; + ownerDocument.activeElement.contentEditable = false; onSelectionChange( start, end ); } else if ( element.contains( anchorNode ) && From 4f5ab5dcc479b63db70cf8b87601073bc3721bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Feb 2022 19:29:17 +0200 Subject: [PATCH 17/67] Further fixes --- .../block-editor/src/components/rich-text/index.js | 12 ++---------- .../__snapshots__/multi-block-selection.test.js.snap | 6 +++++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 0477927e5967a..125848e717b20 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -247,16 +247,8 @@ function RichTextWrapper( changeHandler( __unstableFormats, __unstableText ); } ); }, - selectionStart: - typeof selectionStart === 'number' && - typeof selectionEnd === 'number' - ? selectionStart - : undefined, - selectionEnd: - typeof selectionStart === 'number' && - typeof selectionEnd === 'number' - ? selectionEnd - : undefined, + selectionStart, + selectionEnd, onSelectionChange, placeholder, __unstableIsSelected: isSelected, diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 3ede4a98a20c0..147f25af3a132 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -6,7 +6,11 @@ exports[`Multi-block selection should allow selecting outer edge if there is no " `; -exports[`Multi-block selection should always expand single line selection 1`] = `""`; +exports[`Multi-block selection should always expand single line selection 1`] = ` +" +

2

+" +`; exports[`Multi-block selection should clear selection when clicking next to blocks 1`] = ` " From 150b23f263165c61e0091bafe4fab3a2aa2d0d1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 25 Feb 2022 18:06:30 +0200 Subject: [PATCH 18/67] Fix forward delete --- packages/block-editor/src/store/actions.js | 67 ++++++++++++------- .../multi-block-selection.test.js.snap | 20 ++++++ .../various/multi-block-selection.test.js | 20 ++++++ 3 files changed, 83 insertions(+), 24 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index a180567a0d170..32c56ea258e57 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -726,14 +726,18 @@ export const deleteSelection = ( isForward ) => ( { selectionEnd = selectionFocus; } - const selectionA = isForward ? selectionEnd : selectionStart; - const selectionB = isForward ? selectionStart : selectionEnd; + const targetSelection = isForward ? selectionEnd : selectionStart; + const targetBlock = select.getBlock( targetSelection.clientId ); + const targetBlockType = getBlockType( targetBlock.name ); + + if ( ! targetBlockType.merge ) return; + + const selectionA = selectionStart; + const selectionB = selectionEnd; const blockA = select.getBlock( selectionA.clientId ); const blockAType = getBlockType( blockA.name ); - if ( ! blockAType.merge ) return; - const blockB = select.getBlock( selectionB.clientId ); const blockBType = getBlockType( blockB.name ); @@ -774,48 +778,72 @@ export const deleteSelection = ( isForward ) => ( { ...mapRichTextSettings( attributeDefinitionB ), } ); + const followingBlock = isForward ? cloneA : cloneB; + // We can only merge blocks with similar types // thus, we transform the block to merge first const blocksWithTheSameType = blockA.name === blockB.name - ? [ cloneB ] - : switchToBlockType( cloneB, blockA.name ); + ? [ followingBlock ] + : switchToBlockType( followingBlock, targetBlockType.name ); // If the block types can not match, do nothing if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { return; } - // Calling the merge to update the attributes and remove the block to be merged - const updatedAttributes = blockAType.merge( - cloneA.attributes, - blocksWithTheSameType[ 0 ].attributes - ); + let updatedAttributes; + + if ( isForward ) { + const blockToMerge = blocksWithTheSameType.pop(); + updatedAttributes = targetBlockType.merge( + blockToMerge.attributes, + cloneB.attributes + ); + } else { + const blockToMerge = blocksWithTheSameType.shift(); + updatedAttributes = targetBlockType.merge( + cloneA.attributes, + blockToMerge.attributes + ); + } const newAttributeKey = findKey( updatedAttributes, ( v ) => typeof v === 'string' && v.indexOf( START_OF_SELECTED_AREA ) !== -1 ); + const convertedHtml = updatedAttributes[ newAttributeKey ]; const convertedValue = create( { html: convertedHtml, - ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), + ...mapRichTextSettings( targetBlockType.attributes[ newAttributeKey ] ), } ); const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); const newValue = remove( convertedValue, newOffset, newOffset + 1 ); const newHtml = toHTMLString( { value: newValue, - ...mapRichTextSettings( blockAType.attributes[ newAttributeKey ] ), + ...mapRichTextSettings( targetBlockType.attributes[ newAttributeKey ] ), } ); updatedAttributes[ newAttributeKey ] = newHtml; const selectedBlockClientIds = select.getSelectedBlockClientIds(); + const replacement = [ + ...( isForward ? blocksWithTheSameType : [] ), + { + ...targetBlock, + attributes: { + ...targetBlock.attributes, + ...updatedAttributes, + }, + }, + ...( isForward ? [] : blocksWithTheSameType ), + ]; registry.batch( () => { dispatch.selectionChange( - blockA.clientId, + targetBlock.clientId, newAttributeKey, newOffset, newOffset @@ -823,16 +851,7 @@ export const deleteSelection = ( isForward ) => ( { dispatch.replaceBlocks( selectedBlockClientIds, - [ - { - ...blockA, - attributes: { - ...blockA.attributes, - ...updatedAttributes, - }, - }, - ...blocksWithTheSameType.slice( 1 ), - ], + replacement, 0, // If we don't pass the `indexToSelect` it will default to the last block. select.getSelectedBlocksInitialCaretPosition() ); diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 147f25af3a132..f6693019f5f55 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -74,6 +74,26 @@ exports[`Multi-block selection should cut and paste 2`] = ` " `; +exports[`Multi-block selection should forward delete across blocks 1`] = ` +" +

1[[

+ + + +

+ + + +

]2

+" +`; + +exports[`Multi-block selection should forward delete across blocks 2`] = ` +" +

12

+" +`; + exports[`Multi-block selection should gradually multi-select 1`] = ` "
diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 0813cc2810802..4a570fdd594a8 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -830,4 +830,24 @@ describe( 'Multi-block selection', () => { await page.keyboard.up( 'Shift' ); expect( await getSelectedFlatIndices() ).toEqual( [ 3, 4 ] ); } ); + + it( 'should forward delete across blocks', async () => { + await clickBlockAppender(); + await page.keyboard.type( '1[[' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '## ]2' ); + await page.keyboard.press( 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowUp' ); + await pressKeyWithModifier( 'shift', 'ArrowUp' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + + // Test setup. + expect( await getEditedPostContent() ).toMatchSnapshot(); + + await page.keyboard.press( 'Delete' ); + + // Expect a heading with "12" as its content. + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } ); From 4089e7a2209a1aadea53432986db5ba4f664faca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 25 Feb 2022 18:52:44 +0200 Subject: [PATCH 19/67] Add Enter handling --- .../data/data-core-block-editor.md | 4 + .../src/components/block-tools/index.js | 7 +- packages/block-editor/src/store/actions.js | 96 +++++++++++++++++++ .../multi-block-selection.test.js.snap | 28 ++++++ .../various/multi-block-selection.test.js | 21 ++++ 5 files changed, 155 insertions(+), 1 deletion(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 8c39803639539..7c8d2c3d3c0d7 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1489,6 +1489,10 @@ _Returns_ - `Object`: Action object. +### splitSelection + +Undocumented declaration. + ### startDraggingBlocks Returns an action object used in signalling that the user has begun to drag blocks. diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 8853aef8178f2..677f459bbe2bf 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -10,7 +10,7 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; -import { DELETE, LEFT, RIGHT, UP, DOWN, TAB } from '@wordpress/keycodes'; +import { DELETE, LEFT, RIGHT, UP, DOWN, TAB, ENTER } from '@wordpress/keycodes'; /** * Internal dependencies @@ -53,6 +53,7 @@ export default function BlockTools( { moveBlocksUp, moveBlocksDown, deleteSelection, + splitSelection, } = useDispatch( blockEditorStore ); function onKeyDown( event ) { @@ -120,6 +121,10 @@ export default function BlockTools( { return; } + if ( event.keyCode === ENTER ) { + splitSelection(); + } + event.preventDefault(); } diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 32c56ea258e57..c0224ea21faf4 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -858,6 +858,102 @@ export const deleteSelection = ( isForward ) => ( { } ); }; +export const splitSelection = () => ( { select, dispatch } ) => { + const selectionAnchor = select.getSelectionStart(); + const selectionFocus = select.getSelectionEnd(); + + if ( selectionAnchor.clientId === selectionFocus.clientId ) return; + + // Remove the blocks if the whole blocks are selected. + if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) { + dispatch.removeBlocks( select.getSelectedBlockClientIds() ); + return; + } + + if ( + typeof selectionAnchor.offset === 'undefined' || + typeof selectionFocus.offset === 'undefined' + ) + return; + + const anchorRootClientId = select.getBlockRootClientId( + selectionAnchor.clientId + ); + const focusRootClientId = select.getBlockRootClientId( + selectionFocus.clientId + ); + + // It's not mergeable if the selection doesn't start and end in the same + // block list. Maybe in the future it should be allowed. + if ( anchorRootClientId !== focusRootClientId ) { + return; + } + + const blockOrder = select.getBlockOrder( anchorRootClientId ); + const anchorIndex = blockOrder.indexOf( selectionAnchor.clientId ); + const focusIndex = blockOrder.indexOf( selectionFocus.clientId ); + + // Reassign selection start and end based on order. + let selectionStart, selectionEnd; + + if ( anchorIndex > focusIndex ) { + selectionStart = selectionFocus; + selectionEnd = selectionAnchor; + } else { + selectionStart = selectionAnchor; + selectionEnd = selectionFocus; + } + + const selectionA = selectionStart; + const selectionB = selectionEnd; + + const blockA = select.getBlock( selectionA.clientId ); + const blockAType = getBlockType( blockA.name ); + + const blockB = select.getBlock( selectionB.clientId ); + const blockBType = getBlockType( blockB.name ); + + // Clone the blocks so we don't insert the character in a "live" block. + const cloneA = cloneBlock( blockA ); + const cloneB = cloneBlock( blockB ); + + const htmlA = cloneA.attributes[ selectionA.attributeKey ]; + const htmlB = cloneB.attributes[ selectionB.attributeKey ]; + + const attributeDefinitionA = + blockAType.attributes[ selectionA.attributeKey ]; + const attributeDefinitionB = + blockBType.attributes[ selectionB.attributeKey ]; + + let valueA = create( { + html: htmlA, + ...mapRichTextSettings( attributeDefinitionA ), + } ); + let valueB = create( { + html: htmlB, + ...mapRichTextSettings( attributeDefinitionB ), + } ); + + valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); + valueB = insert( valueB, '', 0, selectionB.offset ); + + cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { + value: valueA, + ...mapRichTextSettings( attributeDefinitionA ), + } ); + cloneB.attributes[ selectionB.attributeKey ] = toHTMLString( { + value: valueB, + ...mapRichTextSettings( attributeDefinitionB ), + } ); + + dispatch.replaceBlocks( + select.getSelectedBlockClientIds(), + [ cloneA, createBlock( 'core/paragraph' ), cloneB ], + 1, // If we don't pass the `indexToSelect` it will default to the last block. + select.getSelectedBlocksInitialCaretPosition() + ); +}; + /** * Action that merges two blocks. * diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index f6693019f5f55..9df521447deb5 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -118,6 +118,34 @@ exports[`Multi-block selection should gradually multi-select 2`] = ` " `; +exports[`Multi-block selection should handle Enter across blocks 1`] = ` +" +

1[[

+ + + +

a

+ + + +

]2

+" +`; + +exports[`Multi-block selection should handle Enter across blocks 2`] = ` +" +

1

+ + + +

+ + + +

2

+" +`; + exports[`Multi-block selection should multi-select from within the list block 1`] = ` "

1

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 4a570fdd594a8..a12c829fa6c65 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -850,4 +850,25 @@ describe( 'Multi-block selection', () => { // Expect a heading with "12" as its content. expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should handle Enter across blocks', async () => { + await clickBlockAppender(); + await page.keyboard.type( '1[[' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( 'a' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '## ]2' ); + await page.keyboard.press( 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowUp' ); + await pressKeyWithModifier( 'shift', 'ArrowUp' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + + // Test setup. + expect( await getEditedPostContent() ).toMatchSnapshot(); + + await page.keyboard.press( 'Enter' ); + + // Expect a heading with "12" as its content. + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } ); From 21a7932eea7a03613c49bb11714f414f93e2d4a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 9 Mar 2022 17:36:15 +0100 Subject: [PATCH 20/67] Remove lines, fix occasional selection failure --- packages/block-editor/src/components/block-list/style.scss | 4 +--- packages/rich-text/src/component/use-input-and-selection.js | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss index 7b16dff3731a3..40575e08e6993 100644 --- a/packages/block-editor/src/components/block-list/style.scss +++ b/packages/block-editor/src/components/block-list/style.scss @@ -27,8 +27,7 @@ &.is-navigate-mode .block-editor-block-list__block.is-selected, &.is-navigate-mode .block-editor-block-list__block.is-hovered, .block-editor-block-list__block.is-highlighted, - .block-editor-block-list__block.is-multi-selected { - + .block-editor-block-list__block.is-highlighted ~ .is-multi-selected { &::after { // Show selection borders around every non-nested block's actual footprint. position: absolute; @@ -70,7 +69,6 @@ outline: $border-width solid transparent; } - .block-editor-block-list__block.is-multi-selected::after, &.is-navigate-mode .block-editor-block-list__block.is-selected::after, & .is-block-moving-mode.block-editor-block-list__block.has-child-selected { box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color); 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 50508035b2f08..fdb3edebeb7db 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -152,7 +152,8 @@ export function useInputAndSelection( props ) { ) { const { start, end } = createRecord(); record.current.activeFormats = EMPTY_ACTIVE_FORMATS; - ownerDocument.activeElement.contentEditable = false; + // Need to find alternative solution for this. + // ownerDocument.activeElement.contentEditable = false; onSelectionChange( start, end ); } else if ( element.contains( anchorNode ) && From 9485788192a01342e4f6b1d75741afb68331cda8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 10 Mar 2022 16:02:01 +0200 Subject: [PATCH 21/67] Stabilise new e2e tests --- .../multi-block-selection.test.js.snap | 8 +++---- .../various/multi-block-selection.test.js | 23 +++++++++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 9df521447deb5..0d84c471beabd 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -76,11 +76,11 @@ exports[`Multi-block selection should cut and paste 2`] = ` exports[`Multi-block selection should forward delete across blocks 1`] = ` " -

1[[

+

1[

-

+

.

@@ -120,11 +120,11 @@ exports[`Multi-block selection should gradually multi-select 2`] = ` exports[`Multi-block selection should handle Enter across blocks 1`] = ` " -

1[[

+

1[

-

a

+

.

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index a12c829fa6c65..94f9d8b403bfd 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -833,13 +833,18 @@ describe( 'Multi-block selection', () => { it( 'should forward delete across blocks', async () => { await clickBlockAppender(); - await page.keyboard.type( '1[[' ); + await page.keyboard.type( '1[' ); await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '.' ); await page.keyboard.press( 'Enter' ); + // "## " creates h2. await page.keyboard.type( '## ]2' ); await page.keyboard.press( 'ArrowLeft' ); - await pressKeyWithModifier( 'shift', 'ArrowUp' ); - await pressKeyWithModifier( 'shift', 'ArrowUp' ); + // Select everything between []. + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); // Test setup. @@ -853,14 +858,18 @@ describe( 'Multi-block selection', () => { it( 'should handle Enter across blocks', async () => { await clickBlockAppender(); - await page.keyboard.type( '1[[' ); + await page.keyboard.type( '1[' ); await page.keyboard.press( 'Enter' ); - await page.keyboard.type( 'a' ); + await page.keyboard.type( '.' ); await page.keyboard.press( 'Enter' ); + // "## " creates h2. await page.keyboard.type( '## ]2' ); await page.keyboard.press( 'ArrowLeft' ); - await pressKeyWithModifier( 'shift', 'ArrowUp' ); - await pressKeyWithModifier( 'shift', 'ArrowUp' ); + // Select everything between []. + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); // Test setup. From b63e96d7f35dc312356b073e1f50cd60b41cee86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 10 Mar 2022 16:47:12 +0200 Subject: [PATCH 22/67] Allow input over selection --- .../src/components/block-tools/index.js | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 677f459bbe2bf..9fb56c298de70 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -10,7 +10,7 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; -import { DELETE, LEFT, RIGHT, UP, DOWN, TAB, ENTER } from '@wordpress/keycodes'; +import { DELETE, ENTER, BACKSPACE } from '@wordpress/keycodes'; /** * Internal dependencies @@ -99,11 +99,7 @@ export default function BlockTools( { return; } - if ( isMatch( 'core/block-editor/delete-multi-selection', event ) ) { - const isForward = event.keyCode === DELETE; - deleteSelection( isForward ); - event.preventDefault(); - } else if ( isMatch( 'core/block-editor/unselect', event ) ) { + if ( isMatch( 'core/block-editor/unselect', event ) ) { event.preventDefault(); clearSelectedBlock(); event.target.ownerDocument.defaultView @@ -111,21 +107,23 @@ export default function BlockTools( { .removeAllRanges(); } - if ( - event.keyCode === UP || - event.keyCode === DOWN || - event.keyCode === LEFT || - event.keyCode === RIGHT || - event.keyCode === TAB - ) { - return; - } - if ( event.keyCode === ENTER ) { splitSelection(); + event.preventDefault(); + } else if ( event.keyCode === DELETE ) { + deleteSelection( true ); + event.preventDefault(); + } else if ( event.keyCode === BACKSPACE ) { + deleteSelection(); + event.preventDefault(); + } else if ( + // If key.length is longer than 1, it's a control key that doesn't + // input anything. + event.key.length === 1 && + ! ( event.metaKey || event.ctrlKey ) + ) { + deleteSelection(); } - - event.preventDefault(); } return ( From 243aea3263afa6a015ca121c2b9a8b373ae43326 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 15 Mar 2022 10:12:05 +0200 Subject: [PATCH 23/67] Remove obsolete comment --- packages/rich-text/src/component/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 93f7cef03d2c0..6efbce0357cae 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -214,9 +214,6 @@ export function useRichText( { return; } - // For merging multi selection, focus is on writing flow, so it should - // move to the selection after merge. This will probably break some - // things. if ( ref.current.ownerDocument.activeElement !== ref.current ) ref.current.focus(); applyFromProps(); From 734bc92382e777a63cb363433efb46dce2dc307b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 15 Mar 2022 12:47:57 +0200 Subject: [PATCH 24/67] Remove scroll to multi-selection --- .../block-list/use-block-props/index.js | 3 - .../use-block-props/use-scroll-into-view.js | 73 ------------------- 2 files changed, 76 deletions(-) delete mode 100644 packages/block-editor/src/components/block-list/use-block-props/use-scroll-into-view.js diff --git a/packages/block-editor/src/components/block-list/use-block-props/index.js b/packages/block-editor/src/components/block-list/use-block-props/index.js index d03cb5ecf70a1..e6518cdf60e91 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/index.js +++ b/packages/block-editor/src/components/block-list/use-block-props/index.js @@ -31,7 +31,6 @@ import { useBlockMovingModeClassNames } from './use-block-moving-mode-class-name import { useFocusHandler } from './use-focus-handler'; import { useEventHandlers } from './use-selected-block-event-handlers'; import { useNavModeExit } from './use-nav-mode-exit'; -import { useScrollIntoView } from './use-scroll-into-view'; import { useBlockRefProvider } from './use-block-refs'; import { useMultiSelection } from './use-multi-selection'; import { useIntersectionObserver } from './use-intersection-observer'; @@ -115,8 +114,6 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { const mergedRefs = useMergeRefs( [ props.ref, useFocusFirstElement( clientId ), - // Must happen after focus because we check for focus in the block. - useScrollIntoView( clientId ), useBlockRefProvider( clientId ), useFocusHandler( clientId ), useMultiSelection( clientId ), diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-scroll-into-view.js b/packages/block-editor/src/components/block-list/use-block-props/use-scroll-into-view.js deleted file mode 100644 index bd6aca0c43645..0000000000000 --- a/packages/block-editor/src/components/block-list/use-block-props/use-scroll-into-view.js +++ /dev/null @@ -1,73 +0,0 @@ -/** - * External dependencies - */ -import scrollIntoView from 'dom-scroll-into-view'; - -/** - * WordPress dependencies - */ -/** - * WordPress dependencies - */ -import { useEffect, useRef } from '@wordpress/element'; -import { useSelect } from '@wordpress/data'; -import { getScrollContainer } from '@wordpress/dom'; - -/** - * Internal dependencies - */ -import { store as blockEditorStore } from '../../../store'; - -export function useScrollIntoView( clientId ) { - const ref = useRef(); - const isSelectionEnd = useSelect( - ( select ) => { - const { isBlockSelected, getBlockSelectionEnd } = select( - blockEditorStore - ); - - return ( - isBlockSelected( clientId ) || - getBlockSelectionEnd() === clientId - ); - }, - [ clientId ] - ); - - // Note that we can't use `useRefEffect` here, since an element change does - // not mean we can scroll. `isSelectionEnd` should be the sole dependency, - // while with `useRefEffect`, the element is a dependency as well. - useEffect( () => { - if ( ! isSelectionEnd ) { - return; - } - - const extentNode = ref.current; - - if ( ! extentNode ) { - return; - } - - // If the block is focused, the browser will already have scrolled into - // view if necessary. - if ( extentNode.contains( extentNode.ownerDocument.activeElement ) ) { - return; - } - - const scrollContainer = - getScrollContainer( extentNode ) || - extentNode.ownerDocument.defaultView; - - // If there's no scroll container, it follows that there's no scrollbar - // and thus there's no need to try to scroll into view. - if ( ! scrollContainer ) { - return; - } - - scrollIntoView( extentNode, scrollContainer, { - onlyScrollIfNeeded: true, - } ); - }, [ isSelectionEnd ] ); - - return ref; -} From 77d449f2ef7ca5ee4034d3f8235b0a11d7901138 Mon Sep 17 00:00:00 2001 From: jasmussen Date: Tue, 15 Mar 2022 12:04:35 +0100 Subject: [PATCH 25/67] Polish highlight and block select style. This commit does a few things: * Changes the highlight style to be 1.5px like the selected styles. This is actually an oversight in trunk, that's just fixed here. * Adds non-text blocks to the list of blocks that get a hidden selection style. This makes it clearer when you're selecting between two paragraphs, but across a block in between them. --- packages/block-editor/src/components/block-list/style.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss index 40575e08e6993..73f1579de94d4 100644 --- a/packages/block-editor/src/components/block-list/style.scss +++ b/packages/block-editor/src/components/block-list/style.scss @@ -26,6 +26,7 @@ // When selecting multiple blocks, we provide an additional selection indicator. &.is-navigate-mode .block-editor-block-list__block.is-selected, &.is-navigate-mode .block-editor-block-list__block.is-hovered, + .block-editor-block-list__block.is-multi-selected:not([contenteditable]), .block-editor-block-list__block.is-highlighted, .block-editor-block-list__block.is-highlighted ~ .is-multi-selected { &::after { @@ -40,7 +41,7 @@ right: $border-width; // Everything else. - box-shadow: 0 0 0 $border-width var(--wp-admin-theme-color); + box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color); border-radius: $radius-block-ui - $border-width; // Border is outset, so subtract the width to achieve correct radius. // Windows High Contrast mode will show this outline. @@ -65,7 +66,7 @@ } .block-editor-block-list__block.is-highlighted::after { - box-shadow: 0 0 0 $border-width var(--wp-admin-theme-color); + box-shadow: 0 0 0 var(--wp-admin-border-width-focus) var(--wp-admin-theme-color); outline: $border-width solid transparent; } From d999acc0d900664838c7f33ed637259da759706c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 15 Mar 2022 19:25:53 +0200 Subject: [PATCH 26/67] Fix for Firefox --- .../block-list/use-block-props/use-multi-selection.js | 5 ++++- packages/rich-text/src/component/use-input-and-selection.js | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js b/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js index 973111a6f8b2f..e2c2eaf90272c 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js @@ -17,8 +17,11 @@ import { getBlockClientId } from '../../../utils/dom'; * @param {boolean} value `contentEditable` value (true or false) */ export function setContentEditableWrapper( node, value ) { + const editor = node.parentElement.closest( '[contenteditable]' ); // Since `closest` considers `node` as a candidate, use `parentElement`. - node.parentElement.closest( '[contenteditable]' ).contentEditable = value; + editor.contentEditable = value; + // Firefox doesn't automatically move focus. + if ( value ) editor.focus(); } /** 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 fdb3edebeb7db..7ec56b2c47a9c 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -152,8 +152,6 @@ export function useInputAndSelection( props ) { ) { const { start, end } = createRecord(); record.current.activeFormats = EMPTY_ACTIVE_FORMATS; - // Need to find alternative solution for this. - // ownerDocument.activeElement.contentEditable = false; onSelectionChange( start, end ); } else if ( element.contains( anchorNode ) && From eae561579bf1e34db62cb28a5c2069257f4685a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 15 Mar 2022 19:47:21 +0200 Subject: [PATCH 27/67] Fix keyboard nav for Firefox --- .../block-editor/src/components/writing-flow/use-arrow-nav.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-editor/src/components/writing-flow/use-arrow-nav.js b/packages/block-editor/src/components/writing-flow/use-arrow-nav.js index b3e462bb7eeaf..8764e035bd065 100644 --- a/packages/block-editor/src/components/writing-flow/use-arrow-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-arrow-nav.js @@ -280,6 +280,8 @@ export default function useArrowNav() { isNavEdge( target, isReverse ) ) { node.contentEditable = true; + // Firefox doesn't automatically move focus. + node.focus(); } } else if ( isVertical && From 8292c5a0e910afacfc1cccbe8e24840fea612805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 16 Mar 2022 15:06:40 +0200 Subject: [PATCH 28/67] Use getDefaultBlockName --- packages/block-editor/src/store/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index c0224ea21faf4..c79cf82e44e8e 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -948,7 +948,7 @@ export const splitSelection = () => ( { select, dispatch } ) => { dispatch.replaceBlocks( select.getSelectedBlockClientIds(), - [ cloneA, createBlock( 'core/paragraph' ), cloneB ], + [ cloneA, createBlock( getDefaultBlockName() ), cloneB ], 1, // If we don't pass the `indexToSelect` it will default to the last block. select.getSelectedBlocksInitialCaretPosition() ); From 3240d515e9cc006c8e2d56f202d7978d03f3ed1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 16 Mar 2022 15:10:03 +0200 Subject: [PATCH 29/67] insert '' => remove --- packages/block-editor/src/store/actions.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index c79cf82e44e8e..60793c8bfbfc4 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -766,7 +766,7 @@ export const deleteSelection = ( isForward ) => ( { // is to insert a special character at the position and then recover it. const START_OF_SELECTED_AREA = '\u0086'; - valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); + valueA = remove( valueA, selectionA.offset, valueA.text.length ); valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { @@ -934,8 +934,8 @@ export const splitSelection = () => ( { select, dispatch } ) => { ...mapRichTextSettings( attributeDefinitionB ), } ); - valueA = insert( valueA, '', selectionA.offset, valueA.text.length ); - valueB = insert( valueB, '', 0, selectionB.offset ); + valueA = remove( valueA, selectionA.offset, valueA.text.length ); + valueB = remove( valueB, 0, selectionB.offset ); cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { value: valueA, From 020bdfe0e90792a84102f9dcd3e767b9e3a7e338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 16 Mar 2022 16:31:08 +0200 Subject: [PATCH 30/67] Simplify selection filling --- .../writing-flow/use-multi-selection.js | 35 ++----------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index 640075b26ad0a..c0f03103f9f37 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -15,32 +15,6 @@ import { useSelect } from '@wordpress/data'; import { store as blockEditorStore } from '../../store'; import { __unstableUseBlockRef as useBlockRef } from '../block-list/use-block-props/use-block-refs'; -/** - * Returns for the deepest node at the start or end of a container node. Ignores - * any text nodes that only contain HTML formatting whitespace. - * - * @param {Element} node Container to search. - * @param {string} type 'start' or 'end'. - */ -function getDeepestNode( node, type ) { - const child = type === 'start' ? 'firstChild' : 'lastChild'; - const sibling = type === 'start' ? 'nextSibling' : 'previousSibling'; - - while ( node[ child ] ) { - node = node[ child ]; - - while ( - node.nodeType === node.TEXT_NODE && - /^[ \t\n]*$/.test( node.data ) && - node[ sibling ] - ) { - node = node[ sibling ]; - } - } - - return node; -} - function selector( select ) { const { isMultiSelecting, @@ -147,13 +121,8 @@ export default function useMultiSelection() { const range = ownerDocument.createRange(); // These must be in the right DOM order. - // The most stable way to select the whole block contents is to start - // and end at the deepest points. - const startNode = getDeepestNode( startRef.current, 'start' ); - const endNode = getDeepestNode( endRef.current, 'end' ); - - range.setStartBefore( startNode ); - range.setEndAfter( endNode ); + range.setStartBefore( startRef.current ); + range.setEndAfter( endRef.current ); selection.removeAllRanges(); selection.addRange( range ); From 1f890f3f6982a251b41a8e71c5861080e3224565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 16 Mar 2022 21:38:22 +0200 Subject: [PATCH 31/67] Fix Firefox focus issue --- .../src/component/use-input-and-selection.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 7ec56b2c47a9c..19791c525f932 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -259,6 +259,20 @@ export function useInputAndSelection( props ) { applyRecord, } = propsRef.current; + // This is a little hack to work around focus issues with nested + // editable elements in Firefox. For some reason the editable child + // element sometimes regains focus, while it should not be focusable + // and focus should remain on the editable parent element. + // To do: try to find the cause of the shifting focus. + const parentEditable = element.parentElement.closest( + '[contenteditable="true"]' + ); + + if ( parentEditable ) { + parentEditable.focus(); + return; + } + if ( ! isSelected ) { // We know for certain that on focus, the old selection is invalid. // It will be recalculated on the next mouseup, keyup, or touchend From 78007eca0522dc4c748768a9cfefd4046b3b8b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 16 Mar 2022 23:24:15 +0200 Subject: [PATCH 32/67] Fix e2e tests --- .../specs/editor/various/multi-block-selection.test.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 94f9d8b403bfd..b9cd0e69ba5dc 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -79,14 +79,13 @@ async function testNativeSelection() { const firstElement = elements[ 0 ]; const lastElement = elements[ elements.length - 1 ]; - const { startContainer, endContainer } = selection.getRangeAt( 0 ); - if ( ! firstElement.contains( startContainer ) ) { - throw 'expected selection to start in the first selected block'; + if ( ! selection.containsNode( firstElement, true ) ) { + throw 'expected selection to include in the first selected block'; } - if ( ! lastElement.contains( endContainer ) ) { - throw 'expected selection to end in the last selected block'; + if ( ! selection.containsNode( lastElement, true ) ) { + throw 'expected selection to include in the last selected block'; } } ); } From 5c6c5f395293039eab0ea6047f49a27b967bd9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 16 Mar 2022 23:57:01 +0200 Subject: [PATCH 33/67] Fix selection after merge --- .../src/components/rich-text/index.js | 2 + .../src/components/rich-text/use-focus.js | 39 +++++++++++++++++++ .../multi-block-selection.test.js.snap | 4 +- .../various/multi-block-selection.test.js | 10 ++++- .../src/component/use-input-and-selection.js | 14 ------- 5 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 packages/block-editor/src/components/rich-text/use-focus.js diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 125848e717b20..a088823232d3f 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -45,6 +45,7 @@ import { useFormatTypes } from './use-format-types'; import { useRemoveBrowserShortcuts } from './use-remove-browser-shortcuts'; import { useShortcuts } from './use-shortcuts'; import { useInputEvents } from './use-input-events'; +import { useFocus } from './use-focus'; import FormatEdit from './format-edit'; import { getMultilineTag, getAllowedFormats } from './utils'; @@ -391,6 +392,7 @@ function RichTextWrapper( disableLineBreaks, onSplitAtEnd, } ), + useFocus(), anchorRef, ] ) } contentEditable={ true } diff --git a/packages/block-editor/src/components/rich-text/use-focus.js b/packages/block-editor/src/components/rich-text/use-focus.js new file mode 100644 index 0000000000000..76f3295128bea --- /dev/null +++ b/packages/block-editor/src/components/rich-text/use-focus.js @@ -0,0 +1,39 @@ +/** + * WordPress dependencies + */ +import { useRefEffect } from '@wordpress/compose'; +import { useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; + +export function useFocus() { + const { isMultiSelecting } = useSelect( blockEditorStore ); + return useRefEffect( ( element ) => { + function onFocus() { + if ( ! isMultiSelecting() ) { + return; + } + + // This is a little hack to work around focus issues with nested + // editable elements in Firefox. For some reason the editable child + // element sometimes regains focus, while it should not be focusable + // and focus should remain on the editable parent element. + // To do: try to find the cause of the shifting focus. + const parentEditable = element.parentElement.closest( + '[contenteditable="true"]' + ); + + if ( parentEditable ) { + parentEditable.focus(); + } + } + + element.addEventListener( 'focus', onFocus ); + return () => { + element.removeEventListener( 'focus', onFocus ); + }; + }, [] ); +} diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 0d84c471beabd..3af0356459ef4 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -90,7 +90,7 @@ exports[`Multi-block selection should forward delete across blocks 1`] = ` exports[`Multi-block selection should forward delete across blocks 2`] = ` " -

12

+

1&2

" `; @@ -138,7 +138,7 @@ exports[`Multi-block selection should handle Enter across blocks 2`] = ` -

+

&

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index b9cd0e69ba5dc..883c1d9924d2a 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -851,7 +851,10 @@ describe( 'Multi-block selection', () => { await page.keyboard.press( 'Delete' ); - // Expect a heading with "12" as its content. + // Ensure selection is in the correct place. + await page.keyboard.type( '&' ); + + // Expect a heading with "1&2" as its content. expect( await getEditedPostContent() ).toMatchSnapshot(); } ); @@ -876,7 +879,10 @@ describe( 'Multi-block selection', () => { await page.keyboard.press( 'Enter' ); - // Expect a heading with "12" as its content. + // Ensure selection is in the correct place. + await page.keyboard.type( '&' ); + + // Expect two blocks with "&" in between. expect( await getEditedPostContent() ).toMatchSnapshot(); } ); } ); 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 19791c525f932..7ec56b2c47a9c 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -259,20 +259,6 @@ export function useInputAndSelection( props ) { applyRecord, } = propsRef.current; - // This is a little hack to work around focus issues with nested - // editable elements in Firefox. For some reason the editable child - // element sometimes regains focus, while it should not be focusable - // and focus should remain on the editable parent element. - // To do: try to find the cause of the shifting focus. - const parentEditable = element.parentElement.closest( - '[contenteditable="true"]' - ); - - if ( parentEditable ) { - parentEditable.focus(); - return; - } - if ( ! isSelected ) { // We know for certain that on focus, the old selection is invalid. // It will be recalculated on the next mouseup, keyup, or touchend From 0096310d84a12a29c5c2314939a012220bca1edc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Mar 2022 15:06:15 +0200 Subject: [PATCH 34/67] Move selection tracking to writing flow --- .../block-list/use-block-props/index.js | 2 -- .../src/components/writing-flow/index.js | 2 ++ .../use-selection-observer.js} | 32 +++++++++++-------- 3 files changed, 21 insertions(+), 15 deletions(-) rename packages/block-editor/src/components/{block-list/use-block-props/use-multi-selection.js => writing-flow/use-selection-observer.js} (91%) diff --git a/packages/block-editor/src/components/block-list/use-block-props/index.js b/packages/block-editor/src/components/block-list/use-block-props/index.js index e6518cdf60e91..a2ecc329b1c54 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/index.js +++ b/packages/block-editor/src/components/block-list/use-block-props/index.js @@ -32,7 +32,6 @@ import { useFocusHandler } from './use-focus-handler'; import { useEventHandlers } from './use-selected-block-event-handlers'; import { useNavModeExit } from './use-nav-mode-exit'; import { useBlockRefProvider } from './use-block-refs'; -import { useMultiSelection } from './use-multi-selection'; import { useIntersectionObserver } from './use-intersection-observer'; import { store as blockEditorStore } from '../../../store'; @@ -116,7 +115,6 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { useFocusFirstElement( clientId ), useBlockRefProvider( clientId ), useFocusHandler( clientId ), - useMultiSelection( clientId ), useEventHandlers( clientId ), useNavModeExit( clientId ), useIsHovered(), diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 7f9907b57a41b..703c70f1148ab 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -18,6 +18,7 @@ import useMultiSelection from './use-multi-selection'; import useTabNav from './use-tab-nav'; import useArrowNav from './use-arrow-nav'; import useSelectAll from './use-select-all'; +import useSelectionObserver from './use-selection-observer'; import { store as blockEditorStore } from '../../store'; export function useWritingFlow() { @@ -31,6 +32,7 @@ export function useWritingFlow() { before, useMergeRefs( [ ref, + useSelectionObserver(), useMultiSelection(), useSelectAll(), useArrowNav(), diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-selection-observer.js similarity index 91% rename from packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js rename to packages/block-editor/src/components/writing-flow/use-selection-observer.js index e2c2eaf90272c..04120572bcb1d 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-selection-observer.js @@ -7,8 +7,11 @@ import { useRefEffect } from '@wordpress/compose'; /** * Internal dependencies */ -import { store as blockEditorStore } from '../../../store'; -import { getBlockClientId } from '../../../utils/dom'; +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; +import { getBlockClientId } from '../../utils/dom'; /** * Sets the `contenteditable` wrapper element to `value`. @@ -17,19 +20,16 @@ import { getBlockClientId } from '../../../utils/dom'; * @param {boolean} value `contentEditable` value (true or false) */ export function setContentEditableWrapper( node, value ) { - const editor = node.parentElement.closest( '[contenteditable]' ); // Since `closest` considers `node` as a candidate, use `parentElement`. - editor.contentEditable = value; + node.contentEditable = value; // Firefox doesn't automatically move focus. - if ( value ) editor.focus(); + if ( value ) node.focus(); } /** * Sets a multi-selection based on the native selection across blocks. - * - * @param {string} clientId Block client ID. */ -export function useMultiSelection( clientId ) { +export default function useSelectionObserver() { const { startMultiSelect, stopMultiSelect, @@ -63,6 +63,7 @@ export function useMultiSelection( clientId ) { return; } + const clientId = getBlockClientId( selection.anchorNode ); const endClientId = getBlockClientId( selection.focusNode ); const isSingularSelection = clientId === endClientId; @@ -135,14 +136,18 @@ export function useMultiSelection( clientId ) { } ); } - function onMouseLeave( { buttons } ) { + function onMouseLeave( { buttons, target } ) { // The primary button must be pressed to initiate selection. // See https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons if ( buttons !== 1 ) { return; } - if ( ! isSelectionEnabled() || ! isBlockSelected( clientId ) ) { + if ( ! target.contentEditable ) { + return; + } + + if ( ! isSelectionEnabled() ) { return; } @@ -172,6 +177,8 @@ export function useMultiSelection( clientId ) { return; } + const clientId = getBlockClientId( event.target ); + if ( event.shiftKey ) { const blockSelectionStart = getBlockSelectionStart(); // By checking `blockSelectionStart` to be set, we handle the @@ -217,11 +224,11 @@ export function useMultiSelection( clientId ) { } node.addEventListener( 'mousedown', onMouseDown ); - node.addEventListener( 'mouseleave', onMouseLeave ); + node.addEventListener( 'mouseout', onMouseLeave ); return () => { node.removeEventListener( 'mousedown', onMouseDown ); - node.removeEventListener( 'mouseleave', onMouseLeave ); + node.removeEventListener( 'mouseout', onMouseLeave ); ownerDocument.removeEventListener( 'selectionchange', onSelectionChange @@ -231,7 +238,6 @@ export function useMultiSelection( clientId ) { }; }, [ - clientId, startMultiSelect, stopMultiSelect, multiSelect, From 722b640091c2b21c9e778bcd26b2bba926f45987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Mar 2022 17:26:17 +0200 Subject: [PATCH 35/67] Fix import --- .../block-list/use-block-props/use-focus-first-element.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js index 4fcb8534883dc..8149520684f6f 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js @@ -15,7 +15,6 @@ import { useSelect } from '@wordpress/data'; */ import { isInsideRootBlock } from '../../../utils/dom'; import { store as blockEditorStore } from '../../../store'; -import { setContentEditableWrapper } from './use-multi-selection'; /** @typedef {import('@wordpress/element').RefObject} RefObject */ @@ -130,7 +129,9 @@ export function useFocusFirstElement( clientId ) { } } - setContentEditableWrapper( ref.current, false ); + ref.current.parentElement.closest( + '[contenteditable]' + ).contentEditable = false; placeCaretAtHorizontalEdge( target, isReverse ); }, [ initialPosition, clientId ] ); From 8c66742bb2a2cf9d0f1cb8ab814d0b7263f25d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Mar 2022 23:03:53 +0200 Subject: [PATCH 36/67] Fix e2e test --- .../e2e-tests/specs/editor/various/multi-block-selection.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 883c1d9924d2a..e8612af915612 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -330,7 +330,6 @@ describe( 'Multi-block selection', () => { await page.keyboard.press( 'ArrowUp' ); await page.keyboard.up( 'Shift' ); await transformBlockTo( 'Group' ); - await page.keyboard.press( 'ArrowDown' ); // Click the first paragraph in the first Group block while pressing `shift` key. const firstParagraph = await page.waitForXPath( "//p[text()='first']" ); From 85bdcd55db821cdcc8f177770e1ac00fa638676a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 18 Mar 2022 23:14:45 +0200 Subject: [PATCH 37/67] Separate click to select hook --- .../src/components/writing-flow/index.js | 2 + .../writing-flow/use-click-selection.js | 90 +++++++++++++++++++ .../writing-flow/use-selection-observer.js | 63 +------------ 3 files changed, 95 insertions(+), 60 deletions(-) create mode 100644 packages/block-editor/src/components/writing-flow/use-click-selection.js diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 703c70f1148ab..1ca1c5b64b23e 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -19,6 +19,7 @@ import useTabNav from './use-tab-nav'; import useArrowNav from './use-arrow-nav'; import useSelectAll from './use-select-all'; import useSelectionObserver from './use-selection-observer'; +import useClickSelection from './use-click-selection'; import { store as blockEditorStore } from '../../store'; export function useWritingFlow() { @@ -33,6 +34,7 @@ export function useWritingFlow() { useMergeRefs( [ ref, useSelectionObserver(), + useClickSelection(), useMultiSelection(), useSelectAll(), useArrowNav(), diff --git a/packages/block-editor/src/components/writing-flow/use-click-selection.js b/packages/block-editor/src/components/writing-flow/use-click-selection.js new file mode 100644 index 0000000000000..54ce9bbffe026 --- /dev/null +++ b/packages/block-editor/src/components/writing-flow/use-click-selection.js @@ -0,0 +1,90 @@ +/** + * WordPress dependencies + */ +import { useSelect, useDispatch } from '@wordpress/data'; +import { useRefEffect } from '@wordpress/compose'; + +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; +import { getBlockClientId } from '../../utils/dom'; + +export default function useClickSelection() { + const { multiSelect, selectBlock } = useDispatch( blockEditorStore ); + const { + isSelectionEnabled, + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + } = useSelect( blockEditorStore ); + return useRefEffect( + ( node ) => { + function onMouseDown( event ) { + // The main button. + // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button + if ( ! isSelectionEnabled() || event.button !== 0 ) { + return; + } + + const clientId = getBlockClientId( event.target ); + + if ( event.shiftKey ) { + const blockSelectionStart = getBlockSelectionStart(); + // By checking `blockSelectionStart` to be set, we handle the + // case where we select a single block. We also have to check + // the selectionEnd (clientId) not to be included in the + // `blockSelectionStart`'s parents because the click event is + // propagated. + const startParents = getBlockParents( blockSelectionStart ); + if ( + blockSelectionStart && + blockSelectionStart !== clientId && + ! startParents?.includes( clientId ) + ) { + const startPath = [ + ...startParents, + blockSelectionStart, + ]; + const endPath = [ + ...getBlockParents( clientId ), + clientId, + ]; + const depth = + Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + // Handle the case of having selected a parent block and + // then shift+click on a child. + if ( start !== end ) { + multiSelect( start, end ); + event.preventDefault(); + } + } + } else if ( hasMultiSelection() ) { + // Allow user to escape out of a multi-selection to a + // singular selection of a block via click. This is handled + // here since focus handling excludes blocks when there is + // multiselection, as focus can be incurred by starting a + // multiselection (focus moved to first block's multi- + // controls). + selectBlock( clientId ); + } + } + + node.addEventListener( 'mousedown', onMouseDown ); + + return () => { + node.removeEventListener( 'mousedown', onMouseDown ); + }; + }, + [ + multiSelect, + selectBlock, + isSelectionEnabled, + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + ] + ); +} 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 04120572bcb1d..8a77f9a073276 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 @@ -19,7 +19,7 @@ import { getBlockClientId } from '../../utils/dom'; * @param {HTMLElement} node Block element. * @param {boolean} value `contentEditable` value (true or false) */ -export function setContentEditableWrapper( node, value ) { +function setContentEditableWrapper( node, value ) { // Since `closest` considers `node` as a candidate, use `parentElement`. node.contentEditable = value; // Firefox doesn't automatically move focus. @@ -39,10 +39,7 @@ export default function useSelectionObserver() { } = useDispatch( blockEditorStore ); const { isSelectionEnabled, - isBlockSelected, getBlockParents, - getBlockSelectionStart, - hasMultiSelection, isSelectionMergeable, } = useSelect( blockEditorStore ); return useRefEffect( @@ -170,64 +167,9 @@ export default function useSelectionObserver() { setContentEditableWrapper( node, true ); } - function onMouseDown( event ) { - // The main button. - // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button - if ( ! isSelectionEnabled() || event.button !== 0 ) { - return; - } - - const clientId = getBlockClientId( event.target ); - - if ( event.shiftKey ) { - const blockSelectionStart = getBlockSelectionStart(); - // By checking `blockSelectionStart` to be set, we handle the - // case where we select a single block. We also have to check - // the selectionEnd (clientId) not to be included in the - // `blockSelectionStart`'s parents because the click event is - // propagated. - const startParents = getBlockParents( blockSelectionStart ); - if ( - blockSelectionStart && - blockSelectionStart !== clientId && - ! startParents?.includes( clientId ) - ) { - const startPath = [ - ...startParents, - blockSelectionStart, - ]; - const endPath = [ - ...getBlockParents( clientId ), - clientId, - ]; - const depth = - Math.min( startPath.length, endPath.length ) - 1; - const start = startPath[ depth ]; - const end = endPath[ depth ]; - // Handle the case of having selected a parent block and - // then shift+click on a child. - if ( start !== end ) { - setContentEditableWrapper( node, true ); - multiSelect( start, end ); - event.preventDefault(); - } - } - } else if ( hasMultiSelection() ) { - // Allow user to escape out of a multi-selection to a - // singular selection of a block via click. This is handled - // here since focus handling excludes blocks when there is - // multiselection, as focus can be incurred by starting a - // multiselection (focus moved to first block's multi- - // controls). - selectBlock( clientId ); - } - } - - node.addEventListener( 'mousedown', onMouseDown ); node.addEventListener( 'mouseout', onMouseLeave ); return () => { - node.removeEventListener( 'mousedown', onMouseDown ); node.removeEventListener( 'mouseout', onMouseLeave ); ownerDocument.removeEventListener( 'selectionchange', @@ -242,9 +184,10 @@ export default function useSelectionObserver() { stopMultiSelect, multiSelect, selectBlock, + selectionChange, isSelectionEnabled, - isBlockSelected, getBlockParents, + isSelectionMergeable, ] ); } From b6c9cdd9f683c8e975a2ba0f9e9de91a62b5637c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sat, 19 Mar 2022 17:54:47 +0200 Subject: [PATCH 38/67] Fix keyboard selection with non rich text --- .../src/components/writing-flow/index.js | 6 +- .../use-keyboard-selection-observer.js | 129 ++++++++++++++++++ ...ver.js => use-mouse-selection-observer.js} | 3 - 3 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js rename packages/block-editor/src/components/writing-flow/{use-selection-observer.js => use-mouse-selection-observer.js} (99%) diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 1ca1c5b64b23e..83d361f487e19 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -18,7 +18,8 @@ import useMultiSelection from './use-multi-selection'; import useTabNav from './use-tab-nav'; import useArrowNav from './use-arrow-nav'; import useSelectAll from './use-select-all'; -import useSelectionObserver from './use-selection-observer'; +import useMouseSelectionObserver from './use-mouse-selection-observer'; +import useKeyboardSelectionObserver from './use-keyboard-selection-observer'; import useClickSelection from './use-click-selection'; import { store as blockEditorStore } from '../../store'; @@ -33,7 +34,8 @@ export function useWritingFlow() { before, useMergeRefs( [ ref, - useSelectionObserver(), + useMouseSelectionObserver(), + useKeyboardSelectionObserver(), useClickSelection(), useMultiSelection(), useSelectAll(), diff --git a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js new file mode 100644 index 0000000000000..0d02d665347f8 --- /dev/null +++ b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js @@ -0,0 +1,129 @@ +/** + * WordPress dependencies + */ +import { useSelect, useDispatch } from '@wordpress/data'; +import { useRefEffect } from '@wordpress/compose'; + +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; +import { getBlockClientId } from '../../utils/dom'; + +function extractSelectionStartNode( selection ) { + const { anchorNode, anchorOffset } = selection; + + if ( anchorNode.nodeType === anchorNode.TEXT_NODE ) { + return anchorNode; + } + + return anchorNode.childNodes[ anchorOffset ]; +} + +function extractSelectionEndNode( selection ) { + const { focusNode, focusOffset } = selection; + + if ( focusNode.nodeType === focusNode.TEXT_NODE ) { + return focusNode; + } + + return focusNode.childNodes[ focusOffset - 1 ]; +} + +/** + * Sets the `contenteditable` wrapper element to `value`. + * + * @param {HTMLElement} node Block element. + * @param {boolean} value `contentEditable` value (true or false) + */ +function setContentEditableWrapper( node, value ) { + // Since `closest` considers `node` as a candidate, use `parentElement`. + node.contentEditable = value; + // Firefox doesn't automatically move focus. + if ( value ) node.focus(); +} + +/** + * Sets a multi-selection based on the native selection across blocks. + */ +export default function useKeyboardSelectionObserver() { + const { multiSelect, selectBlock, selectionChange } = useDispatch( + blockEditorStore + ); + const { getBlockParents, isSelectionMergeable } = useSelect( + blockEditorStore + ); + return useRefEffect( + ( node ) => { + const { ownerDocument } = node; + const { defaultView } = ownerDocument; + + function onSelectionChange() { + const selection = defaultView.getSelection(); + + // If no selection is found, end multi selection and disable the + // contentEditable wrapper. + if ( ! selection.rangeCount || selection.isCollapsed ) { + setContentEditableWrapper( node, false ); + return; + } + + const clientId = getBlockClientId( + extractSelectionStartNode( selection ) + ); + const endClientId = getBlockClientId( + extractSelectionEndNode( selection ) + ); + const isSingularSelection = clientId === endClientId; + + if ( isSingularSelection ) { + selectBlock( clientId ); + } else { + const startPath = [ + ...getBlockParents( clientId ), + clientId, + ]; + const endPath = [ + ...getBlockParents( endClientId ), + endClientId, + ]; + const depth = + Math.min( startPath.length, endPath.length ) - 1; + + // Check if selection is already set by rich text. + multiSelect( startPath[ depth ], endPath[ depth ] ); + + if ( ! isSelectionMergeable() ) { + selectionChange( { + start: { + clientId: startPath[ depth ], + }, + end: { + clientId: endPath[ depth ], + }, + } ); + } + } + } + + ownerDocument.addEventListener( + 'selectionchange', + onSelectionChange + ); + + return () => { + ownerDocument.removeEventListener( + 'selectionchange', + onSelectionChange + ); + }; + }, + [ + multiSelect, + selectBlock, + selectionChange, + getBlockParents, + isSelectionMergeable, + ] + ); +} diff --git a/packages/block-editor/src/components/writing-flow/use-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js similarity index 99% rename from packages/block-editor/src/components/writing-flow/use-selection-observer.js rename to packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js index 8a77f9a073276..aec4fde08586e 100644 --- a/packages/block-editor/src/components/writing-flow/use-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js @@ -4,9 +4,6 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useRefEffect } from '@wordpress/compose'; -/** - * Internal dependencies - */ /** * Internal dependencies */ From 6d3babf0a19ee58d9b3081966e79c7cae5a7b077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 20 Mar 2022 16:08:46 +0200 Subject: [PATCH 39/67] Restrict filling in selection if block already has selection --- .../components/writing-flow/use-arrow-nav.js | 51 +------------------ .../writing-flow/use-multi-selection.js | 17 ++++--- 2 files changed, 10 insertions(+), 58 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-arrow-nav.js b/packages/block-editor/src/components/writing-flow/use-arrow-nav.js index 8764e035bd065..d02233650c392 100644 --- a/packages/block-editor/src/components/writing-flow/use-arrow-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-arrow-nav.js @@ -16,7 +16,7 @@ import { isRTL, } from '@wordpress/dom'; import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes'; -import { useSelect, useDispatch } from '@wordpress/data'; +import { useSelect } from '@wordpress/data'; import { useRefEffect } from '@wordpress/compose'; /** @@ -120,17 +120,12 @@ export function getClosestTabbable( export default function useArrowNav() { const { getSelectedBlockClientId, - getMultiSelectedBlocksStartClientId, getMultiSelectedBlocksEndClientId, getPreviousBlockClientId, getNextBlockClientId, - getFirstMultiSelectedBlockClientId, - getLastMultiSelectedBlockClientId, getSettings, hasMultiSelection, - getSelectionStart, } = useSelect( blockEditorStore ); - const { multiSelect, selectBlock } = useDispatch( blockEditorStore ); return useRefEffect( ( node ) => { // Here a DOMRect is stored while moving the caret vertically so // vertical position of the start position can be restored. This is to @@ -141,44 +136,6 @@ export default function useArrowNav() { verticalRect = null; } - function expandSelection( isReverse ) { - const selectedBlockClientId = getSelectedBlockClientId(); - const selectionStartClientId = getMultiSelectedBlocksStartClientId(); - const selectionEndClientId = getMultiSelectedBlocksEndClientId(); - const selectionBeforeEndClientId = getPreviousBlockClientId( - selectionEndClientId || selectedBlockClientId - ); - const selectionAfterEndClientId = getNextBlockClientId( - selectionEndClientId || selectedBlockClientId - ); - const nextSelectionEndClientId = isReverse - ? selectionBeforeEndClientId - : selectionAfterEndClientId; - - if ( nextSelectionEndClientId ) { - if ( selectionStartClientId === nextSelectionEndClientId ) { - selectBlock( nextSelectionEndClientId ); - } else { - multiSelect( - selectionStartClientId || selectedBlockClientId, - nextSelectionEndClientId - ); - } - } - } - - function moveSelection( isReverse ) { - const selectedFirstClientId = getFirstMultiSelectedBlockClientId(); - const selectedLastClientId = getLastMultiSelectedBlockClientId(); - const focusedBlockClientId = isReverse - ? selectedFirstClientId - : selectedLastClientId; - - if ( focusedBlockClientId ) { - selectBlock( focusedBlockClientId ); - } - } - /** * Returns true if the given target field is the last in its block which * can be considered for tab transition. For example, in a block with @@ -219,12 +176,6 @@ export default function useArrowNav() { const { defaultView } = ownerDocument; if ( hasMultiSelection() ) { - if ( isNav && ! getSelectionStart().attributeKey ) { - const action = isShift ? expandSelection : moveSelection; - action( isReverse ); - event.preventDefault(); - } - return; } diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index c0f03103f9f37..5e09d45618ed5 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -22,7 +22,6 @@ function selector( select ) { hasMultiSelection, getSelectedBlockClientId, getSelectedBlocksInitialCaretPosition, - getSelectionStart, } = select( blockEditorStore ); return { @@ -31,7 +30,6 @@ function selector( select ) { hasMultiSelection: hasMultiSelection(), selectedBlockClientId: getSelectedBlockClientId(), initialPosition: getSelectedBlocksInitialCaretPosition(), - isFullSelection: ! getSelectionStart().attributeKey, }; } @@ -42,7 +40,6 @@ export default function useMultiSelection() { multiSelectedBlockClientIds, hasMultiSelection, selectedBlockClientId, - isFullSelection, } = useSelect( selector, [] ); const selectedRef = useBlockRef( selectedBlockClientId ); // These must be in the right DOM order. @@ -97,10 +94,6 @@ export default function useMultiSelection() { return; } - if ( ! isFullSelection ) { - return; - } - // Allow cross contentEditable selection by temporarily making // all content editable. We can't rely on using the store and // React because re-rending happens too slowly. We need to be @@ -118,6 +111,15 @@ export default function useMultiSelection() { } const selection = defaultView.getSelection(); + + // Abort if the blocks already contain selection. + if ( + selection.containsNode( startRef.current, true ) && + selection.containsNode( endRef.current, true ) + ) { + return; + } + const range = ownerDocument.createRange(); // These must be in the right DOM order. @@ -133,7 +135,6 @@ export default function useMultiSelection() { multiSelectedBlockClientIds, selectedBlockClientId, initialPosition, - isFullSelection, ] ); } From 81c5b85f702e633d932cd2e8a134ecccd9456ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 20 Mar 2022 20:13:21 +0200 Subject: [PATCH 40/67] Fix paste e2e test --- .../src/components/block-list-appender/index.js | 1 + .../__snapshots__/multi-block-selection.test.js.snap | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/block-list-appender/index.js b/packages/block-editor/src/components/block-list-appender/index.js index c9d114fef0c7c..cf5eca2f7ba5a 100644 --- a/packages/block-editor/src/components/block-list-appender/index.js +++ b/packages/block-editor/src/components/block-list-appender/index.js @@ -73,6 +73,7 @@ function BlockListAppender( { 'block-list-appender wp-block', className ) } + contentEditable={ false } // The appender exists to let you add the first Paragraph before // any is inserted. To that end, this appender should visually be // presented as a block. That means theme CSS should style it as if diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 3af0356459ef4..06c873d447255 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -178,23 +178,23 @@ exports[`Multi-block selection should only trigger multi-selection when at the e exports[`Multi-block selection should place the caret at the end of last pasted paragraph (paste mid-block) 1`] = ` " -

first paragra

+

first paragraph

-

first paragraph

+

second paragr

-

second paragrap

+

first paragraph

-

ph

+

second paragrap

-

second paragraph

+

aph

" `; From fa1cbf6a39582c2cf1b0415aad2bc3ddeb29f085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 21 Mar 2022 10:03:12 +0200 Subject: [PATCH 41/67] Fix race condition in e2e tests --- .../block-list/use-block-props/use-focus-first-element.js | 2 ++ .../specs/editor/various/copy-cut-paste-whole-blocks.test.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js index 8149520684f6f..523e36ec241ad 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js @@ -129,6 +129,8 @@ export function useFocusFirstElement( clientId ) { } } + // This is needed because this hook is called before the writing flow + // hook has the chance to turn off content editable. ref.current.parentElement.closest( '[contenteditable]' ).contentEditable = false; diff --git a/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js index c141eb9bb9476..fa2feb7bb529b 100644 --- a/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js @@ -100,6 +100,7 @@ describe( 'Copy/cut/paste of whole blocks', () => { await page.click( '.editor-block-list-item-paragraph' ); await page.keyboard.type( 'P' ); await page.keyboard.press( 'ArrowLeft' ); + await page.evaluate( () => new Promise( requestAnimationFrame ) ); await page.keyboard.press( 'ArrowLeft' ); // Cut group. await pressKeyWithModifier( 'primary', 'x' ); @@ -145,6 +146,7 @@ describe( 'Copy/cut/paste of whole blocks', () => { await page.click( '.editor-block-list-item-paragraph' ); await page.keyboard.type( 'P' ); await page.keyboard.press( 'ArrowLeft' ); + await page.evaluate( () => new Promise( requestAnimationFrame ) ); await page.keyboard.press( 'ArrowLeft' ); // Cut group. await pressKeyWithModifier( 'primary', 'x' ); From fd016d016bf4d28f77b8cb7352e6a10deee724bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 21 Mar 2022 10:24:54 +0200 Subject: [PATCH 42/67] Allow partial selection with shift+click --- .../data/data-core-block-editor.md | 2 +- .../components/block-list-appender/index.js | 4 ++ .../writing-flow/use-click-selection.js | 38 ++----------------- .../use-mouse-selection-observer.js | 1 - packages/block-editor/src/store/actions.js | 3 ++ 5 files changed, 12 insertions(+), 36 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 7c8d2c3d3c0d7..f529ca0ee9480 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1491,7 +1491,7 @@ _Returns_ ### splitSelection -Undocumented declaration. +Split the current selection. ### startDraggingBlocks diff --git a/packages/block-editor/src/components/block-list-appender/index.js b/packages/block-editor/src/components/block-list-appender/index.js index cf5eca2f7ba5a..d6cc56d5e2cc9 100644 --- a/packages/block-editor/src/components/block-list-appender/index.js +++ b/packages/block-editor/src/components/block-list-appender/index.js @@ -73,6 +73,10 @@ function BlockListAppender( { 'block-list-appender wp-block', className ) } + // Needed in case the whole editor is content editable (for multi + // selection). It fixes an edge case where ArrowDown and ArrowRight + // should collapse the selection to the end of that selection and + // not into the appender. contentEditable={ false } // The appender exists to let you add the first Paragraph before // any is inserted. To that end, this appender should visually be diff --git a/packages/block-editor/src/components/writing-flow/use-click-selection.js b/packages/block-editor/src/components/writing-flow/use-click-selection.js index 54ce9bbffe026..7705ceb556fe3 100644 --- a/packages/block-editor/src/components/writing-flow/use-click-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-click-selection.js @@ -27,40 +27,10 @@ export default function useClickSelection() { return; } - const clientId = getBlockClientId( event.target ); - if ( event.shiftKey ) { - const blockSelectionStart = getBlockSelectionStart(); - // By checking `blockSelectionStart` to be set, we handle the - // case where we select a single block. We also have to check - // the selectionEnd (clientId) not to be included in the - // `blockSelectionStart`'s parents because the click event is - // propagated. - const startParents = getBlockParents( blockSelectionStart ); - if ( - blockSelectionStart && - blockSelectionStart !== clientId && - ! startParents?.includes( clientId ) - ) { - const startPath = [ - ...startParents, - blockSelectionStart, - ]; - const endPath = [ - ...getBlockParents( clientId ), - clientId, - ]; - const depth = - Math.min( startPath.length, endPath.length ) - 1; - const start = startPath[ depth ]; - const end = endPath[ depth ]; - // Handle the case of having selected a parent block and - // then shift+click on a child. - if ( start !== end ) { - multiSelect( start, end ); - event.preventDefault(); - } - } + node.contentEditable = true; + // Firefox doesn't automatically move focus. + node.focus(); } else if ( hasMultiSelection() ) { // Allow user to escape out of a multi-selection to a // singular selection of a block via click. This is handled @@ -68,7 +38,7 @@ export default function useClickSelection() { // multiselection, as focus can be incurred by starting a // multiselection (focus moved to first block's multi- // controls). - selectBlock( clientId ); + selectBlock( getBlockClientId( event.target ) ); } } diff --git a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js index aec4fde08586e..6227b11832c56 100644 --- a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js @@ -17,7 +17,6 @@ import { getBlockClientId } from '../../utils/dom'; * @param {boolean} value `contentEditable` value (true or false) */ function setContentEditableWrapper( node, value ) { - // Since `closest` considers `node` as a candidate, use `parentElement`. node.contentEditable = value; // Firefox doesn't automatically move focus. if ( value ) node.focus(); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 60793c8bfbfc4..1d49eefbc2360 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -858,6 +858,9 @@ export const deleteSelection = ( isForward ) => ( { } ); }; +/** + * Split the current selection. + */ export const splitSelection = () => ( { select, dispatch } ) => { const selectionAnchor = select.getSelectionStart(); const selectionFocus = select.getSelectionEnd(); From 8b0c80dd0aa1a5b9b1fd3c66731aaf023277025f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 21 Mar 2022 20:52:16 +0200 Subject: [PATCH 43/67] wip --- .../writing-flow/use-click-selection.js | 17 +++++++++++++++++ .../use-keyboard-selection-observer.js | 13 +++++++++++-- .../use-mouse-selection-observer.js | 13 +++++++++++-- .../writing-flow/use-multi-selection.js | 10 +++++++++- .../various/multi-block-selection.test.js | 1 + .../src/components/visual-editor/index.js | 5 ++++- 6 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-click-selection.js b/packages/block-editor/src/components/writing-flow/use-click-selection.js index 7705ceb556fe3..dbdea07438089 100644 --- a/packages/block-editor/src/components/writing-flow/use-click-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-click-selection.js @@ -42,10 +42,27 @@ export default function useClickSelection() { } } + function onMouseUp() { + const { ownerDocument } = node; + const { defaultView } = ownerDocument; + const selection = defaultView.getSelection(); + + if ( + ! selection.rangeCount || + selection.isCollapsed || + getBlockClientId( selection.anchorNode ) === + getBlockClientId( selection.focusNode ) + ) { + node.contentEditable = false; + } + } + node.addEventListener( 'mousedown', onMouseDown ); + node.addEventListener( 'mouseup', onMouseUp ); return () => { node.removeEventListener( 'mousedown', onMouseDown ); + node.removeEventListener( 'mouseup', onMouseUp ); }; }, [ diff --git a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js index 0d02d665347f8..0d09ba1d974ea 100644 --- a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js @@ -30,6 +30,16 @@ function extractSelectionEndNode( selection ) { return focusNode.childNodes[ focusOffset - 1 ]; } +function findDepth( a, b ) { + let depth = 0; + + while ( a[ depth ] === b[ depth ] ) { + depth++; + } + + return depth; +} + /** * Sets the `contenteditable` wrapper element to `value`. * @@ -87,8 +97,7 @@ export default function useKeyboardSelectionObserver() { ...getBlockParents( endClientId ), endClientId, ]; - const depth = - Math.min( startPath.length, endPath.length ) - 1; + const depth = findDepth( startPath, endPath ); // Check if selection is already set by rich text. multiSelect( startPath[ depth ], endPath[ depth ] ); diff --git a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js index 6227b11832c56..7b1f0512f9e1f 100644 --- a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js @@ -22,6 +22,16 @@ function setContentEditableWrapper( node, value ) { if ( value ) node.focus(); } +function findDepth( a, b ) { + let depth = 0; + + while ( a[ depth ] === b[ depth ] ) { + depth++; + } + + return depth; +} + /** * Sets a multi-selection based on the native selection across blocks. */ @@ -94,8 +104,7 @@ export default function useSelectionObserver() { ...getBlockParents( endClientId ), endClientId, ]; - const depth = - Math.min( startPath.length, endPath.length ) - 1; + const depth = findDepth( startPath, endPath ); // Check if selection is already set by rich text. multiSelect( startPath[ depth ], endPath[ depth ] ); diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index 5e09d45618ed5..516619801ed5f 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -14,6 +14,7 @@ import { useSelect } from '@wordpress/data'; */ import { store as blockEditorStore } from '../../store'; import { __unstableUseBlockRef as useBlockRef } from '../block-list/use-block-props/use-block-refs'; +import { getBlockClientId } from '../../utils/dom'; function selector( select ) { const { @@ -114,8 +115,15 @@ export default function useMultiSelection() { // Abort if the blocks already contain selection. if ( + selection.rangeCount && selection.containsNode( startRef.current, true ) && - selection.containsNode( endRef.current, true ) + getBlockClientId( startRef.current ) === + getBlockClientId( + selection.getRangeAt( 0 ).startContainer + ) && + selection.containsNode( endRef.current, true ) && + getBlockClientId( endRef.current ) === + getBlockClientId( selection.getRangeAt( 0 ).endContainer ) ) { return; } diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index e8612af915612..4dbed7553f51c 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -353,6 +353,7 @@ describe( 'Multi-block selection', () => { await page.mouse.move( x + 20, y ); await page.mouse.down(); await page.keyboard.up( 'Shift' ); + await page.mouse.up(); await page.keyboard.type( 'hi' ); expect( await getEditedPostContent() ).toMatchInlineSnapshot( ` " diff --git a/packages/edit-post/src/components/visual-editor/index.js b/packages/edit-post/src/components/visual-editor/index.js index 6e5cf20d750ed..83788db45c5ad 100644 --- a/packages/edit-post/src/components/visual-editor/index.js +++ b/packages/edit-post/src/components/visual-editor/index.js @@ -236,7 +236,10 @@ export default function VisualEditor( { styles } ) { /> ) } { ! isTemplateMode && ( -
+
) } From bd3b1f19ed7d096addeeb2952d1bafeda175d8f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 22 Mar 2022 22:03:35 +0200 Subject: [PATCH 44/67] Fix selection issue after quote merge --- .../multi-block-selection.test.js.snap | 16 +++++++++++++ .../various/multi-block-selection.test.js | 24 +++++++++++++++++++ packages/rich-text/src/component/index.js | 8 ++++++- 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 06c873d447255..a5192ae7596f5 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -146,6 +146,22 @@ exports[`Multi-block selection should handle Enter across blocks 2`] = ` " `; +exports[`Multi-block selection should merge into quote with correct selection 1`] = ` +" +

1[

+ + + +

]2

+" +`; + +exports[`Multi-block selection should merge into quote with correct selection 2`] = ` +" +

1

&2

+" +`; + exports[`Multi-block selection should multi-select from within the list block 1`] = ` "

1

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 4dbed7553f51c..9566f8c2ccde4 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -885,4 +885,28 @@ describe( 'Multi-block selection', () => { // Expect two blocks with "&" in between. expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should merge into quote with correct selection', async () => { + await clickBlockAppender(); + await page.keyboard.type( '> 1[' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( ']2' ); + await page.keyboard.press( 'ArrowLeft' ); + // Select everything between []. + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + + // Test setup. + expect( await getEditedPostContent() ).toMatchSnapshot(); + + await page.keyboard.press( 'Backspace' ); + + // Ensure selection is in the correct place. + await page.keyboard.type( '&' ); + + // Expect two blocks with "&" in between. + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } ); diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 6efbce0357cae..a50512f86d06b 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -214,8 +214,14 @@ export function useRichText( { return; } - if ( ref.current.ownerDocument.activeElement !== ref.current ) + if ( ref.current.ownerDocument.activeElement !== ref.current ) { + // Can't focus if there is a parent content editable. + ref.current.parentElement.closest( + '[contenteditable]' + ).contentEditable = false; ref.current.focus(); + } + applyFromProps(); hadSelectionUpdate.current = false; }, [ hadSelectionUpdate.current ] ); From bc1654139541e686c79221d3243a234357de4e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Tue, 22 Mar 2022 23:36:40 +0200 Subject: [PATCH 45/67] Fix e2e tests --- packages/rich-text/src/component/index.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index a50512f86d06b..db0fe3a4b8965 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -216,9 +216,14 @@ export function useRichText( { if ( ref.current.ownerDocument.activeElement !== ref.current ) { // Can't focus if there is a parent content editable. - ref.current.parentElement.closest( - '[contenteditable]' - ).contentEditable = false; + if ( + typeof record.current.start !== 'undefined' && + typeof record.current.end !== 'undefined' + ) { + ref.current.parentElement.closest( + '[contenteditable]' + ).contentEditable = false; + } ref.current.focus(); } From f3e2242b8254e9712fec287f67e5d096039355a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Mar 2022 16:33:24 +0200 Subject: [PATCH 46/67] wip --- .../data/data-core-block-editor.md | 13 --- .../use-keyboard-selection-observer.js | 23 +----- .../use-mouse-selection-observer.js | 20 +---- packages/block-editor/src/store/actions.js | 59 +++++++++++--- packages/block-editor/src/store/selectors.js | 79 ------------------- packages/block-library/src/image/image.js | 1 + 6 files changed, 54 insertions(+), 141 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index f529ca0ee9480..facdee4410f34 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1058,19 +1058,6 @@ _Returns_ - `boolean`: True if it should be possible to multi-select blocks, false if multi-selection is disabled. -### isSelectionMergeable - -Check wether the selection is mergeable. - -_Parameters_ - -- _state_ `Object`: Editor state. -- _isForward_ `boolean`: Wether to merge forwards. - -_Returns_ - -- `boolean`: Wether the selection is mergeable. - ### isTyping Returns true if the user is typing, or false otherwise. diff --git a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js index 0d09ba1d974ea..a8d88787569e7 100644 --- a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js @@ -60,9 +60,7 @@ export default function useKeyboardSelectionObserver() { const { multiSelect, selectBlock, selectionChange } = useDispatch( blockEditorStore ); - const { getBlockParents, isSelectionMergeable } = useSelect( - blockEditorStore - ); + const { getBlockParents } = useSelect( blockEditorStore ); return useRefEffect( ( node ) => { const { ownerDocument } = node; @@ -101,17 +99,6 @@ export default function useKeyboardSelectionObserver() { // Check if selection is already set by rich text. multiSelect( startPath[ depth ], endPath[ depth ] ); - - if ( ! isSelectionMergeable() ) { - selectionChange( { - start: { - clientId: startPath[ depth ], - }, - end: { - clientId: endPath[ depth ], - }, - } ); - } } } @@ -127,12 +114,6 @@ export default function useKeyboardSelectionObserver() { ); }; }, - [ - multiSelect, - selectBlock, - selectionChange, - getBlockParents, - isSelectionMergeable, - ] + [ multiSelect, selectBlock, selectionChange, getBlockParents ] ); } diff --git a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js index 7b1f0512f9e1f..0e13182dc5ef6 100644 --- a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js @@ -43,11 +43,9 @@ export default function useSelectionObserver() { selectBlock, selectionChange, } = useDispatch( blockEditorStore ); - const { - isSelectionEnabled, - getBlockParents, - isSelectionMergeable, - } = useSelect( blockEditorStore ); + const { isSelectionEnabled, getBlockParents } = useSelect( + blockEditorStore + ); return useRefEffect( ( node ) => { const { ownerDocument } = node; @@ -108,17 +106,6 @@ export default function useSelectionObserver() { // Check if selection is already set by rich text. multiSelect( startPath[ depth ], endPath[ depth ] ); - - if ( ! isSelectionMergeable() ) { - selectionChange( { - start: { - clientId: startPath[ depth ], - }, - end: { - clientId: endPath[ depth ], - }, - } ); - } } } @@ -192,7 +179,6 @@ export default function useSelectionObserver() { selectionChange, isSelectionEnabled, getBlockParents, - isSelectionMergeable, ] ); } diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 1d49eefbc2360..a4bd45e31c85a 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -729,9 +729,6 @@ export const deleteSelection = ( isForward ) => ( { const targetSelection = isForward ? selectionEnd : selectionStart; const targetBlock = select.getBlock( targetSelection.clientId ); const targetBlockType = getBlockType( targetBlock.name ); - - if ( ! targetBlockType.merge ) return; - const selectionA = selectionStart; const selectionB = selectionEnd; @@ -762,9 +759,19 @@ export const deleteSelection = ( isForward ) => ( { ...mapRichTextSettings( attributeDefinitionB ), } ); + const followingBlock = isForward ? cloneA : cloneB; + let blocksWithTheSameType = + blockA.name === blockB.name + ? [ followingBlock ] + : switchToBlockType( followingBlock, targetBlockType.name ); + const canMerge = + targetBlockType.merge && + blocksWithTheSameType && + blocksWithTheSameType.length; + // A robust way to retain selection position through various transforms // is to insert a special character at the position and then recover it. - const START_OF_SELECTED_AREA = '\u0086'; + const START_OF_SELECTED_AREA = canMerge ? '\u0086' : ''; valueA = remove( valueA, selectionA.offset, valueA.text.length ); valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); @@ -778,20 +785,50 @@ export const deleteSelection = ( isForward ) => ( { ...mapRichTextSettings( attributeDefinitionB ), } ); - const followingBlock = isForward ? cloneA : cloneB; + if ( ! canMerge ) { + const selectedBlockClientIds = select.getSelectedBlockClientIds(); + const replacement = [ + { + ...blockA, + attributes: { + ...blockA.attributes, + ...cloneA.attributes, + }, + }, + { + ...blockB, + attributes: { + ...blockB.attributes, + ...cloneB.attributes, + }, + }, + ]; + + registry.batch( () => { + dispatch.replaceBlocks( + selectedBlockClientIds, + replacement, + 0, // If we don't pass the `indexToSelect` it will default to the last block. + select.getSelectedBlocksInitialCaretPosition() + ); + + dispatch.selectionChange( + blockB.clientId, + selectionB.attributeKey, + 0, + 0 + ); + } ); + return; + } // We can only merge blocks with similar types // thus, we transform the block to merge first - const blocksWithTheSameType = + blocksWithTheSameType = blockA.name === blockB.name ? [ followingBlock ] : switchToBlockType( followingBlock, targetBlockType.name ); - // If the block types can not match, do nothing - if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { - return; - } - let updatedAttributes; if ( isForward ) { diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index acdea46c5b2c6..7ded791161943 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -28,7 +28,6 @@ import { hasBlockSupport, getPossibleBlockTransformations, parse, - switchToBlockType, } from '@wordpress/blocks'; import { Platform } from '@wordpress/element'; import { applyFilters } from '@wordpress/hooks'; @@ -896,84 +895,6 @@ export function getMultiSelectedBlocksEndClientId( state ) { return selectionEnd.clientId || null; } -/** - * Check wether the selection is mergeable. - * - * @param {Object} state Editor state. - * @param {boolean} isForward Wether to merge forwards. - * - * @return {boolean} Wether the selection is mergeable. - */ -export function isSelectionMergeable( state, isForward ) { - const selectionAnchor = getSelectionStart( state ); - const selectionFocus = getSelectionEnd( state ); - - // It's not mergeable if the start and end are within the same block. - if ( selectionAnchor.clientId === selectionFocus.clientId ) return; - - // It's not mergeable if there's no rich text selection. - if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) - return; - if ( - typeof selectionAnchor.offset === 'undefined' || - typeof selectionFocus.offset === 'undefined' - ) - return; - - const anchorRootClientId = getBlockRootClientId( - state, - selectionAnchor.clientId - ); - const focusRootClientId = getBlockRootClientId( - state, - selectionFocus.clientId - ); - - // It's not mergeable if the selection doesn't start and end in the same - // block list. Maybe in the future it should be allowed. - if ( anchorRootClientId !== focusRootClientId ) { - return; - } - - const blockOrder = getBlockOrder( state, anchorRootClientId ); - const anchorIndex = blockOrder.indexOf( selectionAnchor.clientId ); - const focusIndex = blockOrder.indexOf( selectionFocus.clientId ); - - // Reassign selection start and end based on order. - let selectionStart, selectionEnd; - - if ( anchorIndex > focusIndex ) { - selectionStart = selectionFocus; - selectionEnd = selectionAnchor; - } else { - selectionStart = selectionAnchor; - selectionEnd = selectionFocus; - } - - const targetBlockClientId = isForward - ? selectionEnd.clientId - : selectionStart.clientId; - const blockToMergeClientId = isForward - ? selectionStart.clientId - : selectionEnd.clientId; - - const targetBlock = getBlock( state, targetBlockClientId ); - const targetBlockType = getBlockType( targetBlock.name ); - - if ( ! targetBlockType.merge ) return; - - const blockToMerge = getBlock( state, blockToMergeClientId ); - - // It's mergeable if the blocks are of the same type. - if ( blockToMerge.name === targetBlock.name ) return true; - - // If the blocks are of a different type, try to transform the block being - // merged into the same type of block. - const blocksToMerge = switchToBlockType( blockToMerge, targetBlock.name ); - - return blocksToMerge && blocksToMerge.length; -} - /** * Returns an array containing all block client IDs in the editor in the order * they appear. Optionally accepts a root client ID of the block list for which diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 0fa472afca503..83642ddf8f0dd 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -589,6 +589,7 @@ export default function Image( { { ( ! RichText.isEmpty( caption ) || isSelected ) && ( Date: Wed, 23 Mar 2022 18:52:17 +0200 Subject: [PATCH 47/67] Expand selection on Delete if content can't be merged --- .../use-keyboard-selection-observer.js | 11 ++- .../writing-flow/use-multi-selection.js | 25 ++--- packages/block-editor/src/store/actions.js | 95 +++++++++---------- 3 files changed, 62 insertions(+), 69 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js index a8d88787569e7..ff9ceab3c906f 100644 --- a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js @@ -66,6 +66,8 @@ export default function useKeyboardSelectionObserver() { const { ownerDocument } = node; const { defaultView } = ownerDocument; + let rafId; + function onSelectionChange() { const selection = defaultView.getSelection(); @@ -97,8 +99,12 @@ export default function useKeyboardSelectionObserver() { ]; const depth = findDepth( startPath, endPath ); - // Check if selection is already set by rich text. - multiSelect( startPath[ depth ], endPath[ depth ] ); + // We must allow rich text to set selection first. This + // `selectionchange` listener was added first so it will be + // called before the rich text one. + rafId = defaultView.requestAnimationFrame( () => { + multiSelect( startPath[ depth ], endPath[ depth ] ); + } ); } } @@ -108,6 +114,7 @@ export default function useKeyboardSelectionObserver() { ); return () => { + defaultView.cancelAnimationFrame( rafId ); ownerDocument.removeEventListener( 'selectionchange', onSelectionChange diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index 516619801ed5f..c0f03103f9f37 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -14,7 +14,6 @@ import { useSelect } from '@wordpress/data'; */ import { store as blockEditorStore } from '../../store'; import { __unstableUseBlockRef as useBlockRef } from '../block-list/use-block-props/use-block-refs'; -import { getBlockClientId } from '../../utils/dom'; function selector( select ) { const { @@ -23,6 +22,7 @@ function selector( select ) { hasMultiSelection, getSelectedBlockClientId, getSelectedBlocksInitialCaretPosition, + getSelectionStart, } = select( blockEditorStore ); return { @@ -31,6 +31,7 @@ function selector( select ) { hasMultiSelection: hasMultiSelection(), selectedBlockClientId: getSelectedBlockClientId(), initialPosition: getSelectedBlocksInitialCaretPosition(), + isFullSelection: ! getSelectionStart().attributeKey, }; } @@ -41,6 +42,7 @@ export default function useMultiSelection() { multiSelectedBlockClientIds, hasMultiSelection, selectedBlockClientId, + isFullSelection, } = useSelect( selector, [] ); const selectedRef = useBlockRef( selectedBlockClientId ); // These must be in the right DOM order. @@ -95,6 +97,10 @@ export default function useMultiSelection() { return; } + if ( ! isFullSelection ) { + return; + } + // Allow cross contentEditable selection by temporarily making // all content editable. We can't rely on using the store and // React because re-rending happens too slowly. We need to be @@ -112,22 +118,6 @@ export default function useMultiSelection() { } const selection = defaultView.getSelection(); - - // Abort if the blocks already contain selection. - if ( - selection.rangeCount && - selection.containsNode( startRef.current, true ) && - getBlockClientId( startRef.current ) === - getBlockClientId( - selection.getRangeAt( 0 ).startContainer - ) && - selection.containsNode( endRef.current, true ) && - getBlockClientId( endRef.current ) === - getBlockClientId( selection.getRangeAt( 0 ).endContainer ) - ) { - return; - } - const range = ownerDocument.createRange(); // These must be in the right DOM order. @@ -143,6 +133,7 @@ export default function useMultiSelection() { multiSelectedBlockClientIds, selectedBlockClientId, initialPosition, + isFullSelection, ] ); } diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index a4bd45e31c85a..bc25bde25dc50 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -692,11 +692,20 @@ export const deleteSelection = ( isForward ) => ( { return; } + function expandSelection() { + dispatch.selectionChange( { + start: { clientId: selectionAnchor.clientId }, + end: { clientId: selectionFocus.clientId }, + } ); + } + if ( typeof selectionAnchor.offset === 'undefined' || typeof selectionFocus.offset === 'undefined' - ) + ) { + expandSelection(); return; + } const anchorRootClientId = select.getBlockRootClientId( selectionAnchor.clientId @@ -729,6 +738,12 @@ export const deleteSelection = ( isForward ) => ( { const targetSelection = isForward ? selectionEnd : selectionStart; const targetBlock = select.getBlock( targetSelection.clientId ); const targetBlockType = getBlockType( targetBlock.name ); + + if ( ! targetBlockType.merge ) { + expandSelection(); + return; + } + const selectionA = selectionStart; const selectionB = selectionEnd; @@ -759,19 +774,9 @@ export const deleteSelection = ( isForward ) => ( { ...mapRichTextSettings( attributeDefinitionB ), } ); - const followingBlock = isForward ? cloneA : cloneB; - let blocksWithTheSameType = - blockA.name === blockB.name - ? [ followingBlock ] - : switchToBlockType( followingBlock, targetBlockType.name ); - const canMerge = - targetBlockType.merge && - blocksWithTheSameType && - blocksWithTheSameType.length; - // A robust way to retain selection position through various transforms // is to insert a special character at the position and then recover it. - const START_OF_SELECTED_AREA = canMerge ? '\u0086' : ''; + const START_OF_SELECTED_AREA = '\u0086'; valueA = remove( valueA, selectionA.offset, valueA.text.length ); valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); @@ -785,50 +790,21 @@ export const deleteSelection = ( isForward ) => ( { ...mapRichTextSettings( attributeDefinitionB ), } ); - if ( ! canMerge ) { - const selectedBlockClientIds = select.getSelectedBlockClientIds(); - const replacement = [ - { - ...blockA, - attributes: { - ...blockA.attributes, - ...cloneA.attributes, - }, - }, - { - ...blockB, - attributes: { - ...blockB.attributes, - ...cloneB.attributes, - }, - }, - ]; - - registry.batch( () => { - dispatch.replaceBlocks( - selectedBlockClientIds, - replacement, - 0, // If we don't pass the `indexToSelect` it will default to the last block. - select.getSelectedBlocksInitialCaretPosition() - ); - - dispatch.selectionChange( - blockB.clientId, - selectionB.attributeKey, - 0, - 0 - ); - } ); - return; - } + const followingBlock = isForward ? cloneA : cloneB; // We can only merge blocks with similar types // thus, we transform the block to merge first - blocksWithTheSameType = + const blocksWithTheSameType = blockA.name === blockB.name ? [ followingBlock ] : switchToBlockType( followingBlock, targetBlockType.name ); + // If the block types can not match, do nothing + if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { + expandSelection(); + return; + } + let updatedAttributes; if ( isForward ) { @@ -869,6 +845,7 @@ export const deleteSelection = ( isForward ) => ( { const replacement = [ ...( isForward ? blocksWithTheSameType : [] ), { + // Preserve the original client ID. ...targetBlock, attributes: { ...targetBlock.attributes, @@ -988,7 +965,25 @@ export const splitSelection = () => ( { select, dispatch } ) => { dispatch.replaceBlocks( select.getSelectedBlockClientIds(), - [ cloneA, createBlock( getDefaultBlockName() ), cloneB ], + [ + { + // Preserve the original client ID. + ...blockA, + attributes: { + ...blockA.attributes, + ...cloneA.attributes, + }, + }, + createBlock( getDefaultBlockName() ), + { + // Preserve the original client ID. + ...blockB, + attributes: { + ...blockB.attributes, + ...cloneB.attributes, + }, + }, + ], 1, // If we don't pass the `indexToSelect` it will default to the last block. select.getSelectedBlocksInitialCaretPosition() ); From 92cfa157249a651a8cfc2fda63fdac4ec141d00a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Wed, 23 Mar 2022 22:00:37 +0200 Subject: [PATCH 48/67] Refine --- .../src/components/block-tools/index.js | 40 ++++++-- .../writing-flow/use-multi-selection.js | 4 +- packages/block-editor/src/store/actions.js | 47 ++++----- packages/block-editor/src/store/selectors.js | 97 +++++++++++++++++++ .../various/multi-block-selection.test.js | 1 + 5 files changed, 151 insertions(+), 38 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 9fb56c298de70..5af98c9ac8197 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -11,6 +11,7 @@ import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; import { DELETE, ENTER, BACKSPACE } from '@wordpress/keycodes'; +import { createBlock, getDefaultBlockName } from '@wordpress/blocks'; /** * Internal dependencies @@ -41,9 +42,12 @@ export default function BlockTools( { [] ); const isMatch = useShortcutEventMatch(); - const { getSelectedBlockClientIds, getBlockRootClientId } = useSelect( - blockEditorStore - ); + const { + getSelectedBlockClientIds, + getBlockRootClientId, + __unstableIsSelectionMergeable, + __unstableIsFullySelected, + } = useSelect( blockEditorStore ); const { duplicateBlocks, removeBlocks, @@ -54,6 +58,8 @@ export default function BlockTools( { moveBlocksDown, deleteSelection, splitSelection, + __unstableExpandSelection, + replaceBlocks, } = useDispatch( blockEditorStore ); function onKeyDown( event ) { @@ -108,21 +114,35 @@ export default function BlockTools( { } if ( event.keyCode === ENTER ) { - splitSelection(); - event.preventDefault(); - } else if ( event.keyCode === DELETE ) { - deleteSelection( true ); event.preventDefault(); - } else if ( event.keyCode === BACKSPACE ) { - deleteSelection(); + if ( __unstableIsFullySelected() ) { + replaceBlocks( + clientIds, + createBlock( getDefaultBlockName() ) + ); + } else { + splitSelection(); + } + } else if ( event.keyCode === BACKSPACE || event.keyCode === DELETE ) { event.preventDefault(); + if ( __unstableIsFullySelected() ) { + removeBlocks( clientIds ); + } else if ( __unstableIsSelectionMergeable() ) { + deleteSelection( event.keyCode === DELETE ); + } else { + __unstableExpandSelection(); + } } else if ( // If key.length is longer than 1, it's a control key that doesn't // input anything. event.key.length === 1 && ! ( event.metaKey || event.ctrlKey ) ) { - deleteSelection(); + if ( __unstableIsSelectionMergeable() ) { + deleteSelection( event.keyCode === DELETE ); + } else { + event.preventDefault(); + } } } diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index c0f03103f9f37..01c492a773013 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -22,7 +22,7 @@ function selector( select ) { hasMultiSelection, getSelectedBlockClientId, getSelectedBlocksInitialCaretPosition, - getSelectionStart, + __unstableIsFullySelected, } = select( blockEditorStore ); return { @@ -31,7 +31,7 @@ function selector( select ) { hasMultiSelection: hasMultiSelection(), selectedBlockClientId: getSelectedBlockClientId(), initialPosition: getSelectedBlocksInitialCaretPosition(), - isFullSelection: ! getSelectionStart().attributeKey, + isFullSelection: __unstableIsFullySelected(), }; } diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index bc25bde25dc50..2eaf644bac421 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -686,26 +686,14 @@ export const deleteSelection = ( isForward ) => ( { if ( selectionAnchor.clientId === selectionFocus.clientId ) return; - // Remove the blocks if the whole blocks are selected. - if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) { - dispatch.removeBlocks( select.getSelectedBlockClientIds() ); - return; - } - - function expandSelection() { - dispatch.selectionChange( { - start: { clientId: selectionAnchor.clientId }, - end: { clientId: selectionFocus.clientId }, - } ); - } - + // It's not mergeable if there's no rich text selection. if ( + ! selectionAnchor.attributeKey || + ! selectionFocus.attributeKey || typeof selectionAnchor.offset === 'undefined' || typeof selectionFocus.offset === 'undefined' - ) { - expandSelection(); - return; - } + ) + return false; const anchorRootClientId = select.getBlockRootClientId( selectionAnchor.clientId @@ -740,7 +728,6 @@ export const deleteSelection = ( isForward ) => ( { const targetBlockType = getBlockType( targetBlock.name ); if ( ! targetBlockType.merge ) { - expandSelection(); return; } @@ -801,7 +788,6 @@ export const deleteSelection = ( isForward ) => ( { // If the block types can not match, do nothing if ( ! blocksWithTheSameType || ! blocksWithTheSameType.length ) { - expandSelection(); return; } @@ -881,13 +867,10 @@ export const splitSelection = () => ( { select, dispatch } ) => { if ( selectionAnchor.clientId === selectionFocus.clientId ) return; - // Remove the blocks if the whole blocks are selected. - if ( ! selectionAnchor.attributeKey || ! selectionFocus.attributeKey ) { - dispatch.removeBlocks( select.getSelectedBlockClientIds() ); - return; - } - + // Can't split if the selection is not set. if ( + ! selectionAnchor.attributeKey || + ! selectionFocus.attributeKey || typeof selectionAnchor.offset === 'undefined' || typeof selectionFocus.offset === 'undefined' ) @@ -900,7 +883,7 @@ export const splitSelection = () => ( { select, dispatch } ) => { selectionFocus.clientId ); - // It's not mergeable if the selection doesn't start and end in the same + // It's not splittable if the selection doesn't start and end in the same // block list. Maybe in the future it should be allowed. if ( anchorRootClientId !== focusRootClientId ) { return; @@ -989,6 +972,18 @@ export const splitSelection = () => ( { select, dispatch } ) => { ); }; +/** + * Expand the selection to cover the entire blocks, removing partial selection. + */ +export const __unstableExpandSelection = () => ( { select, dispatch } ) => { + const selectionAnchor = select.getSelectionStart(); + const selectionFocus = select.getSelectionEnd(); + dispatch.selectionChange( { + start: { clientId: selectionAnchor.clientId }, + end: { clientId: selectionFocus.clientId }, + } ); +}; + /** * Action that merges two blocks. * diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 7ded791161943..b601ed50dc01f 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -28,6 +28,7 @@ import { hasBlockSupport, getPossibleBlockTransformations, parse, + switchToBlockType, } from '@wordpress/blocks'; import { Platform } from '@wordpress/element'; import { applyFilters } from '@wordpress/hooks'; @@ -895,6 +896,102 @@ export function getMultiSelectedBlocksEndClientId( state ) { return selectionEnd.clientId || null; } +/** + * Returns true if the selection is not partial. + * + * @param {Object} state Editor state. + * + * @return {boolean} Wether the selection is mergeable. + */ +export function __unstableIsFullySelected( state ) { + const selectionAnchor = getSelectionStart( state ); + const selectionFocus = getSelectionEnd( state ); + return ( + ! selectionAnchor.attributeKey && + ! selectionFocus.attributeKey && + typeof selectionAnchor.offset === 'undefined' && + typeof selectionFocus.offset === 'undefined' + ); +} + +/** + * Check wether the selection is mergeable. + * + * @param {Object} state Editor state. + * @param {boolean} isForward Wether to merge forwards. + * + * @return {boolean} Wether the selection is mergeable. + */ +export function __unstableIsSelectionMergeable( state, isForward ) { + const selectionAnchor = getSelectionStart( state ); + const selectionFocus = getSelectionEnd( state ); + + // It's not mergeable if the start and end are within the same block. + if ( selectionAnchor.clientId === selectionFocus.clientId ) return false; + + // It's not mergeable if there's no rich text selection. + if ( + ! selectionAnchor.attributeKey || + ! selectionFocus.attributeKey || + typeof selectionAnchor.offset === 'undefined' || + typeof selectionFocus.offset === 'undefined' + ) + return false; + + const anchorRootClientId = getBlockRootClientId( + state, + selectionAnchor.clientId + ); + const focusRootClientId = getBlockRootClientId( + state, + selectionFocus.clientId + ); + + // It's not mergeable if the selection doesn't start and end in the same + // block list. Maybe in the future it should be allowed. + if ( anchorRootClientId !== focusRootClientId ) { + return false; + } + + const blockOrder = getBlockOrder( state, anchorRootClientId ); + const anchorIndex = blockOrder.indexOf( selectionAnchor.clientId ); + const focusIndex = blockOrder.indexOf( selectionFocus.clientId ); + + // Reassign selection start and end based on order. + let selectionStart, selectionEnd; + + if ( anchorIndex > focusIndex ) { + selectionStart = selectionFocus; + selectionEnd = selectionAnchor; + } else { + selectionStart = selectionAnchor; + selectionEnd = selectionFocus; + } + + const targetBlockClientId = isForward + ? selectionEnd.clientId + : selectionStart.clientId; + const blockToMergeClientId = isForward + ? selectionStart.clientId + : selectionEnd.clientId; + + const targetBlock = getBlock( state, targetBlockClientId ); + const targetBlockType = getBlockType( targetBlock.name ); + + if ( ! targetBlockType.merge ) return false; + + const blockToMerge = getBlock( state, blockToMergeClientId ); + + // It's mergeable if the blocks are of the same type. + if ( blockToMerge.name === targetBlock.name ) return true; + + // If the blocks are of a different type, try to transform the block being + // merged into the same type of block. + const blocksToMerge = switchToBlockType( blockToMerge, targetBlock.name ); + + return blocksToMerge && blocksToMerge.length; +} + /** * Returns an array containing all block client IDs in the editor in the order * they appear. Optionally accepts a root client ID of the block list for which diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 9566f8c2ccde4..105667501ae5b 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -873,6 +873,7 @@ describe( 'Multi-block selection', () => { await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + await page.evaluate( () => new Promise( requestAnimationFrame ) ); // Test setup. expect( await getEditedPostContent() ).toMatchSnapshot(); From 660e9ca07c499fb150d24f7a83e4de06698729fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 24 Mar 2022 00:54:54 +0200 Subject: [PATCH 49/67] Polish --- .../data/data-core-block-editor.md | 14 +--------- .../src/components/block-tools/index.js | 10 +++---- .../src/components/rich-text/index.js | 2 -- .../src/components/writing-flow/index.js | 2 ++ .../src/components/writing-flow/readme.md | 28 +++++++++++++++++++ .../use-firefox-comat.js} | 14 +++------- .../use-keyboard-selection-observer.js | 18 +++++++++++- packages/block-editor/src/store/actions.js | 22 ++++++++++----- packages/block-library/src/image/image.js | 1 - .../multi-block-selection.test.js.snap | 16 +++++++++++ .../various/multi-block-selection.test.js | 20 +++++++++++++ 11 files changed, 108 insertions(+), 39 deletions(-) create mode 100644 packages/block-editor/src/components/writing-flow/readme.md rename packages/block-editor/src/components/{rich-text/use-focus.js => writing-flow/use-firefox-comat.js} (72%) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index facdee4410f34..40079f46c6018 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1110,14 +1110,6 @@ _Returns_ - `Object`: Action object. -### deleteSelection - -Delete the current selection. - -_Parameters_ - -- _isForward_ `boolean`: - ### duplicateBlocks Action that duplicates a list of blocks. @@ -1398,7 +1390,7 @@ Action that changes the position of the user caret. _Parameters_ -- _clientId_ `string|Object`: The selected block client ID. +- _clientId_ `string|WPSelection`: The selected block client ID. - _attributeKey_ `string`: The selected block attribute key. - _startOffset_ `number`: The start offset. - _endOffset_ `number`: The end offset. @@ -1476,10 +1468,6 @@ _Returns_ - `Object`: Action object. -### splitSelection - -Split the current selection. - ### startDraggingBlocks Returns an action object used in signalling that the user has begun to drag blocks. diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 5af98c9ac8197..a2cfef777f6a2 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -56,8 +56,8 @@ export default function BlockTools( { clearSelectedBlock, moveBlocksUp, moveBlocksDown, - deleteSelection, - splitSelection, + __unstableDeleteSelection, + __unstableSplitSelection, __unstableExpandSelection, replaceBlocks, } = useDispatch( blockEditorStore ); @@ -121,14 +121,14 @@ export default function BlockTools( { createBlock( getDefaultBlockName() ) ); } else { - splitSelection(); + __unstableSplitSelection(); } } else if ( event.keyCode === BACKSPACE || event.keyCode === DELETE ) { event.preventDefault(); if ( __unstableIsFullySelected() ) { removeBlocks( clientIds ); } else if ( __unstableIsSelectionMergeable() ) { - deleteSelection( event.keyCode === DELETE ); + __unstableDeleteSelection( event.keyCode === DELETE ); } else { __unstableExpandSelection(); } @@ -139,7 +139,7 @@ export default function BlockTools( { ! ( event.metaKey || event.ctrlKey ) ) { if ( __unstableIsSelectionMergeable() ) { - deleteSelection( event.keyCode === DELETE ); + __unstableDeleteSelection( event.keyCode === DELETE ); } else { event.preventDefault(); } diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index a088823232d3f..125848e717b20 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -45,7 +45,6 @@ import { useFormatTypes } from './use-format-types'; import { useRemoveBrowserShortcuts } from './use-remove-browser-shortcuts'; import { useShortcuts } from './use-shortcuts'; import { useInputEvents } from './use-input-events'; -import { useFocus } from './use-focus'; import FormatEdit from './format-edit'; import { getMultilineTag, getAllowedFormats } from './utils'; @@ -392,7 +391,6 @@ function RichTextWrapper( disableLineBreaks, onSplitAtEnd, } ), - useFocus(), anchorRef, ] ) } contentEditable={ true } diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 83d361f487e19..d0e7393893c6d 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -21,6 +21,7 @@ import useSelectAll from './use-select-all'; import useMouseSelectionObserver from './use-mouse-selection-observer'; import useKeyboardSelectionObserver from './use-keyboard-selection-observer'; import useClickSelection from './use-click-selection'; +import useFirefoxCompat from './use-firefox-comat'; import { store as blockEditorStore } from '../../store'; export function useWritingFlow() { @@ -34,6 +35,7 @@ export function useWritingFlow() { before, useMergeRefs( [ ref, + useFirefoxCompat(), useMouseSelectionObserver(), useKeyboardSelectionObserver(), useClickSelection(), diff --git a/packages/block-editor/src/components/writing-flow/readme.md b/packages/block-editor/src/components/writing-flow/readme.md new file mode 100644 index 0000000000000..eb8c720ca12e7 --- /dev/null +++ b/packages/block-editor/src/components/writing-flow/readme.md @@ -0,0 +1,28 @@ +# Writing Flow + +This hook handles selection across blocks. + +## Partial multi-block selection + +Selecting across blocks is possible by temporarily setting the `contentEditable` +attribute to `true`. This sounds scary, but we prevent all default behaviours +except for selection, so don't worry. :) + +* For selection by mouse, we make everything editable when the mouse leaves an + editable field. +* For Shift+Click selection, we do it on `mousedown`. +* For keyboard selection we do it when the selection reaches the edge of an + editable field. + +In the future, we should consider using the `contentEditable` attribute for +arrow key navigation as well. + +Now that it's possible to select across blocks, we need to sync this state to +the block editor store. We can do this by listening to the `selectionchange` +event. In writing flow, we can sync the selected block client ID, but when the +selection starts or ends in a rich text field, it will be rich text that sync a +more precise position (the block attribute key and offset in addition to the +client ID). + +With the selection state in the block editor store, we can now handle things +like Backspace, Delete, and Enter. diff --git a/packages/block-editor/src/components/rich-text/use-focus.js b/packages/block-editor/src/components/writing-flow/use-firefox-comat.js similarity index 72% rename from packages/block-editor/src/components/rich-text/use-focus.js rename to packages/block-editor/src/components/writing-flow/use-firefox-comat.js index 76f3295128bea..425f5dd77546f 100644 --- a/packages/block-editor/src/components/rich-text/use-focus.js +++ b/packages/block-editor/src/components/writing-flow/use-firefox-comat.js @@ -9,7 +9,7 @@ import { useSelect } from '@wordpress/data'; */ import { store as blockEditorStore } from '../../store'; -export function useFocus() { +export default function useFirefoxCompat() { const { isMultiSelecting } = useSelect( blockEditorStore ); return useRefEffect( ( element ) => { function onFocus() { @@ -22,18 +22,12 @@ export function useFocus() { // element sometimes regains focus, while it should not be focusable // and focus should remain on the editable parent element. // To do: try to find the cause of the shifting focus. - const parentEditable = element.parentElement.closest( - '[contenteditable="true"]' - ); - - if ( parentEditable ) { - parentEditable.focus(); - } + element.focus(); } - element.addEventListener( 'focus', onFocus ); + element.addEventListener( 'focusin', onFocus ); return () => { - element.removeEventListener( 'focus', onFocus ); + element.removeEventListener( 'focusin', onFocus ); }; }, [] ); } diff --git a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js index ff9ceab3c906f..06b4536c81a26 100644 --- a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js @@ -10,6 +10,14 @@ import { useRefEffect } from '@wordpress/compose'; import { store as blockEditorStore } from '../../store'; import { getBlockClientId } from '../../utils/dom'; +/** + * Extract the selection start node from the selection. When the anchor node is + * not a text node, the selection offset is the index of a child node. + * + * @param {Selection} selection The selection. + * + * @return {Element} The selection start node. + */ function extractSelectionStartNode( selection ) { const { anchorNode, anchorOffset } = selection; @@ -20,6 +28,15 @@ function extractSelectionStartNode( selection ) { return anchorNode.childNodes[ anchorOffset ]; } +/** + * Extract the selection end node from the selection. When the focus node is not + * a text node, the selection offset is the index of a child node. The selection + * reaches up to but excluding that child node. + * + * @param {Selection} selection The selection. + * + * @return {Element} The selection start node. + */ function extractSelectionEndNode( selection ) { const { focusNode, focusOffset } = selection; @@ -47,7 +64,6 @@ function findDepth( a, b ) { * @param {boolean} value `contentEditable` value (true or false) */ function setContentEditableWrapper( node, value ) { - // Since `closest` considers `node` as a candidate, use `parentElement`. node.contentEditable = value; // Firefox doesn't automatically move focus. if ( value ) node.focus(); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 2eaf644bac421..689ba2e5f4549 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -100,6 +100,15 @@ export const validateBlocksToTemplate = ( blocks ) => ( { * text value. See `wp.richText.create`. */ +/** + * A selection object. + * + * @typedef {Object} WPSelection + * + * @property {WPBlockSelection} start The selection start. + * @property {WPBlockSelection} end The selection end. + */ + /* eslint-disable jsdoc/valid-types */ /** * Returns an action object used in signalling that selection state should be @@ -676,7 +685,7 @@ function mapRichTextSettings( attributeDefinition ) { * * @param {boolean} isForward */ -export const deleteSelection = ( isForward ) => ( { +export const __unstableDeleteSelection = ( isForward ) => ( { registry, select, dispatch, @@ -861,7 +870,7 @@ export const deleteSelection = ( isForward ) => ( { /** * Split the current selection. */ -export const splitSelection = () => ( { select, dispatch } ) => { +export const __unstableSplitSelection = () => ( { select, dispatch } ) => { const selectionAnchor = select.getSelectionStart(); const selectionFocus = select.getSelectionEnd(); @@ -995,7 +1004,6 @@ export const mergeBlocks = ( firstBlockClientId, secondBlockClientId ) => ( { dispatch, } ) => { const blocks = [ firstBlockClientId, secondBlockClientId ]; - dispatch( { type: 'MERGE_BLOCKS', blocks } ); const [ clientIdA, clientIdB ] = blocks; @@ -1289,10 +1297,10 @@ export function exitFormattedText() { /** * Action that changes the position of the user caret. * - * @param {string|Object} clientId The selected block client ID. - * @param {string} attributeKey The selected block attribute key. - * @param {number} startOffset The start offset. - * @param {number} endOffset The end offset. + * @param {string|WPSelection} clientId The selected block client ID. + * @param {string} attributeKey The selected block attribute key. + * @param {number} startOffset The start offset. + * @param {number} endOffset The end offset. * * @return {Object} Action object. */ diff --git a/packages/block-library/src/image/image.js b/packages/block-library/src/image/image.js index 83642ddf8f0dd..0fa472afca503 100644 --- a/packages/block-library/src/image/image.js +++ b/packages/block-library/src/image/image.js @@ -589,7 +589,6 @@ export default function Image( { { ( ! RichText.isEmpty( caption ) || isSelected ) && ( +
+ + + +

a

+" +`; + +exports[`Multi-block selection should select separator (single element block) 2`] = ` +" +

&

+" +`; + exports[`Multi-block selection should set attributes for multiple paragraphs 1`] = ` "

1

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 105667501ae5b..977b079b2fbfa 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -910,4 +910,24 @@ describe( 'Multi-block selection', () => { // Expect two blocks with "&" in between. expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should select separator (single element block)', async () => { + await clickBlockAppender(); + await page.keyboard.type( '/hr' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( 'a' ); + await pressKeyWithModifier( 'shift', 'ArrowUp' ); + + // Test setup. + expect( await getEditedPostContent() ).toMatchSnapshot(); + + await page.keyboard.press( 'Backspace' ); + + // Ensure selection is in the correct place. + await page.keyboard.type( '&' ); + + // Expect two blocks with "&" in between. + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } ); From 0219ce01251ef9a0286602f29541225fafc44986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 25 Mar 2022 13:25:41 +0200 Subject: [PATCH 50/67] Revert firefox compat move --- .../block-editor/src/components/rich-text/index.js | 2 ++ .../use-firefox-compat.js} | 14 ++++++++++---- .../src/components/writing-flow/index.js | 2 -- 3 files changed, 12 insertions(+), 6 deletions(-) rename packages/block-editor/src/components/{writing-flow/use-firefox-comat.js => rich-text/use-firefox-compat.js} (72%) diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 125848e717b20..3e0adca5bf779 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -45,6 +45,7 @@ import { useFormatTypes } from './use-format-types'; import { useRemoveBrowserShortcuts } from './use-remove-browser-shortcuts'; import { useShortcuts } from './use-shortcuts'; import { useInputEvents } from './use-input-events'; +import { useFirefoxCompat } from './use-firefox-compat'; import FormatEdit from './format-edit'; import { getMultilineTag, getAllowedFormats } from './utils'; @@ -391,6 +392,7 @@ function RichTextWrapper( disableLineBreaks, onSplitAtEnd, } ), + useFirefoxCompat(), anchorRef, ] ) } contentEditable={ true } diff --git a/packages/block-editor/src/components/writing-flow/use-firefox-comat.js b/packages/block-editor/src/components/rich-text/use-firefox-compat.js similarity index 72% rename from packages/block-editor/src/components/writing-flow/use-firefox-comat.js rename to packages/block-editor/src/components/rich-text/use-firefox-compat.js index 425f5dd77546f..59a5718508163 100644 --- a/packages/block-editor/src/components/writing-flow/use-firefox-comat.js +++ b/packages/block-editor/src/components/rich-text/use-firefox-compat.js @@ -9,7 +9,7 @@ import { useSelect } from '@wordpress/data'; */ import { store as blockEditorStore } from '../../store'; -export default function useFirefoxCompat() { +export function useFirefoxCompat() { const { isMultiSelecting } = useSelect( blockEditorStore ); return useRefEffect( ( element ) => { function onFocus() { @@ -22,12 +22,18 @@ export default function useFirefoxCompat() { // element sometimes regains focus, while it should not be focusable // and focus should remain on the editable parent element. // To do: try to find the cause of the shifting focus. - element.focus(); + const parentEditable = element.parentElement.closest( + '[contenteditable="true"]' + ); + + if ( parentEditable ) { + parentEditable.focus(); + } } - element.addEventListener( 'focusin', onFocus ); + element.addEventListener( 'focus', onFocus ); return () => { - element.removeEventListener( 'focusin', onFocus ); + element.removeEventListener( 'focus', onFocus ); }; }, [] ); } diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index d0e7393893c6d..83d361f487e19 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -21,7 +21,6 @@ import useSelectAll from './use-select-all'; import useMouseSelectionObserver from './use-mouse-selection-observer'; import useKeyboardSelectionObserver from './use-keyboard-selection-observer'; import useClickSelection from './use-click-selection'; -import useFirefoxCompat from './use-firefox-comat'; import { store as blockEditorStore } from '../../store'; export function useWritingFlow() { @@ -35,7 +34,6 @@ export function useWritingFlow() { before, useMergeRefs( [ ref, - useFirefoxCompat(), useMouseSelectionObserver(), useKeyboardSelectionObserver(), useClickSelection(), From a573615db50f36182495bd9343469c38d8649fa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 25 Mar 2022 14:17:09 +0200 Subject: [PATCH 51/67] Revert unnessary changes for this PR --- .../block-editor/src/components/rich-text/split-value.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/split-value.js b/packages/block-editor/src/components/rich-text/split-value.js index 7ea2b87bc3953..86e801882dcd2 100644 --- a/packages/block-editor/src/components/rich-text/split-value.js +++ b/packages/block-editor/src/components/rich-text/split-value.js @@ -20,12 +20,8 @@ export function splitValue( { return; } - // Ensure the value has a selection. This might happen when trying to split - // an empty value before there was a `selectionchange` event. - const { start = 0, end = 0 } = value; - const valueWithSelection = { ...value, start, end }; const blocks = []; - const [ before, after ] = split( valueWithSelection ); + const [ before, after ] = split( value ); const hasPastedBlocks = pastedBlocks.length > 0; let lastPastedBlockIndex = -1; From 07e13e74381bb0df71440de30d20b625bcde039b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 25 Mar 2022 15:18:38 +0200 Subject: [PATCH 52/67] Remove unnessary cloning --- packages/block-editor/src/store/actions.js | 52 ++++++++++------------ 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 689ba2e5f4549..cf40758ed4810 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -749,12 +749,8 @@ export const __unstableDeleteSelection = ( isForward ) => ( { const blockB = select.getBlock( selectionB.clientId ); const blockBType = getBlockType( blockB.name ); - // Clone the blocks so we don't insert the character in a "live" block. - const cloneA = cloneBlock( blockA ); - const cloneB = cloneBlock( blockB ); - - const htmlA = cloneA.attributes[ selectionA.attributeKey ]; - const htmlB = cloneB.attributes[ selectionB.attributeKey ]; + const htmlA = blockA.attributes[ selectionA.attributeKey ]; + const htmlB = blockB.attributes[ selectionB.attributeKey ]; const attributeDefinitionA = blockAType.attributes[ selectionA.attributeKey ]; @@ -777,13 +773,18 @@ export const __unstableDeleteSelection = ( isForward ) => ( { valueA = remove( valueA, selectionA.offset, valueA.text.length ); valueB = insert( valueB, START_OF_SELECTED_AREA, 0, selectionB.offset ); - cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { - value: valueA, - ...mapRichTextSettings( attributeDefinitionA ), + // Clone the blocks so we don't manipulate the original. + const cloneA = cloneBlock( blockA, { + [ selectionA.attributeKey ]: toHTMLString( { + value: valueA, + ...mapRichTextSettings( attributeDefinitionA ), + } ), } ); - cloneB.attributes[ selectionB.attributeKey ] = toHTMLString( { - value: valueB, - ...mapRichTextSettings( attributeDefinitionB ), + const cloneB = cloneBlock( blockB, { + [ selectionB.attributeKey ]: toHTMLString( { + value: valueB, + ...mapRichTextSettings( attributeDefinitionB ), + } ), } ); const followingBlock = isForward ? cloneA : cloneB; @@ -922,12 +923,8 @@ export const __unstableSplitSelection = () => ( { select, dispatch } ) => { const blockB = select.getBlock( selectionB.clientId ); const blockBType = getBlockType( blockB.name ); - // Clone the blocks so we don't insert the character in a "live" block. - const cloneA = cloneBlock( blockA ); - const cloneB = cloneBlock( blockB ); - - const htmlA = cloneA.attributes[ selectionA.attributeKey ]; - const htmlB = cloneB.attributes[ selectionB.attributeKey ]; + const htmlA = blockA.attributes[ selectionA.attributeKey ]; + const htmlB = blockB.attributes[ selectionB.attributeKey ]; const attributeDefinitionA = blockAType.attributes[ selectionA.attributeKey ]; @@ -946,15 +943,6 @@ export const __unstableSplitSelection = () => ( { select, dispatch } ) => { valueA = remove( valueA, selectionA.offset, valueA.text.length ); valueB = remove( valueB, 0, selectionB.offset ); - cloneA.attributes[ selectionA.attributeKey ] = toHTMLString( { - value: valueA, - ...mapRichTextSettings( attributeDefinitionA ), - } ); - cloneB.attributes[ selectionB.attributeKey ] = toHTMLString( { - value: valueB, - ...mapRichTextSettings( attributeDefinitionB ), - } ); - dispatch.replaceBlocks( select.getSelectedBlockClientIds(), [ @@ -963,7 +951,10 @@ export const __unstableSplitSelection = () => ( { select, dispatch } ) => { ...blockA, attributes: { ...blockA.attributes, - ...cloneA.attributes, + [ selectionA.attributeKey ]: toHTMLString( { + value: valueA, + ...mapRichTextSettings( attributeDefinitionA ), + } ), }, }, createBlock( getDefaultBlockName() ), @@ -972,7 +963,10 @@ export const __unstableSplitSelection = () => ( { select, dispatch } ) => { ...blockB, attributes: { ...blockB.attributes, - ...cloneB.attributes, + [ selectionB.attributeKey ]: toHTMLString( { + value: valueB, + ...mapRichTextSettings( attributeDefinitionB ), + } ), }, }, ], From 022f9fb9fef6c476c8c63b67908614b1b6565a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Fri, 25 Mar 2022 16:03:46 +0200 Subject: [PATCH 53/67] Fix typos --- docs/reference-guides/data/data-core-block-editor.md | 2 +- packages/block-editor/src/store/actions.js | 2 +- packages/block-editor/src/store/selectors.js | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 40079f46c6018..602251b853c94 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1462,7 +1462,7 @@ _Parameters_ - _rootClientId_ `?string`: Optional root client ID of block list on which to insert. - _index_ `?number`: Index at which block should be inserted. -- _\_\_unstableOptions_ `Object`: Wether or not to show an inserter button. +- _\_\_unstableOptions_ `Object`: Whether or not to show an inserter button. _Returns_ diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index cf40758ed4810..29e0775f1b621 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -610,7 +610,7 @@ export const insertBlocks = ( * @param {?string} rootClientId Optional root client ID of block list on * which to insert. * @param {?number} index Index at which block should be inserted. - * @param {Object} __unstableOptions Wether or not to show an inserter button. + * @param {Object} __unstableOptions Whether or not to show an inserter button. * * @return {Object} Action object. */ diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index b601ed50dc01f..df1e488da4822 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -901,7 +901,7 @@ export function getMultiSelectedBlocksEndClientId( state ) { * * @param {Object} state Editor state. * - * @return {boolean} Wether the selection is mergeable. + * @return {boolean} Whether the selection is mergeable. */ export function __unstableIsFullySelected( state ) { const selectionAnchor = getSelectionStart( state ); @@ -915,12 +915,12 @@ export function __unstableIsFullySelected( state ) { } /** - * Check wether the selection is mergeable. + * Check whether the selection is mergeable. * * @param {Object} state Editor state. - * @param {boolean} isForward Wether to merge forwards. + * @param {boolean} isForward Whether to merge forwards. * - * @return {boolean} Wether the selection is mergeable. + * @return {boolean} Whether the selection is mergeable. */ export function __unstableIsSelectionMergeable( state, isForward ) { const selectionAnchor = getSelectionStart( state ); From aab3f6c1c87dac7215d4fbde6a6b069ffdb4c7aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sat, 26 Mar 2022 02:12:38 +0200 Subject: [PATCH 54/67] Move input handling to writing flow --- .../src/components/block-tools/index.js | 124 ++++++------------ .../src/components/writing-flow/index.js | 2 + .../src/components/writing-flow/use-input.js | 83 ++++++++++++ 3 files changed, 126 insertions(+), 83 deletions(-) create mode 100644 packages/block-editor/src/components/writing-flow/use-input.js diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index a2cfef777f6a2..d581a43dae4a8 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -10,8 +10,6 @@ import { useSelect, useDispatch } from '@wordpress/data'; import { useViewportMatch } from '@wordpress/compose'; import { Popover } from '@wordpress/components'; import { __unstableUseShortcutEventMatch as useShortcutEventMatch } from '@wordpress/keyboard-shortcuts'; -import { DELETE, ENTER, BACKSPACE } from '@wordpress/keycodes'; -import { createBlock, getDefaultBlockName } from '@wordpress/blocks'; /** * Internal dependencies @@ -42,12 +40,9 @@ export default function BlockTools( { [] ); const isMatch = useShortcutEventMatch(); - const { - getSelectedBlockClientIds, - getBlockRootClientId, - __unstableIsSelectionMergeable, - __unstableIsFullySelected, - } = useSelect( blockEditorStore ); + const { getSelectedBlockClientIds, getBlockRootClientId } = useSelect( + blockEditorStore + ); const { duplicateBlocks, removeBlocks, @@ -56,92 +51,55 @@ export default function BlockTools( { clearSelectedBlock, moveBlocksUp, moveBlocksDown, - __unstableDeleteSelection, - __unstableSplitSelection, - __unstableExpandSelection, - replaceBlocks, } = useDispatch( blockEditorStore ); function onKeyDown( event ) { - const clientIds = getSelectedBlockClientIds(); - - if ( ! clientIds.length ) return; - if ( isMatch( 'core/block-editor/move-up', event ) ) { - event.preventDefault(); - const rootClientId = getBlockRootClientId( first( clientIds ) ); - moveBlocksUp( clientIds, rootClientId ); + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length ) { + event.preventDefault(); + const rootClientId = getBlockRootClientId( first( clientIds ) ); + moveBlocksUp( clientIds, rootClientId ); + } } else if ( isMatch( 'core/block-editor/move-down', event ) ) { - event.preventDefault(); - const rootClientId = getBlockRootClientId( first( clientIds ) ); - moveBlocksDown( clientIds, rootClientId ); + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length ) { + event.preventDefault(); + const rootClientId = getBlockRootClientId( first( clientIds ) ); + moveBlocksDown( clientIds, rootClientId ); + } } else if ( isMatch( 'core/block-editor/duplicate', event ) ) { - event.preventDefault(); - duplicateBlocks( clientIds ); + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length ) { + event.preventDefault(); + duplicateBlocks( clientIds ); + } } else if ( isMatch( 'core/block-editor/remove', event ) ) { - event.preventDefault(); - removeBlocks( clientIds ); + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length ) { + event.preventDefault(); + removeBlocks( clientIds ); + } } else if ( isMatch( 'core/block-editor/insert-after', event ) ) { - event.preventDefault(); - insertAfterBlock( last( clientIds ) ); - } else if ( isMatch( 'core/block-editor/insert-before', event ) ) { - event.preventDefault(); - insertBeforeBlock( first( clientIds ) ); - } - - /** - * Check if the target element is a text area, input or - * event.defaultPrevented and return early. In all these - * cases backspace could be handled elsewhere. - */ - if ( - [ 'INPUT', 'TEXTAREA' ].includes( event.target.nodeName ) || - event.defaultPrevented - ) { - return; - } - - if ( clientIds.length === 1 ) { - return; - } - - if ( isMatch( 'core/block-editor/unselect', event ) ) { - event.preventDefault(); - clearSelectedBlock(); - event.target.ownerDocument.defaultView - .getSelection() - .removeAllRanges(); - } - - if ( event.keyCode === ENTER ) { - event.preventDefault(); - if ( __unstableIsFullySelected() ) { - replaceBlocks( - clientIds, - createBlock( getDefaultBlockName() ) - ); - } else { - __unstableSplitSelection(); + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length ) { + event.preventDefault(); + insertAfterBlock( last( clientIds ) ); } - } else if ( event.keyCode === BACKSPACE || event.keyCode === DELETE ) { - event.preventDefault(); - if ( __unstableIsFullySelected() ) { - removeBlocks( clientIds ); - } else if ( __unstableIsSelectionMergeable() ) { - __unstableDeleteSelection( event.keyCode === DELETE ); - } else { - __unstableExpandSelection(); + } else if ( isMatch( 'core/block-editor/insert-before', event ) ) { + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length ) { + event.preventDefault(); + insertBeforeBlock( first( clientIds ) ); } - } else if ( - // If key.length is longer than 1, it's a control key that doesn't - // input anything. - event.key.length === 1 && - ! ( event.metaKey || event.ctrlKey ) - ) { - if ( __unstableIsSelectionMergeable() ) { - __unstableDeleteSelection( event.keyCode === DELETE ); - } else { + } else if ( isMatch( 'core/block-editor/unselect', event ) ) { + const clientIds = getSelectedBlockClientIds(); + if ( clientIds.length > 1 ) { event.preventDefault(); + clearSelectedBlock(); + event.target.ownerDocument.defaultView + .getSelection() + .removeAllRanges(); } } } diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 83d361f487e19..6f76d26ce97ae 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -21,6 +21,7 @@ import useSelectAll from './use-select-all'; import useMouseSelectionObserver from './use-mouse-selection-observer'; import useKeyboardSelectionObserver from './use-keyboard-selection-observer'; import useClickSelection from './use-click-selection'; +import useInput from './use-input'; import { store as blockEditorStore } from '../../store'; export function useWritingFlow() { @@ -34,6 +35,7 @@ export function useWritingFlow() { before, useMergeRefs( [ ref, + useInput(), useMouseSelectionObserver(), useKeyboardSelectionObserver(), useClickSelection(), diff --git a/packages/block-editor/src/components/writing-flow/use-input.js b/packages/block-editor/src/components/writing-flow/use-input.js new file mode 100644 index 0000000000000..08aeb1cdccdc8 --- /dev/null +++ b/packages/block-editor/src/components/writing-flow/use-input.js @@ -0,0 +1,83 @@ +/** + * WordPress dependencies + */ +import { useSelect, useDispatch } from '@wordpress/data'; +import { useRefEffect } from '@wordpress/compose'; +import { ENTER, BACKSPACE, DELETE } from '@wordpress/keycodes'; +import { createBlock, getDefaultBlockName } from '@wordpress/blocks'; + +/** + * Internal dependencies + */ +import { store as blockEditorStore } from '../../store'; + +/** + * Handles input for selections across blocks. + */ +export default function useInput() { + const { + __unstableIsFullySelected, + getSelectedBlockClientIds, + __unstableIsSelectionMergeable, + hasMultiSelection, + } = useSelect( blockEditorStore ); + const { + replaceBlocks, + __unstableSplitSelection, + removeBlocks, + __unstableDeleteSelection, + __unstableExpandSelection, + } = useDispatch( blockEditorStore ); + + return useRefEffect( ( node ) => { + function onKeyDown( event ) { + if ( event.defaultPrevented ) { + return; + } + + if ( ! hasMultiSelection() ) { + return; + } + + if ( event.keyCode === ENTER ) { + event.preventDefault(); + if ( __unstableIsFullySelected() ) { + replaceBlocks( + getSelectedBlockClientIds(), + createBlock( getDefaultBlockName() ) + ); + } else { + __unstableSplitSelection(); + } + } else if ( + event.keyCode === BACKSPACE || + event.keyCode === DELETE + ) { + event.preventDefault(); + if ( __unstableIsFullySelected() ) { + removeBlocks( getSelectedBlockClientIds() ); + } else if ( __unstableIsSelectionMergeable() ) { + __unstableDeleteSelection( event.keyCode === DELETE ); + } else { + __unstableExpandSelection(); + } + } else if ( + // If key.length is longer than 1, it's a control key that doesn't + // input anything. + event.key.length === 1 && + ! ( event.metaKey || event.ctrlKey ) + ) { + if ( __unstableIsSelectionMergeable() ) { + __unstableDeleteSelection( event.keyCode === DELETE ); + } else { + event.preventDefault(); + } + } + } + + node.addEventListener( 'keydown', onKeyDown ); + return () => { + node.removeEventListener( 'keydown', onKeyDown ); + }; + }, [] ); +} From 8597e0f5f5012b903d832fdab17a69a912f0f6d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sat, 26 Mar 2022 10:33:53 +0200 Subject: [PATCH 55/67] Fix e2e test --- ...ock-editor-keyboard-shortcuts.test.js.snap | 26 +++++++++++++++++++ .../block-editor-keyboard-shortcuts.test.js | 8 +++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-editor-keyboard-shortcuts.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-editor-keyboard-shortcuts.test.js.snap index 3be07280f1846..ccce7a2b3c383 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-editor-keyboard-shortcuts.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-editor-keyboard-shortcuts.test.js.snap @@ -125,3 +125,29 @@ exports[`block editor keyboard shortcuts test shortcuts handling through portals

3rd

" `; + +exports[`block editor keyboard shortcuts test shortcuts handling through portals in the same tree should propagate properly and duplicate selected blocks 1`] = ` +" +

1st

+ + + +

2nd

+ + + +

3rd

+ + + +

1st

+ + + +

2nd

+ + + +

3rd

+" +`; diff --git a/packages/e2e-tests/specs/editor/various/block-editor-keyboard-shortcuts.test.js b/packages/e2e-tests/specs/editor/various/block-editor-keyboard-shortcuts.test.js index 25c7fd5ccd224..98f5e06790090 100644 --- a/packages/e2e-tests/specs/editor/various/block-editor-keyboard-shortcuts.test.js +++ b/packages/e2e-tests/specs/editor/various/block-editor-keyboard-shortcuts.test.js @@ -76,16 +76,14 @@ describe( 'block editor keyboard shortcuts', () => { await pressKeyWithModifier( 'primary', 'a' ); await pressKeyWithModifier( 'primary', 'a' ); } ); - it( 'should propagate properly and delete selected blocks', async () => { + it( 'should propagate properly and duplicate selected blocks', async () => { await clickBlockToolbarButton( 'Options' ); const label = 'Duplicate'; await page.$x( `//div[@role="menu"]//span[contains(concat(" ", @class, " "), " components-menu-item__item ")][contains(text(), "${ label }")]` ); - await page.keyboard.press( 'Delete' ); - expect( await getEditedPostContent() ).toMatchInlineSnapshot( - `""` - ); + await pressKeyWithModifier( 'primaryShift', 'd' ); + expect( await getEditedPostContent() ).toMatchSnapshot(); } ); it( 'should prevent deleting multiple selected blocks from inputs', async () => { await clickBlockToolbarButton( 'Options' ); From 88a848bfeaa81b7858d5d15b81542cf9ea7779f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 00:09:39 +0200 Subject: [PATCH 56/67] Add more comments --- .../specs/editor/various/copy-cut-paste-whole-blocks.test.js | 2 ++ .../specs/editor/various/multi-block-selection.test.js | 1 + packages/rich-text/src/component/index.js | 4 +++- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js index fa2feb7bb529b..ef5608fb84403 100644 --- a/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js @@ -100,6 +100,7 @@ describe( 'Copy/cut/paste of whole blocks', () => { await page.click( '.editor-block-list-item-paragraph' ); await page.keyboard.type( 'P' ); await page.keyboard.press( 'ArrowLeft' ); + // Delay from writing-flow/use-keyboard-selection-observer.js await page.evaluate( () => new Promise( requestAnimationFrame ) ); await page.keyboard.press( 'ArrowLeft' ); // Cut group. @@ -146,6 +147,7 @@ describe( 'Copy/cut/paste of whole blocks', () => { await page.click( '.editor-block-list-item-paragraph' ); await page.keyboard.type( 'P' ); await page.keyboard.press( 'ArrowLeft' ); + // Delay from writing-flow/use-keyboard-selection-observer.js await page.evaluate( () => new Promise( requestAnimationFrame ) ); await page.keyboard.press( 'ArrowLeft' ); // Cut group. diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 977b079b2fbfa..964d2dc95576f 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -873,6 +873,7 @@ describe( 'Multi-block selection', () => { await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); + // Delay from writing-flow/use-keyboard-selection-observer.js await page.evaluate( () => new Promise( requestAnimationFrame ) ); // Test setup. diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index db0fe3a4b8965..68f436d524f9b 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -215,7 +215,9 @@ export function useRichText( { } if ( ref.current.ownerDocument.activeElement !== ref.current ) { - // Can't focus if there is a parent content editable. + // Can't focus if there is a parent content editable. WritingFlow + // should turn this off, but this component (child) renders before + // WritingFlow (parent) renders. if ( typeof record.current.start !== 'undefined' && typeof record.current.end !== 'undefined' From dfc4cb2a23488a8a87b485c7b5c3adad649bc4db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 01:30:08 +0200 Subject: [PATCH 57/67] Try removing duplicate code --- .../src/components/writing-flow/index.js | 8 +- ...tion-observer.js => use-drag-selection.js} | 124 +++++------------- ...-observer.js => use-selection-observer.js} | 2 +- 3 files changed, 36 insertions(+), 98 deletions(-) rename packages/block-editor/src/components/writing-flow/{use-mouse-selection-observer.js => use-drag-selection.js} (53%) rename packages/block-editor/src/components/writing-flow/{use-keyboard-selection-observer.js => use-selection-observer.js} (98%) diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 6f76d26ce97ae..dfd44e828c7f8 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -18,8 +18,8 @@ import useMultiSelection from './use-multi-selection'; import useTabNav from './use-tab-nav'; import useArrowNav from './use-arrow-nav'; import useSelectAll from './use-select-all'; -import useMouseSelectionObserver from './use-mouse-selection-observer'; -import useKeyboardSelectionObserver from './use-keyboard-selection-observer'; +import useDragSelection from './use-drag-selection'; +import useSelectionObserver from './use-selection-observer'; import useClickSelection from './use-click-selection'; import useInput from './use-input'; import { store as blockEditorStore } from '../../store'; @@ -36,8 +36,8 @@ export function useWritingFlow() { useMergeRefs( [ ref, useInput(), - useMouseSelectionObserver(), - useKeyboardSelectionObserver(), + useDragSelection(), + useSelectionObserver(), useClickSelection(), useMultiSelection(), useSelectAll(), diff --git a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-drag-selection.js similarity index 53% rename from packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js rename to packages/block-editor/src/components/writing-flow/use-drag-selection.js index 0e13182dc5ef6..4a05c45afdcd4 100644 --- a/packages/block-editor/src/components/writing-flow/use-mouse-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-drag-selection.js @@ -8,7 +8,6 @@ import { useRefEffect } from '@wordpress/compose'; * Internal dependencies */ import { store as blockEditorStore } from '../../store'; -import { getBlockClientId } from '../../utils/dom'; /** * Sets the `contenteditable` wrapper element to `value`. @@ -22,28 +21,14 @@ function setContentEditableWrapper( node, value ) { if ( value ) node.focus(); } -function findDepth( a, b ) { - let depth = 0; - - while ( a[ depth ] === b[ depth ] ) { - depth++; - } - - return depth; -} - /** * Sets a multi-selection based on the native selection across blocks. */ -export default function useSelectionObserver() { - const { - startMultiSelect, - stopMultiSelect, - multiSelect, - selectBlock, - selectionChange, - } = useDispatch( blockEditorStore ); - const { isSelectionEnabled, getBlockParents } = useSelect( +export default function useDragSelection() { + const { startMultiSelect, stopMultiSelect } = useDispatch( + blockEditorStore + ); + const { isSelectionEnabled, hasMultiSelection } = useSelect( blockEditorStore ); return useRefEffect( @@ -54,73 +39,37 @@ export default function useSelectionObserver() { let anchorElement; let rafId; - function onSelectionChange( { isSelectionEnd } ) { - const selection = defaultView.getSelection(); - - // If no selection is found, end multi selection and disable the - // contentEditable wrapper. - if ( ! selection.rangeCount || selection.isCollapsed ) { - setContentEditableWrapper( node, false ); - return; - } - - const clientId = getBlockClientId( selection.anchorNode ); - const endClientId = getBlockClientId( selection.focusNode ); - const isSingularSelection = clientId === endClientId; - - if ( isSingularSelection ) { - selectBlock( clientId ); + function onMouseUp() { + // Equivalent to attaching the listener once. + defaultView.removeEventListener( 'mouseup', onMouseUp ); + // The browser selection won't have updated yet at this point, + // so wait until the next animation frame to get the browser + // selection. + rafId = defaultView.requestAnimationFrame( () => { + if ( hasMultiSelection() ) { + return; + } // If the selection is complete (on mouse up), and no // multiple blocks have been selected, set focus back to the // anchor element. if the anchor element contains the // selection. Additionally, the contentEditable wrapper can // now be disabled again. - if ( isSelectionEnd ) { - setContentEditableWrapper( node, false ); - - if ( selection.rangeCount ) { - const { - commonAncestorContainer, - } = selection.getRangeAt( 0 ); - - if ( - anchorElement.contains( - commonAncestorContainer - ) - ) { - anchorElement.focus(); - } + setContentEditableWrapper( node, false ); + + const selection = defaultView.getSelection(); + + if ( selection.rangeCount ) { + const { + commonAncestorContainer, + } = selection.getRangeAt( 0 ); + + if ( + anchorElement.contains( commonAncestorContainer ) + ) { + anchorElement.focus(); } } - } else { - const startPath = [ - ...getBlockParents( clientId ), - clientId, - ]; - const endPath = [ - ...getBlockParents( endClientId ), - endClientId, - ]; - const depth = findDepth( startPath, endPath ); - - // Check if selection is already set by rich text. - multiSelect( startPath[ depth ], endPath[ depth ] ); - } - } - - function onSelectionEnd() { - ownerDocument.removeEventListener( - 'selectionchange', - onSelectionChange - ); - // Equivalent to attaching the listener once. - defaultView.removeEventListener( 'mouseup', onSelectionEnd ); - // The browser selection won't have updated yet at this point, - // so wait until the next animation frame to get the browser - // selection. - rafId = defaultView.requestAnimationFrame( () => { - onSelectionChange( { isSelectionEnd: true } ); stopMultiSelect(); } ); } @@ -146,11 +95,7 @@ export default function useSelectionObserver() { // `onSelectionStart` is called after `mousedown` and // `mouseleave` (from a block). The selection ends when // `mouseup` happens anywhere in the window. - ownerDocument.addEventListener( - 'selectionchange', - onSelectionChange - ); - defaultView.addEventListener( 'mouseup', onSelectionEnd ); + defaultView.addEventListener( 'mouseup', onMouseUp ); // Allow cross contentEditable selection by temporarily making // all content editable. We can't rely on using the store and @@ -163,22 +108,15 @@ export default function useSelectionObserver() { return () => { node.removeEventListener( 'mouseout', onMouseLeave ); - ownerDocument.removeEventListener( - 'selectionchange', - onSelectionChange - ); - defaultView.removeEventListener( 'mouseup', onSelectionEnd ); + defaultView.removeEventListener( 'mouseup', onMouseUp ); defaultView.cancelAnimationFrame( rafId ); }; }, [ startMultiSelect, stopMultiSelect, - multiSelect, - selectBlock, - selectionChange, isSelectionEnabled, - getBlockParents, + hasMultiSelection, ] ); } diff --git a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js b/packages/block-editor/src/components/writing-flow/use-selection-observer.js similarity index 98% rename from packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js rename to packages/block-editor/src/components/writing-flow/use-selection-observer.js index 06b4536c81a26..a9f253a8f23b1 100644 --- a/packages/block-editor/src/components/writing-flow/use-keyboard-selection-observer.js +++ b/packages/block-editor/src/components/writing-flow/use-selection-observer.js @@ -72,7 +72,7 @@ function setContentEditableWrapper( node, value ) { /** * Sets a multi-selection based on the native selection across blocks. */ -export default function useKeyboardSelectionObserver() { +export default function useSelectionObserver() { const { multiSelect, selectBlock, selectionChange } = useDispatch( blockEditorStore ); From dfdcf59dbcc9558827ae87d920ff534576310e58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 02:24:13 +0200 Subject: [PATCH 58/67] Make e2e tests pass again --- .../src/components/writing-flow/use-drag-selection.js | 2 +- .../src/components/writing-flow/use-selection-observer.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) 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 4a05c45afdcd4..7cfd83d144f7c 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 @@ -40,6 +40,7 @@ export default function useDragSelection() { let rafId; function onMouseUp() { + stopMultiSelect(); // Equivalent to attaching the listener once. defaultView.removeEventListener( 'mouseup', onMouseUp ); // The browser selection won't have updated yet at this point, @@ -70,7 +71,6 @@ export default function useDragSelection() { anchorElement.focus(); } } - stopMultiSelect(); } ); } 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 a9f253a8f23b1..7a7a874edc2b3 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 @@ -128,6 +128,7 @@ export default function useSelectionObserver() { 'selectionchange', onSelectionChange ); + defaultView.addEventListener( 'mouseup', onSelectionChange ); return () => { defaultView.cancelAnimationFrame( rafId ); @@ -135,6 +136,7 @@ export default function useSelectionObserver() { 'selectionchange', onSelectionChange ); + defaultView.removeEventListener( 'mouseup', onSelectionChange ); }; }, [ multiSelect, selectBlock, selectionChange, getBlockParents ] From 67a0c20ad357f594362a58977bd0d5af12617b43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 13:19:51 +0300 Subject: [PATCH 59/67] Remove raf hack --- .../writing-flow/use-selection-observer.js | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) 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 7a7a874edc2b3..9d65dc3d8656b 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 @@ -82,8 +82,6 @@ export default function useSelectionObserver() { const { ownerDocument } = node; const { defaultView } = ownerDocument; - let rafId; - function onSelectionChange() { const selection = defaultView.getSelection(); @@ -115,28 +113,39 @@ export default function useSelectionObserver() { ]; const depth = findDepth( startPath, endPath ); - // We must allow rich text to set selection first. This - // `selectionchange` listener was added first so it will be - // called before the rich text one. - rafId = defaultView.requestAnimationFrame( () => { - multiSelect( startPath[ depth ], endPath[ depth ] ); - } ); + multiSelect( startPath[ depth ], endPath[ depth ] ); } } - ownerDocument.addEventListener( - 'selectionchange', - onSelectionChange - ); - defaultView.addEventListener( 'mouseup', onSelectionChange ); + function addListeners() { + ownerDocument.addEventListener( + 'selectionchange', + onSelectionChange + ); + defaultView.addEventListener( 'mouseup', onSelectionChange ); + } - return () => { - defaultView.cancelAnimationFrame( rafId ); + function removeListeners() { 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 ] From 50d24077a1ea0c94edeaf229decfe19b182cbf4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 14:34:15 +0300 Subject: [PATCH 60/67] Fix block first focus --- .../block-list/use-block-props/use-focus-first-element.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js index 523e36ec241ad..c3a2ebc1aa6c7 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js @@ -31,7 +31,6 @@ function useInitialPosition( clientId ) { ( select ) => { const { getSelectedBlocksInitialCaretPosition, - isMultiSelecting, isNavigationMode, isBlockSelected, } = select( blockEditorStore ); @@ -40,7 +39,7 @@ function useInitialPosition( clientId ) { return; } - if ( isMultiSelecting() || isNavigationMode() ) { + if ( isNavigationMode() ) { return; } @@ -72,11 +71,11 @@ function isFormElement( element ) { export function useFocusFirstElement( clientId ) { const ref = useRef(); const initialPosition = useInitialPosition( clientId ); - const { isBlockSelected } = useSelect( blockEditorStore ); + const { isBlockSelected, isMultiSelecting } = useSelect( blockEditorStore ); useEffect( () => { // Check if the block is still selected at the time this effect runs. - if ( ! isBlockSelected( clientId ) ) { + if ( ! isBlockSelected( clientId ) || isMultiSelecting() ) { return; } From fb09ae3e9a9938fc40236deb419fd969195cb5e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 20:10:48 +0300 Subject: [PATCH 61/67] Fix comments in e2e tests about raf --- .../specs/editor/various/copy-cut-paste-whole-blocks.test.js | 4 ++-- .../specs/editor/various/multi-block-selection.test.js | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js index ef5608fb84403..19d194d9607b7 100644 --- a/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/copy-cut-paste-whole-blocks.test.js @@ -100,7 +100,7 @@ describe( 'Copy/cut/paste of whole blocks', () => { await page.click( '.editor-block-list-item-paragraph' ); await page.keyboard.type( 'P' ); await page.keyboard.press( 'ArrowLeft' ); - // Delay from writing-flow/use-keyboard-selection-observer.js + // Needs to be investigated why this is needed. await page.evaluate( () => new Promise( requestAnimationFrame ) ); await page.keyboard.press( 'ArrowLeft' ); // Cut group. @@ -147,7 +147,7 @@ describe( 'Copy/cut/paste of whole blocks', () => { await page.click( '.editor-block-list-item-paragraph' ); await page.keyboard.type( 'P' ); await page.keyboard.press( 'ArrowLeft' ); - // Delay from writing-flow/use-keyboard-selection-observer.js + // Needs to be investigated why this is needed. await page.evaluate( () => new Promise( requestAnimationFrame ) ); await page.keyboard.press( 'ArrowLeft' ); // Cut group. diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 964d2dc95576f..26166433fbdca 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -873,8 +873,6 @@ describe( 'Multi-block selection', () => { await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); await pressKeyWithModifier( 'shift', 'ArrowLeft' ); - // Delay from writing-flow/use-keyboard-selection-observer.js - await page.evaluate( () => new Promise( requestAnimationFrame ) ); // Test setup. expect( await getEditedPostContent() ).toMatchSnapshot(); From f04a1506bdc4ebad4da0da41b7eb37fd1ff9048b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Sun, 27 Mar 2022 23:49:27 +0300 Subject: [PATCH 62/67] Fix shift+click selection and add e2e test --- .../use-block-props/use-focus-handler.js | 8 ++++++ .../multi-block-selection.test.js.snap | 16 ++++++++++++ .../various/multi-block-selection.test.js | 25 +++++++++++++++++++ .../src/component/use-input-and-selection.js | 6 +++++ 4 files changed, 55 insertions(+) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js b/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js index ac9e0f8c6d1bd..4e9ffa4572500 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js @@ -30,6 +30,14 @@ export function useFocusHandler( clientId ) { * @param {FocusEvent} event Focus event. */ function onFocus( event ) { + // When the whole editor is editable, let writing flow handle + // selection. + if ( + node.parentElement.closest( '[contenteditable="true"]' ) + ) { + return; + } + // Check synchronously because a non-selected block might be // getting data through `useSelect` asynchronously. if ( isBlockSelected( clientId ) ) { diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index e5f3f556c5a6c..df1be62cbe878 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -192,6 +192,22 @@ exports[`Multi-block selection should only trigger multi-selection when at the e " `; +exports[`Multi-block selection should partially select with shift + click 1`] = ` +" +

1[

+ + + +

]2

+" +`; + +exports[`Multi-block selection should partially select with shift + click 2`] = ` +" +

1&2

+" +`; + exports[`Multi-block selection should place the caret at the end of last pasted paragraph (paste mid-block) 1`] = ` "

first paragraph

diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 26166433fbdca..29004182e01e8 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -929,4 +929,29 @@ describe( 'Multi-block selection', () => { // Expect two blocks with "&" in between. expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should partially select with shift + click', async () => { + await clickBlockAppender(); + await pressKeyWithModifier( 'primary', 'b' ); + await page.keyboard.type( '1' ); + await pressKeyWithModifier( 'primary', 'b' ); + await page.keyboard.type( '[' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( ']2' ); + await page.keyboard.press( 'ArrowLeft' ); + await page.keyboard.down( 'Shift' ); + await page.click( 'strong' ); + await page.keyboard.up( 'Shift' ); + + // Test setup. + expect( await getEditedPostContent() ).toMatchSnapshot(); + + await page.keyboard.press( 'Backspace' ); + + // Ensure selection is in the correct place. + await page.keyboard.type( '&' ); + + // Expect two blocks with "&" in between. + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } ); 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 7ec56b2c47a9c..173a54dd463dd 100644 --- a/packages/rich-text/src/component/use-input-and-selection.js +++ b/packages/rich-text/src/component/use-input-and-selection.js @@ -259,6 +259,12 @@ export function useInputAndSelection( props ) { applyRecord, } = propsRef.current; + // When the whole editor is editable, let writing flow handle + // selection. + if ( element.parentElement.closest( '[contenteditable="true"]' ) ) { + return; + } + if ( ! isSelected ) { // We know for certain that on focus, the old selection is invalid. // It will be recalculated on the next mouseup, keyup, or touchend From dca35d9d60188cbb68616c3d8ac174406ac4eb9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 28 Mar 2022 00:59:31 +0300 Subject: [PATCH 63/67] Remove unnecessary code --- .../writing-flow/use-click-selection.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-click-selection.js b/packages/block-editor/src/components/writing-flow/use-click-selection.js index dbdea07438089..7705ceb556fe3 100644 --- a/packages/block-editor/src/components/writing-flow/use-click-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-click-selection.js @@ -42,27 +42,10 @@ export default function useClickSelection() { } } - function onMouseUp() { - const { ownerDocument } = node; - const { defaultView } = ownerDocument; - const selection = defaultView.getSelection(); - - if ( - ! selection.rangeCount || - selection.isCollapsed || - getBlockClientId( selection.anchorNode ) === - getBlockClientId( selection.focusNode ) - ) { - node.contentEditable = false; - } - } - node.addEventListener( 'mousedown', onMouseDown ); - node.addEventListener( 'mouseup', onMouseUp ); return () => { node.removeEventListener( 'mousedown', onMouseDown ); - node.removeEventListener( 'mouseup', onMouseUp ); }; }, [ From 593194b6ef458ec4e51ca5556b9a927b54d924a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 28 Mar 2022 10:10:42 +0300 Subject: [PATCH 64/67] Check to see which tests are failing without this --- .../block-list/use-block-props/use-focus-first-element.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js index c3a2ebc1aa6c7..02ea0143bdee2 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-focus-first-element.js @@ -128,12 +128,6 @@ export function useFocusFirstElement( clientId ) { } } - // This is needed because this hook is called before the writing flow - // hook has the chance to turn off content editable. - ref.current.parentElement.closest( - '[contenteditable]' - ).contentEditable = false; - placeCaretAtHorizontalEdge( target, isReverse ); }, [ initialPosition, clientId ] ); From ab19bcf0759d36b5c3c15cb36f17c7f0f11cb3b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 28 Mar 2022 10:52:14 +0300 Subject: [PATCH 65/67] Check if this is still needed --- packages/rich-text/src/component/index.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 68f436d524f9b..0cb718287c684 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -215,17 +215,6 @@ export function useRichText( { } if ( ref.current.ownerDocument.activeElement !== ref.current ) { - // Can't focus if there is a parent content editable. WritingFlow - // should turn this off, but this component (child) renders before - // WritingFlow (parent) renders. - if ( - typeof record.current.start !== 'undefined' && - typeof record.current.end !== 'undefined' - ) { - ref.current.parentElement.closest( - '[contenteditable]' - ).contentEditable = false; - } ref.current.focus(); } From 347c6b717c3bf9e2aacb52bfd8e63d65139a3192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 28 Mar 2022 11:31:26 +0300 Subject: [PATCH 66/67] Fix shift+click selection inside block --- .../components/writing-flow/use-click-selection.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-click-selection.js b/packages/block-editor/src/components/writing-flow/use-click-selection.js index 7705ceb556fe3..3d7aec8d4ea4c 100644 --- a/packages/block-editor/src/components/writing-flow/use-click-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-click-selection.js @@ -27,10 +27,15 @@ export default function useClickSelection() { return; } + const startClientId = getBlockSelectionStart(); + const clickedClientId = getBlockClientId( event.target ); + if ( event.shiftKey ) { - node.contentEditable = true; - // Firefox doesn't automatically move focus. - node.focus(); + if ( startClientId !== clickedClientId ) { + node.contentEditable = true; + // Firefox doesn't automatically move focus. + node.focus(); + } } else if ( hasMultiSelection() ) { // Allow user to escape out of a multi-selection to a // singular selection of a block via click. This is handled @@ -38,7 +43,7 @@ export default function useClickSelection() { // multiselection, as focus can be incurred by starting a // multiselection (focus moved to first block's multi- // controls). - selectBlock( getBlockClientId( event.target ) ); + selectBlock( clickedClientId ); } } From 1659b3714acb2f705088dfce2243cc373f76f815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Mon, 28 Mar 2022 11:59:07 +0300 Subject: [PATCH 67/67] Fix selection inside list and quote blocks --- .../src/components/writing-flow/use-drag-selection.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 7cfd83d144f7c..f11c07a9d4c2e 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 @@ -81,7 +81,11 @@ export default function useDragSelection() { return; } - if ( ! target.contentEditable ) { + // 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 + // multi selecting when the mouse leaves the wrapper. + if ( ! target.getAttribute( 'contenteditable' ) ) { return; }