Editor: WritingFlow: Consider selection collapsed for Shift+Arrow at navigable edge

packages/dom/CHANGELOG.md

@@ -1,4 +1,8 @@
-## 2.0.9 (Unreleased)
+## 2.1.0 (Unreleased)
+
+### New Feature
+
+- New function: `isSelectionForward( selection: Selection )`
 
 ### Bug Fix
 

packages/dom/src/dom.js

@@ -25,7 +25,7 @@ const {
  *
  * @return {boolean} Whether the selection is forward.
  */
-function isSelectionForward( selection ) {
+export function isSelectionForward( selection ) {
 	const {
 		anchorNode,
 		focusNode,

packages/e2e-tests/specs/__snapshots__/writing-flow.test.js.snap

@@ -161,3 +161,19 @@ exports[`adding blocks should not delete surrounding space when deleting a word
 <p>1 2 3</p>
 <!-- /wp:paragraph -->"
 `;
+
+exports[`adding blocks should respect non-collapsed selection directionality in determining edge 1`] = `
+"<!-- wp:paragraph -->
+<p>As I expected</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`adding blocks should respect non-collapsed selection directionality in determining edge 2`] = `
+"<!-- wp:paragraph -->
+<p>As I expected</p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p>A<br>B- After B<br>C</p>
+<!-- /wp:paragraph -->"
+`;

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

@@ -137,4 +137,19 @@ describe( 'Multi-block selection', () => {
 		const speakTextContent = await page.$eval( '#a11y-speak-assertive', ( element ) => element.textContent );
 		expect( speakTextContent.trim() ).toEqual( '3 blocks selected.' );
 	} );
+
+	it( 'should multi-select from the collapsed edge of a block', async () => {
+		// Explanation: Ensure that WritingFlow's consideration of a navigable
+		// edge of a block doesn't wrongly prevent multi-selection.
+		await clickBlockAppender();
+		await page.keyboard.type( 'First' );
+		await page.keyboard.press( 'Enter' );
+
+		// In a new paragraph, the selection is collapsed at the leading edge
+		// of the node. Shift+ArrowUp should multi-select with the prior block.
+		await pressKeyWithModifier( 'shift', 'ArrowUp' );
+
+		const count = await page.$$eval( '.is-multi-selected', ( blocks ) => blocks.length );
+		expect( count ).toBe( 2 );
+	} );
 } );

packages/e2e-tests/specs/writing-flow.test.js

@@ -7,6 +7,7 @@ import {
 	createNewPost,
 	pressKeyTimes,
 	pressKeyWithModifier,
+	insertBlock,
 } from '@wordpress/e2e-test-utils';
 
 describe( 'adding blocks', () => {
@@ -290,4 +291,55 @@ describe( 'adding blocks', () => {
 		// Check that none of the paragraph blocks have <br> in them.
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
+
+	it( 'should respect non-collapsed selection directionality in determining edge', async () => {
+		// Bug: When a user selects a text fragment in a forward direction by
+		// holding shift and, while continuing to hold shift, presses ArrowUp
+		// or ArrowLeft, no attempt should be made to expand block selection.
+		await clickBlockAppender();
+		await page.keyboard.type( 'As expected' );
+
+		// Move caret before "expected" and create a forward selection.
+		await pressKeyTimes( 'ArrowLeft', 8 );
+		await page.keyboard.down( 'Shift' );
+		await pressKeyTimes( 'ArrowRight', 8 );
+
+		// Verify ArrowUp behavior should simply collapse the selection, by
+		// confirming that a continuation of typing would occur at the non-
+		// collapsed selection preceding "expected".
+		await page.keyboard.press( 'ArrowUp' );
+		await page.keyboard.up( 'Shift' );
+		await page.keyboard.type( 'I ' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+
+		// Variation: In a paragraph consisting of multiple lines of text, it
+		// should similarly progressively collapse selection when pressing
+		// Shift+Arrow in the reverse while multiple lines are selected.
+		await insertBlock( 'Paragraph' );
+		await page.keyboard.type( 'A' );
+		await pressKeyWithModifier( 'shift', 'Enter' );
+		await page.keyboard.type( 'B' );
+		await pressKeyWithModifier( 'shift', 'Enter' );
+		await page.keyboard.type( 'C' );
+
+		// Create selection, this time in reverse (assure adequate coverage for
+		// both directions of selection). The caret at the time of selection
+		// should be after the "C" of the last line. Thus, Shift+ArrowUp would
+		// select to the end of the prior line of text.
+		await page.keyboard.down( 'Shift' );
+		await page.keyboard.press( 'ArrowUp' );
+		await page.keyboard.press( 'ArrowUp' );
+
+		// Verify ArrowDown behavior should collapse the selection to the
+		// second line. First collapse while Shift is held should collapse to
+		// the second line. Verify by uncollapsing selection at the extent (by
+		// pressing ArrowLeft) and typing where it should be at second line.
+		await page.keyboard.press( 'ArrowDown' );
+		await page.keyboard.up( 'Shift' );
+		await page.keyboard.press( 'ArrowLeft' );
+		await page.keyboard.type( '- After B' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
 } );

packages/editor/src/components/writing-flow/index.js

@@ -16,6 +16,7 @@ import {
 	placeCaretAtHorizontalEdge,
 	placeCaretAtVerticalEdge,
 	isEntirelySelected,
+	isSelectionForward,
 } from '@wordpress/dom';
 import { UP, DOWN, LEFT, RIGHT, isKeyboardEvent } from '@wordpress/keycodes';
 import { withSelect, withDispatch } from '@wordpress/data';
@@ -281,13 +282,41 @@ class WritingFlow extends Component {
 			this.verticalRect = computeCaretRect( target );
 		}
 
-		if ( isShift && ( hasMultiSelection || (
-			this.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.
-			this.expandSelection( isReverse );
-			event.preventDefault();
+		if ( isShift ) {
+			/**
+			 * Returns true if the current selection should be considered at
+			 * the navigable edge, considering that Shift modifier is held.
+			 *
+			 * @return {boolean} Whether selection is at navigable edge.
+			 */
+			const isSelectionAtShiftEdge = () => {
+				// For a non-collapsed selection, consider that the direction
+				// of the selection should align with the direction of the key
+				// considered for it to be considered at the edge.
+				//
+				// For example, if holding Shift while a forward selection has
+				// been made, pressing ArrowUp should inherit default browser
+				// behavior, _not_ expand the block selection.
+				const selection = getSelection();
+				if (
+					! selection.isCollapsed &&
+					isSelectionForward( selection ) !== ! isReverse
+				) {
+					return false;
+				}
+
+				return (
+					this.isTabbableEdge( target, isReverse ) &&
+					isNavEdge( target, isReverse )
+				);
+			};
+
+			if ( hasMultiSelection || isSelectionAtShiftEdge() ) {
+				// Shift key is down, and there is multi selection or we're at
+				// the end of the current block.
+				this.expandSelection( isReverse );
+				event.preventDefault();
+			}
 		} else if ( hasMultiSelection ) {
 			// Moving from block multi-selection to single block selection
 			this.moveSelection( isReverse );