From 3a0aac36903e01b9551ace3d6114961324fc25f4 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew@andrewduthie.com>
Date: Thu, 24 May 2018 12:09:01 -0400
Subject: [PATCH] Writing Flow: Force tab stop at focusable wrapper when inner
 blocks exist

---
 editor/components/writing-flow/index.js       | 16 ++++-
 editor/utils/dom.js                           | 12 ++++
 editor/utils/test/dom.js                      | 37 ++++++++++
 .../__snapshots__/writing-flow.test.js.snap   | 23 +++++++
 test/e2e/specs/writing-flow.test.js           | 67 +++++++++++++++++++
 5 files changed, 153 insertions(+), 2 deletions(-)
 create mode 100644 editor/utils/test/dom.js
 create mode 100644 test/e2e/specs/__snapshots__/writing-flow.test.js.snap
 create mode 100644 test/e2e/specs/writing-flow.test.js

diff --git a/editor/components/writing-flow/index.js b/editor/components/writing-flow/index.js
index abc3f24f78003a..57136104602de9 100644
--- a/editor/components/writing-flow/index.js
+++ b/editor/components/writing-flow/index.js
@@ -27,6 +27,7 @@ import './style.scss';
 import {
 	isBlockFocusStop,
 	isInSameBlock,
+	hasInnerBlocksContext,
 } from '../../utils/dom';
 
 /**
@@ -105,11 +106,22 @@ class WritingFlow extends Component {
 				return false;
 			}
 
-			// Prefer text fields, but settle for block focus stop.
-			if ( ! isTextField( node ) && ! isBlockFocusStop( node ) ) {
+			// Prefer text fields...
+			if ( isTextField( node ) ) {
+				return true;
+			}
+
+			// ...but settle for block focus stop.
+			if ( ! isBlockFocusStop( node ) ) {
 				return false;
 			}
 
+			// If element contains inner blocks, stop immediately at its focus
+			// wrapper.
+			if ( hasInnerBlocksContext( node ) ) {
+				return true;
+			}
+
 			// If navigating out of a block (in reverse), don't consider its
 			// block focus stop.
 			if ( node.contains( target ) ) {
diff --git a/editor/utils/dom.js b/editor/utils/dom.js
index 28f27a9d51e1af..6fc736ccdb9e2d 100644
--- a/editor/utils/dom.js
+++ b/editor/utils/dom.js
@@ -52,3 +52,15 @@ export function isBlockFocusStop( element ) {
 export function isInSameBlock( a, b ) {
 	return a.closest( '[data-block]' ) === b.closest( '[data-block]' );
 }
+
+/**
+ * Returns true if the given HTMLElement contains inner blocks (an InnerBlocks
+ * element).
+ *
+ * @param {HTMLElement} element Element to test.
+ *
+ * @return {boolean} Whether element contains inner blocks.
+ */
+export function hasInnerBlocksContext( element ) {
+	return !! element.querySelector( '.editor-block-list__layout' );
+}
diff --git a/editor/utils/test/dom.js b/editor/utils/test/dom.js
new file mode 100644
index 00000000000000..69be43576373f3
--- /dev/null
+++ b/editor/utils/test/dom.js
@@ -0,0 +1,37 @@
+/**
+ * Internal dependencies
+ */
+import { hasInnerBlocksContext } from '../dom';
+
+describe( 'hasInnerBlocksContext()', () => {
+	it( 'should return false for a block node which has no inner blocks', () => {
+		const wrapper = document.createElement( 'div' );
+		wrapper.innerHTML = (
+			'<div class="editor-block-list__block" data-type="core/paragraph" tabindex="0">' +
+			'	<div class="editor-block-list__block-edit" aria-label="Block: Paragraph">' +
+			'		<p contenteditable="true">This is a test.</p>' +
+			'	</div>' +
+			'</div>'
+		);
+
+		const blockNode = wrapper.firstChild;
+		expect( hasInnerBlocksContext( blockNode ) ).toBe( false );
+	} );
+
+	it( 'should return true for a block node which contains inner blocks', () => {
+		const wrapper = document.createElement( 'div' );
+		wrapper.innerHTML = (
+			'<div class="editor-block-list__block" data-type="core/columns" tabindex="0">' +
+			'	<div class="editor-block-list__block-edit" aria-label="Block: Columns (beta)">' +
+			'		<div class="wp-block-columns has-2-columns">' +
+			'			<div class="editor-block-list__layout layout-column-1"></div>' +
+			'			<div class="editor-block-list__layout layout-column-2"></div>' +
+			'		</div>' +
+			'	</div>' +
+			'</div>'
+		);
+
+		const blockNode = wrapper.firstChild;
+		expect( hasInnerBlocksContext( blockNode ) ).toBe( true );
+	} );
+} );
diff --git a/test/e2e/specs/__snapshots__/writing-flow.test.js.snap b/test/e2e/specs/__snapshots__/writing-flow.test.js.snap
new file mode 100644
index 00000000000000..e9ad304b10799f
--- /dev/null
+++ b/test/e2e/specs/__snapshots__/writing-flow.test.js.snap
@@ -0,0 +1,23 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`adding blocks Should navigate inner blocks with arrow keys 1`] = `
+"<!-- wp:paragraph -->
+<p>First paragraph</p>
+<!-- /wp:paragraph -->
+
+<!-- wp:columns -->
+<div class=\\"wp-block-columns has-2-columns\\">
+	<!-- wp:paragraph {\\"layout\\":\\"column-1\\"} -->
+	<p class=\\"layout-column-1\\">First column paragraph</p>
+	<!-- /wp:paragraph -->
+
+	<!-- wp:paragraph {\\"layout\\":\\"column-2\\"} -->
+	<p class=\\"layout-column-2\\">Second column paragraph</p>
+	<!-- /wp:paragraph -->
+</div>
+<!-- /wp:columns -->
+
+<!-- wp:paragraph -->
+<p>Second paragraph</p>
+<!-- /wp:paragraph -->"
+`;
diff --git a/test/e2e/specs/writing-flow.test.js b/test/e2e/specs/writing-flow.test.js
new file mode 100644
index 00000000000000..70665c8e2f1921
--- /dev/null
+++ b/test/e2e/specs/writing-flow.test.js
@@ -0,0 +1,67 @@
+/**
+ * Internal dependencies
+ */
+import '../support/bootstrap';
+import {
+	newPost,
+	newDesktopBrowserPage,
+	getHTMLFromCodeEditor,
+} from '../support/utils';
+
+describe( 'adding blocks', () => {
+	beforeAll( async () => {
+		await newDesktopBrowserPage();
+	} );
+
+	beforeEach( async () => {
+		await newPost();
+	} );
+
+	it( 'Should navigate inner blocks with arrow keys', async () => {
+		let activeElementText;
+
+		// Add demo content
+		await page.click( '.editor-default-block-appender__content' );
+		await page.keyboard.type( 'First paragraph' );
+		await page.keyboard.press( 'Enter' );
+		await page.keyboard.type( '/columns' );
+		await page.keyboard.press( 'Enter' );
+		await page.keyboard.type( 'First column paragraph' );
+
+		// Arrow down should navigate through layouts in columns block (to
+		// its default appender).
+		await page.keyboard.press( 'ArrowDown' );
+		await page.keyboard.type( 'Second column paragraph' );
+
+		// Arrow down from last of layouts exits nested context to default
+		// appender of root level.
+		await page.keyboard.press( 'ArrowDown' );
+		await page.keyboard.type( 'Second paragraph' );
+
+		// Arrow up into nested context focuses last text input
+		await page.keyboard.press( 'ArrowUp' );
+		activeElementText = await page.evaluate( () => document.activeElement.textContent );
+		expect( activeElementText ).toBe( 'Second column paragraph' );
+
+		// Arrow up in inner blocks should navigate through text fields.
+		await page.keyboard.press( 'ArrowUp' );
+		activeElementText = await page.evaluate( () => document.activeElement.textContent );
+		expect( activeElementText ).toBe( 'First column paragraph' );
+
+		// Arrow up from first text field in nested context focuses wrapper
+		// before escaping out.
+		await page.keyboard.press( 'ArrowUp' );
+		const activeElementBlockType = await page.evaluate( () => (
+			document.activeElement.getAttribute( 'data-type' )
+		) );
+		expect( activeElementBlockType ).toBe( 'core/columns' );
+
+		// Arrow up from focused (columns) block wrapper exits nested context
+		// to prior text input.
+		await page.keyboard.press( 'ArrowUp' );
+		activeElementText = await page.evaluate( () => document.activeElement.textContent );
+		expect( activeElementText ).toBe( 'First paragraph' );
+
+		expect( await getHTMLFromCodeEditor() ).toMatchSnapshot();
+	} );
+} );