[Writing Flow]: Try to fix multi-selection with shift+click (#40687)

* [Writing Flow]: Try to fix multi-selection with `shift+click`

* change check with collapsed selection

* address feedback

* target specific mouse event with shift

* add e2e tests

packages/block-editor/src/components/writing-flow/use-click-selection.js

@@ -11,10 +11,9 @@ import { store as blockEditorStore } from '../../store';
 import { getBlockClientId } from '../../utils/dom';
 
 export default function useClickSelection() {
-	const { multiSelect, selectBlock } = useDispatch( blockEditorStore );
+	const { selectBlock } = useDispatch( blockEditorStore );
 	const {
 		isSelectionEnabled,
-		getBlockParents,
 		getBlockSelectionStart,
 		hasMultiSelection,
 	} = useSelect( blockEditorStore );
@@ -54,10 +53,8 @@ export default function useClickSelection() {
 			};
 		},
 		[
-			multiSelect,
 			selectBlock,
 			isSelectionEnabled,
-			getBlockParents,
 			getBlockSelectionStart,
 			hasMultiSelection,
 		]

packages/block-editor/src/components/writing-flow/use-selection-observer.js

@@ -76,43 +76,81 @@ export default function useSelectionObserver() {
 	const { multiSelect, selectBlock, selectionChange } = useDispatch(
 		blockEditorStore
 	);
-	const { getBlockParents } = useSelect( blockEditorStore );
+	const { getBlockParents, getBlockSelectionStart } = useSelect(
+		blockEditorStore
+	);
 	return useRefEffect(
 		( node ) => {
 			const { ownerDocument } = node;
 			const { defaultView } = ownerDocument;
 
-			function onSelectionChange() {
+			function onSelectionChange( event ) {
 				const selection = defaultView.getSelection();
-
 				// If no selection is found, end multi selection and disable the
 				// contentEditable wrapper.
-				if ( ! selection.rangeCount || selection.isCollapsed ) {
+				if ( ! selection.rangeCount ) {
+					setContentEditableWrapper( node, false );
+					return;
+				}
+				// If selection is collapsed and we haven't used `shift+click`,
+				// end multi selection and disable the contentEditable wrapper.
+				// We have to check about `shift+click` case because elements
+				// that don't support text selection might be involved, and we might
+				// update the clientIds to multi-select blocks.
+				// For now we check if the event is a `mouse` event.
+				const isClickShift = event.shiftKey && event.type === 'mouseup';
+				if ( selection.isCollapsed && ! isClickShift ) {
 					setContentEditableWrapper( node, false );
 					return;
 				}
 
-				const clientId = getBlockClientId(
+				let startClientId = getBlockClientId(
 					extractSelectionStartNode( selection )
 				);
-				const endClientId = getBlockClientId(
+				let endClientId = getBlockClientId(
 					extractSelectionEndNode( selection )
 				);
+				// If the selection has changed and we had pressed `shift+click`,
+				// we need to check if in an element that doesn't support
+				// text selection has been clicked.
+				if ( isClickShift ) {
+					const selectedClientId = getBlockSelectionStart();
+					const clickedClientId = getBlockClientId( event.target );
+					// `endClientId` is not defined if we end the selection by clicking a non-selectable block.
+					// We need to check if there was already a selection with a non-selectable focusNode.
+					const focusNodeIsNonSelectable =
+						clickedClientId !== endClientId;
+					if (
+						( startClientId === endClientId &&
+							selection.isCollapsed ) ||
+						! endClientId ||
+						focusNodeIsNonSelectable
+					) {
+						endClientId = clickedClientId;
+					}
+					// Handle the case when we have a non-selectable block
+					// selected and click another one.
+					if ( startClientId !== selectedClientId ) {
+						startClientId = selectedClientId;
+					}
+				}
 
-				// If the selection did not involve a block, return early.
-				if ( clientId === undefined && endClientId === undefined ) {
+				// If the selection did not involve a block, return.
+				if (
+					startClientId === undefined &&
+					endClientId === undefined
+				) {
 					setContentEditableWrapper( node, false );
 					return;
 				}
 
-				const isSingularSelection = clientId === endClientId;
-
+				const isSingularSelection = startClientId === endClientId;
 				if ( isSingularSelection ) {
-					selectBlock( clientId );
+					selectBlock( startClientId );
 				} else {
 					const startPath = [
-						...getBlockParents( clientId ),
-						clientId,
+						...getBlockParents( startClientId ),
+						startClientId,
 					];
 					const endPath = [
 						...getBlockParents( endClientId ),

packages/e2e-tests/specs/editor/various/multi-block-selection.test.js

@@ -10,6 +10,7 @@ import {
 	clickBlockToolbarButton,
 	clickButton,
 	clickMenuItem,
+	insertBlock,
 	openListView,
 	saveDraft,
 	transformBlockTo,
@@ -975,4 +976,51 @@ describe( 'Multi-block selection', () => {
 		// Expect two blocks with "&" in between.
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
+	describe( 'shift+click multi-selection', () => {
+		it( 'should multi-select block with text selection and a block without text selection', async () => {
+			await page.keyboard.press( 'Enter' );
+			await page.keyboard.type( 'hi' );
+			await page.keyboard.press( 'Enter' );
+			await insertBlock( 'Spacer' );
+			await page.keyboard.press( 'ArrowUp' );
+
+			const spacerBlock = await page.waitForSelector(
+				'.wp-block.wp-block-spacer'
+			);
+			const boundingBox = await spacerBlock.boundingBox();
+			const mousePosition = {
+				x: boundingBox.x + boundingBox.width / 2,
+				y: boundingBox.y + boundingBox.height / 2,
+			};
+			await page.keyboard.down( 'Shift' );
+			await page.mouse.click( mousePosition.x, mousePosition.y );
+			await page.keyboard.up( 'Shift' );
+
+			const selectedBlocks = await page.$$(
+				'.wp-block.is-multi-selected'
+			);
+			expect( selectedBlocks.length ).toBe( 2 );
+		} );
+		it( 'should multi-select blocks without text selection', async () => {
+			await insertBlock( 'Spacer' );
+			// Get the first spacer block element.
+			const spacerBlock = await page.waitForSelector(
+				'.wp-block.wp-block-spacer'
+			);
+			const boundingBox = await spacerBlock.boundingBox();
+			await page.keyboard.press( 'Enter' );
+			await insertBlock( 'Spacer' );
+			const mousePosition = {
+				x: boundingBox.x + boundingBox.width / 2,
+				y: boundingBox.y + boundingBox.height / 2,
+			};
+			await page.keyboard.down( 'Shift' );
+			await page.mouse.click( mousePosition.x, mousePosition.y );
+			await page.keyboard.up( 'Shift' );
+			const selectedBlocks = await page.$$(
+				'.wp-block.is-multi-selected'
+			);
+			expect( selectedBlocks.length ).toBe( 2 );
+		} );
+	} );
 } );

packages/rich-text/src/component/use-input-and-selection.js

@@ -238,7 +238,7 @@ export function useInputAndSelection( props ) {
 		function onCompositionStart() {
 			isComposing = true;
 			// Do not update the selection when characters are being composed as
-			// this rerenders the component and might distroy internal browser
+			// this rerenders the component and might destroy internal browser
 			// editing state.
 			ownerDocument.removeEventListener(
 				'selectionchange',