Writing flow: fix vertical arrow nav in table (and generally grid) (#…

…22105)

* Writing flow: fix arrow nav in table (and generally gird)

* Add e2e test

* fix columns e2e test

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

@@ -93,10 +93,18 @@ export function isNavigationCandidate( element, keyCode, hasModifier ) {
  * @param {Element} target           Currently focused text field.
  * @param {boolean} isReverse        True if considering as the first field.
  * @param {Element} containerElement Element containing all blocks.
+ * @param {boolean} onlyVertical     Wether to only consider tabbable elements
+ *                                   that are visually above or under the
+ *                                   target.
  *
  * @return {?Element} Optimal tab target, if one exists.
  */
-export function getClosestTabbable( target, isReverse, containerElement ) {
+export function getClosestTabbable(
+	target,
+	isReverse,
+	containerElement,
+	onlyVertical
+) {
 	// Since the current focus target is not guaranteed to be a text field,
 	// find all focusables. Tabbability is considered later.
 	let focusableNodes = focus.focusable.find( containerElement );
@@ -112,12 +120,29 @@ export function getClosestTabbable( target, isReverse, containerElement ) {
 		focusableNodes.indexOf( target ) + 1
 	);
 
+	let targetRect;
+
+	if ( onlyVertical ) {
+		targetRect = target.getBoundingClientRect();
+	}
+
 	function isTabCandidate( node, i, array ) {
 		// Not a candidate if the node is not tabbable.
 		if ( ! focus.tabbable.isTabbableIndex( node ) ) {
 			return false;
 		}
 
+		if ( onlyVertical ) {
+			const nodeRect = node.getBoundingClientRect();
+
+			if (
+				nodeRect.left >= targetRect.right ||
+				nodeRect.right <= targetRect.left
+			) {
+				return false;
+			}
+		}
+
 		// Prefer text fields...
 		if ( isTextField( node ) ) {
 			return true;
@@ -517,7 +542,8 @@ export default function WritingFlow( { children } ) {
 			const closestTabbable = getClosestTabbable(
 				target,
 				isReverse,
-				container.current
+				container.current,
+				true
 			);
 
 			if ( closestTabbable ) {

packages/e2e-tests/specs/editor/blocks/__snapshots__/table.test.js.snap

@@ -53,3 +53,9 @@ exports[`Table displays a form for choosing the row and column count of the tabl
 <figure class=\\"wp-block-table\\"><table><tbody><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr><tr><td></td><td></td><td></td><td></td><td></td></tr></tbody></table></figure>
 <!-- /wp:table -->"
 `;
+
+exports[`Table up and down arrow navigation 1`] = `
+"<!-- wp:table -->
+<figure class=\\"wp-block-table\\"><table><tbody><tr><td>1</td><td>4</td></tr><tr><td>2</td><td>3</td></tr></tbody></table></figure>
+<!-- /wp:table -->"
+`;

packages/e2e-tests/specs/editor/blocks/table.test.js

@@ -267,4 +267,22 @@ describe( 'Table', () => {
 
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
+
+	it( 'up and down arrow navigation', async () => {
+		await insertBlock( 'Table' );
+
+		// Create the table.
+		await clickButton( createButtonLabel );
+
+		await page.keyboard.press( 'Tab' );
+		await page.keyboard.type( '1' );
+		await page.keyboard.press( 'ArrowDown' );
+		await page.keyboard.type( '2' );
+		await page.keyboard.press( 'ArrowRight' );
+		await page.keyboard.type( '3' );
+		await page.keyboard.press( 'ArrowUp' );
+		await page.keyboard.type( '4' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
 } );

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

@@ -80,25 +80,7 @@ describe( 'Writing Flow', () => {
 		activeBlockName = await getActiveBlockName();
 		expect( activeBlockName ).toBe( 'core/column' );
 		await page.keyboard.press( 'ArrowUp' );
-		activeBlockName = await getActiveBlockName();
-		expect( activeBlockName ).toBe( 'core/paragraph' );
-		activeElementText = await page.evaluate(
-			() => document.activeElement.textContent
-		);
-		expect( activeElementText ).toBe( '1st col' );
-
-		// Arrow up from first text field in nested context focuses column and
-		// columns wrappers before escaping out.
-		let activeElementBlockType;
-		await page.keyboard.press( 'ArrowUp' );
-		activeElementBlockType = await page.evaluate( () =>
-			document.activeElement.getAttribute( 'data-type' )
-		);
-		expect( activeElementBlockType ).toBe( 'core/column' );
-		activeBlockName = await getActiveBlockName();
-		expect( activeBlockName ).toBe( 'core/column' );
-		await page.keyboard.press( 'ArrowUp' );
-		activeElementBlockType = await page.evaluate( () =>
+		const activeElementBlockType = await page.evaluate( () =>
 			document.activeElement.getAttribute( 'data-type' )
 		);
 		expect( activeElementBlockType ).toBe( 'core/columns' );