From 160067a1456b5df8474e8981fefa54be39ca770c Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 14 Jan 2022 02:49:48 -0500 Subject: [PATCH 01/33] Finally fix selector in test I think. --- packages/e2e-tests/specs/editor/various/writing-flow.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 14a1a142daa687..244314396fb383 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,7 +26,7 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( ':focus [aria-label="Two columns; equal split"]' ); + await page.click( 'button[aria-label="Two columns; equal split"]' ); await page.click( ':focus .block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); From 12514083e23de020d007e25b8bdcc99e27bdba58 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 00:50:07 -0500 Subject: [PATCH 02/33] If not contenteditable Block, find next focussable element. --- .../use-block-props/use-focus-first-element.js | 9 +++++++++ .../e2e-tests/specs/editor/various/writing-flow.test.js | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) 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 3d9a05bb2bdfac..f900bd127b1624 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 @@ -96,6 +96,15 @@ export function useFocusFirstElement( clientId ) { return; } + // Check to see if Block contains focussable element before a generic caret insert. + if ( ! ref.current.getAttribute( 'contenteditable' ) ) { + const focusElements = focus.tabbable.findNext( ref.current ); + if ( focusElements ) { + focusElements.focus(); + return; + } + } + setContentEditableWrapper( ref.current, false ); placeCaretAtHorizontalEdge( target, isReverse ); diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 244314396fb383..14a1a142daa687 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,7 +26,7 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( 'button[aria-label="Two columns; equal split"]' ); + await page.click( ':focus [aria-label="Two columns; equal split"]' ); await page.click( ':focus .block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); From e735aa31fed98f09f355552fca661c7a1543f11c Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 02:07:43 -0500 Subject: [PATCH 03/33] Try to make tests stable. --- packages/e2e-tests/specs/editor/various/writing-flow.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 14a1a142daa687..244314396fb383 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,7 +26,7 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( ':focus [aria-label="Two columns; equal split"]' ); + await page.click( 'button[aria-label="Two columns; equal split"]' ); await page.click( ':focus .block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); From 3c6311d18abb17c2f315c36b1702107861fe7873 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 02:50:50 -0500 Subject: [PATCH 04/33] Try to fix tests. --- packages/e2e-tests/specs/editor/various/writing-flow.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 244314396fb383..c295b9e53f5a9c 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -27,7 +27,7 @@ const addParagraphsAndColumnsDemo = async () => { ); await page.keyboard.press( 'Enter' ); await page.click( 'button[aria-label="Two columns; equal split"]' ); - await page.click( ':focus .block-editor-button-block-appender' ); + await page.click( 'button.block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); await pressKeyTimes( 'Tab', 2 ); // Tab to paragraph result. From f18270d46f499f6fa3db023eb1489d649f8d59fd Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 09:09:09 -0500 Subject: [PATCH 05/33] Update snapshot. --- .../various/__snapshots__/writing-flow.test.js.snap | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 1b19d10c91b3ae..407fcff1297c18 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -278,6 +278,12 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` " `; +exports[`Writing Flow should only consider the content as one tab stop 1`] = ` +" +
2
+" +`; + exports[`Writing Flow should preserve horizontal position when navigating vertically between blocks 1`] = ` "

abc

@@ -297,9 +303,3 @@ exports[`Writing Flow should remember initial vertical position 1`] = `


2

" `; - -exports[`Writing Flow should only consider the content as one tab stop 1`] = ` -" -
2
-" -`; From 8f352e8552e365eb3131c74ad1287127880e715c Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 16:40:19 -0500 Subject: [PATCH 06/33] Fix focus getting stuck on child Block wrapper. Adjust to find only specific whitelisted element. --- .../use-block-props/use-focus-first-element.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 f900bd127b1624..d78d9678c3689e 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 @@ -98,7 +98,16 @@ export function useFocusFirstElement( clientId ) { // Check to see if Block contains focussable element before a generic caret insert. if ( ! ref.current.getAttribute( 'contenteditable' ) ) { - const focusElements = focus.tabbable.findNext( ref.current ); + const whitelistTags = [ 'a', 'button' ]; + let focusElements = focus.tabbable.findNext( ref.current ); + // In the event a Block has children, focus will find the next element with tabindex which would happen to be the wrapping element of a child Block. This ensures we only focus on whitelisted tag such as link/button. + do { + focusElements = focus.tabbable.findNext( focusElements ); + } while ( + ! whitelistTags.some( + ( tag ) => focusElements.tagName.toLowerCase() === tag + ) + ); if ( focusElements ) { focusElements.focus(); return; From b947e8cb9996a74dc585efe0270248786c728788 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 20:04:31 -0500 Subject: [PATCH 07/33] Revert tests and start fresh. --- .../various/__snapshots__/writing-flow.test.js.snap | 12 ++++++------ .../specs/editor/various/writing-flow.test.js | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 407fcff1297c18..1b19d10c91b3ae 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -278,12 +278,6 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` " `; -exports[`Writing Flow should only consider the content as one tab stop 1`] = ` -" -
2
-" -`; - exports[`Writing Flow should preserve horizontal position when navigating vertically between blocks 1`] = ` "

abc

@@ -303,3 +297,9 @@ exports[`Writing Flow should remember initial vertical position 1`] = `


2

" `; + +exports[`Writing Flow should only consider the content as one tab stop 1`] = ` +" +
2
+" +`; diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index c295b9e53f5a9c..14a1a142daa687 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,8 +26,8 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( 'button[aria-label="Two columns; equal split"]' ); - await page.click( 'button.block-editor-button-block-appender' ); + await page.click( ':focus [aria-label="Two columns; equal split"]' ); + await page.click( ':focus .block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); await pressKeyTimes( 'Tab', 2 ); // Tab to paragraph result. From 6ee7747757214e5d02b11b81e0148c009313eb74 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 31 Jan 2022 22:59:47 -0500 Subject: [PATCH 08/33] More work on tests. --- .../various/__snapshots__/writing-flow.test.js.snap | 8 ++------ .../e2e-tests/specs/editor/various/writing-flow.test.js | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 1b19d10c91b3ae..9eb548044357a2 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -278,6 +278,8 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` " `; +exports[`Writing Flow should only consider the content as one tab stop 1`] = `""`; + exports[`Writing Flow should preserve horizontal position when navigating vertically between blocks 1`] = ` "

abc

@@ -297,9 +299,3 @@ exports[`Writing Flow should remember initial vertical position 1`] = `


2

" `; - -exports[`Writing Flow should only consider the content as one tab stop 1`] = ` -" -
2
-" -`; diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 14a1a142daa687..c295b9e53f5a9c 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,8 +26,8 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( ':focus [aria-label="Two columns; equal split"]' ); - await page.click( ':focus .block-editor-button-block-appender' ); + await page.click( 'button[aria-label="Two columns; equal split"]' ); + await page.click( 'button.block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); await pressKeyTimes( 'Tab', 2 ); // Tab to paragraph result. From 818115e788346d46f23f13946f5724bcddf99d62 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 1 Feb 2022 17:28:41 -0500 Subject: [PATCH 09/33] Add td to whitelisted elements. Check to see if first element has focus potential. Ensure the element to focus is within the current Block. Try to make tests stable. --- .../use-block-props/use-focus-first-element.js | 15 ++++++++++++--- .../__snapshots__/writing-flow.test.js.snap | 6 +++++- .../specs/editor/various/writing-flow.test.js | 5 ----- 3 files changed, 17 insertions(+), 9 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 d78d9678c3689e..eefe48825b33a8 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 @@ -98,11 +98,20 @@ export function useFocusFirstElement( clientId ) { // Check to see if Block contains focussable element before a generic caret insert. if ( ! ref.current.getAttribute( 'contenteditable' ) ) { - const whitelistTags = [ 'a', 'button' ]; - let focusElements = focus.tabbable.findNext( ref.current ); + const whitelistTags = [ 'a', 'button', 'td' ]; + let focusElements; + let firstFocusElements = true; // In the event a Block has children, focus will find the next element with tabindex which would happen to be the wrapping element of a child Block. This ensures we only focus on whitelisted tag such as link/button. do { - focusElements = focus.tabbable.findNext( focusElements ); + // The first element may be okay to focus. Go ahead and check. + if ( firstFocusElements ) { + focusElements = focus.tabbable.findNext( ref.current ); + firstFocusElements = false; + } else { + focusElements = focus.tabbable.findNext( focusElements ); + } + // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. + if ( ! ref.current.contains( focusElements ) ) break; } while ( ! whitelistTags.some( ( tag ) => focusElements.tagName.toLowerCase() === tag diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 9eb548044357a2..71943003cf923f 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -278,7 +278,11 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` " `; -exports[`Writing Flow should only consider the content as one tab stop 1`] = `""`; +exports[`Writing Flow should only consider the content as one tab stop 1`] = ` +" +
2
+" +`; exports[`Writing Flow should preserve horizontal position when navigating vertically between blocks 1`] = ` " diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index c295b9e53f5a9c..6fe2f4e0ef4bc0 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -666,11 +666,6 @@ describe( 'Writing Flow', () => { await page.keyboard.press( 'Enter' ); await page.keyboard.type( '/table' ); await page.keyboard.press( 'Enter' ); - // Move into the placeholder UI. - await page.keyboard.press( 'ArrowDown' ); - // Tab to the "Create table" button. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Tab' ); // Create the table. await page.keyboard.press( 'Space' ); // Return focus after focus loss. This should be fixed. From 21e15dd15b81698833a381aa8c6b3c0e1aa5c6b6 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 1 Feb 2022 22:54:15 -0500 Subject: [PATCH 10/33] Revert some code. --- .../use-focus-first-element.js | 24 ++++--------------- .../__snapshots__/writing-flow.test.js.snap | 12 +++++----- .../specs/editor/various/writing-flow.test.js | 9 +++++-- 3 files changed, 18 insertions(+), 27 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 eefe48825b33a8..68a4e0a39fbddf 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 @@ -98,25 +98,11 @@ export function useFocusFirstElement( clientId ) { // Check to see if Block contains focussable element before a generic caret insert. if ( ! ref.current.getAttribute( 'contenteditable' ) ) { - const whitelistTags = [ 'a', 'button', 'td' ]; - let focusElements; - let firstFocusElements = true; - // In the event a Block has children, focus will find the next element with tabindex which would happen to be the wrapping element of a child Block. This ensures we only focus on whitelisted tag such as link/button. - do { - // The first element may be okay to focus. Go ahead and check. - if ( firstFocusElements ) { - focusElements = focus.tabbable.findNext( ref.current ); - firstFocusElements = false; - } else { - focusElements = focus.tabbable.findNext( focusElements ); - } - // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. - if ( ! ref.current.contains( focusElements ) ) break; - } while ( - ! whitelistTags.some( - ( tag ) => focusElements.tagName.toLowerCase() === tag - ) - ); + const focusElements = focus.tabbable.findNext( ref.current ); + // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. + if ( ! ref.current.contains( focusElements ) ) { + return; + } if ( focusElements ) { focusElements.focus(); return; diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 71943003cf923f..1b19d10c91b3ae 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -278,12 +278,6 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` " `; -exports[`Writing Flow should only consider the content as one tab stop 1`] = ` -" -
2
-" -`; - exports[`Writing Flow should preserve horizontal position when navigating vertically between blocks 1`] = ` "

abc

@@ -303,3 +297,9 @@ exports[`Writing Flow should remember initial vertical position 1`] = `


2

" `; + +exports[`Writing Flow should only consider the content as one tab stop 1`] = ` +" +
2
+" +`; diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 6fe2f4e0ef4bc0..14a1a142daa687 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,8 +26,8 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( 'button[aria-label="Two columns; equal split"]' ); - await page.click( 'button.block-editor-button-block-appender' ); + await page.click( ':focus [aria-label="Two columns; equal split"]' ); + await page.click( ':focus .block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); await pressKeyTimes( 'Tab', 2 ); // Tab to paragraph result. @@ -666,6 +666,11 @@ describe( 'Writing Flow', () => { await page.keyboard.press( 'Enter' ); await page.keyboard.type( '/table' ); await page.keyboard.press( 'Enter' ); + // Move into the placeholder UI. + await page.keyboard.press( 'ArrowDown' ); + // Tab to the "Create table" button. + await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); // Create the table. await page.keyboard.press( 'Space' ); // Return focus after focus loss. This should be fixed. From e3f373b573c9759f1cc7b806c0d5ecd284a120ed Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 2 Feb 2022 01:14:25 -0500 Subject: [PATCH 11/33] Back to test fixing. --- .../various/__snapshots__/writing-flow.test.js.snap | 12 ++++++------ .../specs/editor/various/writing-flow.test.js | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 1b19d10c91b3ae..407fcff1297c18 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -278,6 +278,12 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` " `; +exports[`Writing Flow should only consider the content as one tab stop 1`] = ` +" +
2
+" +`; + exports[`Writing Flow should preserve horizontal position when navigating vertically between blocks 1`] = ` "

abc

@@ -297,9 +303,3 @@ exports[`Writing Flow should remember initial vertical position 1`] = `


2

" `; - -exports[`Writing Flow should only consider the content as one tab stop 1`] = ` -" -
2
-" -`; diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 14a1a142daa687..df94913912c0ec 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -26,8 +26,8 @@ const addParagraphsAndColumnsDemo = async () => { `//*[contains(@class, "components-autocomplete__result") and contains(@class, "is-selected") and contains(text(), 'Columns')]` ); await page.keyboard.press( 'Enter' ); - await page.click( ':focus [aria-label="Two columns; equal split"]' ); - await page.click( ':focus .block-editor-button-block-appender' ); + await page.click( 'button[aria-label="Two columns; equal split"]' ); + await page.click( '.block-editor-button-block-appender' ); await page.waitForSelector( '.block-editor-inserter__search input:focus' ); await page.keyboard.type( 'Paragraph' ); await pressKeyTimes( 'Tab', 2 ); // Tab to paragraph result. From 038d1b886c13f17bb60f849d6f96ef0421187f10 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 2 Feb 2022 18:32:27 -0500 Subject: [PATCH 12/33] Try again to prevent focus in child Blocks. Tests should pass this time, maybe. --- .../use-focus-first-element.js | 16 +++++++++--- .../various/toolbar-roving-tabindex.test.js | 25 ++++++++----------- 2 files changed, 22 insertions(+), 19 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 68a4e0a39fbddf..dbd503975509e1 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 @@ -98,13 +98,21 @@ export function useFocusFirstElement( clientId ) { // Check to see if Block contains focussable element before a generic caret insert. if ( ! ref.current.getAttribute( 'contenteditable' ) ) { - const focusElements = focus.tabbable.findNext( ref.current ); + let focusElement = focus.tabbable.findNext( ref.current ); // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. - if ( ! ref.current.contains( focusElements ) ) { + if ( ! ref.current.contains( focusElement ) ) { return; } - if ( focusElements ) { - focusElements.focus(); + // Need to try our best to detect child Blocks and not focus them straight away. + const disallowedTags = [ 'div', 'figure' ]; + const checkDisallowedTags = disallowedTags.some( + ( tag ) => focusElement.tagName.toLowerCase() === tag + ); + if ( checkDisallowedTags ) { + focusElement = false; + } + if ( focusElement ) { + focusElement.focus(); return; } } diff --git a/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js b/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js index 485d36089dd172..61cbf9ffd577d0 100644 --- a/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js +++ b/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js @@ -20,18 +20,16 @@ async function expectLabelToHaveFocus( label ) { ).toBe( label ); } -async function testBlockToolbarKeyboardNavigation( - currentBlockLabel, - currentBlockTitle -) { +async function testBlockToolbarKeyboardNavigation( currentBlockTitle ) { await focusBlockToolbar(); await expectLabelToHaveFocus( currentBlockTitle ); await page.keyboard.press( 'ArrowRight' ); await expectLabelToHaveFocus( 'Move up' ); await page.keyboard.press( 'Tab' ); - await expectLabelToHaveFocus( currentBlockLabel ); await pressKeyWithModifier( 'shift', 'Tab' ); await expectLabelToHaveFocus( 'Move up' ); + await page.keyboard.press( 'ArrowLeft' ); + await expectLabelToHaveFocus( currentBlockTitle ); } async function wrapCurrentBlockWithGroup( currentBlockTitle ) { @@ -64,10 +62,7 @@ describe( 'Toolbar roving tabindex', () => { it( 'ensures paragraph block toolbar uses roving tabindex', async () => { await insertBlock( 'Paragraph' ); await page.keyboard.type( 'Paragraph' ); - await testBlockToolbarKeyboardNavigation( - 'Paragraph block', - 'Paragraph' - ); + await testBlockToolbarKeyboardNavigation( 'Paragraph' ); await wrapCurrentBlockWithGroup( 'Paragraph' ); await testGroupKeyboardNavigation( 'Paragraph block', 'Paragraph' ); } ); @@ -75,7 +70,7 @@ describe( 'Toolbar roving tabindex', () => { it( 'ensures heading block toolbar uses roving tabindex', async () => { await insertBlock( 'Heading' ); await page.keyboard.type( 'Heading' ); - await testBlockToolbarKeyboardNavigation( 'Block: Heading', 'Heading' ); + await testBlockToolbarKeyboardNavigation( 'Heading' ); await wrapCurrentBlockWithGroup( 'Heading' ); await testGroupKeyboardNavigation( 'Block: Heading', 'Heading' ); } ); @@ -83,34 +78,34 @@ describe( 'Toolbar roving tabindex', () => { it( 'ensures list block toolbar uses roving tabindex', async () => { await insertBlock( 'List' ); await page.keyboard.type( 'List' ); - await testBlockToolbarKeyboardNavigation( 'Block: List', 'List' ); + await testBlockToolbarKeyboardNavigation( 'List' ); await wrapCurrentBlockWithGroup( 'List' ); await testGroupKeyboardNavigation( 'Block: List', 'List' ); } ); it( 'ensures image block toolbar uses roving tabindex', async () => { await insertBlock( 'Image' ); - await testBlockToolbarKeyboardNavigation( 'Block: Image', 'Image' ); + await testBlockToolbarKeyboardNavigation( 'Image' ); await wrapCurrentBlockWithGroup( 'Image' ); await testGroupKeyboardNavigation( 'Block: Image', 'Image' ); } ); it( 'ensures table block toolbar uses roving tabindex', async () => { await insertBlock( 'Table' ); - await testBlockToolbarKeyboardNavigation( 'Block: Table', 'Table' ); + await testBlockToolbarKeyboardNavigation( 'Table' ); // Move focus to the first toolbar item. await page.keyboard.press( 'Home' ); await expectLabelToHaveFocus( 'Table' ); await page.click( '.blocks-table__placeholder-button' ); await page.keyboard.press( 'Tab' ); - await testBlockToolbarKeyboardNavigation( 'Body cell text', 'Table' ); + await expectLabelToHaveFocus( 'Body cell text' ); await wrapCurrentBlockWithGroup( 'Table' ); await testGroupKeyboardNavigation( 'Block: Table', 'Table' ); } ); it( 'ensures custom html block toolbar uses roving tabindex', async () => { await insertBlock( 'Custom HTML' ); - await testBlockToolbarKeyboardNavigation( 'HTML', 'Custom HTML' ); + await testBlockToolbarKeyboardNavigation( 'Custom HTML' ); await wrapCurrentBlockWithGroup( 'Custom HTML' ); await testGroupKeyboardNavigation( 'Block: Custom HTML', From be095b8a112221cd626ce16ef7b2eb6372794b1e Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 2 Feb 2022 23:03:42 -0500 Subject: [PATCH 13/33] Update another snapshot. --- .../various/__snapshots__/multi-block-selection.test.js.snap | 2 +- 1 file changed, 1 insertion(+), 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 3ede4a98a20c0b..1c85d83cb4b1d2 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 @@ -116,7 +116,7 @@ exports[`Multi-block selection should not multi select single block 1`] = ` exports[`Multi-block selection should only trigger multi-selection when at the end 1`] = ` " -

1.

+

4.

From fbffdf5fde6400312089789d420d2a0511a7bfb5 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 16 Feb 2022 14:34:44 -0500 Subject: [PATCH 14/33] More work on E2E tests. --- packages/e2e-tests/specs/editor/various/block-deletion.test.js | 1 + packages/e2e-tests/specs/editor/various/list-view.test.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/block-deletion.test.js b/packages/e2e-tests/specs/editor/various/block-deletion.test.js index 62403f8f40b1bd..e4497d45fbfaf8 100644 --- a/packages/e2e-tests/specs/editor/various/block-deletion.test.js +++ b/packages/e2e-tests/specs/editor/various/block-deletion.test.js @@ -192,6 +192,7 @@ describe( 'deleting all blocks', () => { // Add and remove a block. await insertBlock( 'Image' ); await page.waitForSelector( 'figure[data-type="core/image"]' ); + await page.keyboard.press( 'ArrowUp' ); await page.keyboard.press( 'Backspace' ); // Verify there is no selected block. diff --git a/packages/e2e-tests/specs/editor/various/list-view.test.js b/packages/e2e-tests/specs/editor/various/list-view.test.js index 7894eb704cfdb4..81de5745693b9f 100644 --- a/packages/e2e-tests/specs/editor/various/list-view.test.js +++ b/packages/e2e-tests/specs/editor/various/list-view.test.js @@ -87,11 +87,12 @@ describe( 'List view', () => { await page.keyboard.press( 'Enter' ); const canvasImageBlock = await page.waitForSelector( - 'figure[aria-label="Block: Image"]' + 'button.block-editor-media-placeholder__upload-button' ); expect( canvasImageBlock ).toHaveFocus(); // Delete the image block in the canvas. + await page.keyboard.press( 'ArrowUp' ); await page.keyboard.press( 'Backspace' ); // List view should have two rows. From c4218343ce2024879dd96da38e485dfe3b3a11d6 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 21 Feb 2022 18:11:55 -0500 Subject: [PATCH 15/33] Cleanup code and try to fix more tests. --- .../use-focus-first-element.js | 7 ++----- .../multi-block-selection.test.js.snap | 20 ++++++++++--------- .../specs/editor/various/list-view.test.js | 6 +++--- 3 files changed, 16 insertions(+), 17 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 dbd503975509e1..f3448652716596 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 @@ -98,7 +98,7 @@ export function useFocusFirstElement( clientId ) { // Check to see if Block contains focussable element before a generic caret insert. if ( ! ref.current.getAttribute( 'contenteditable' ) ) { - let focusElement = focus.tabbable.findNext( ref.current ); + const focusElement = focus.tabbable.findNext( ref.current ); // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. if ( ! ref.current.contains( focusElement ) ) { return; @@ -108,10 +108,7 @@ export function useFocusFirstElement( clientId ) { const checkDisallowedTags = disallowedTags.some( ( tag ) => focusElement.tagName.toLowerCase() === tag ); - if ( checkDisallowedTags ) { - focusElement = false; - } - if ( focusElement ) { + if ( ! checkDisallowedTags && focusElement ) { focusElement.focus(); return; } 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 1c85d83cb4b1d2..25c58b84fdbc35 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 @@ -72,20 +72,22 @@ exports[`Multi-block selection should cut and paste 2`] = ` exports[`Multi-block selection should gradually multi-select 1`] = ` " -
-
+
+
+ + + +
+
+ + +

1

2

-
- - - -
-
-" +" `; exports[`Multi-block selection should gradually multi-select 2`] = ` diff --git a/packages/e2e-tests/specs/editor/various/list-view.test.js b/packages/e2e-tests/specs/editor/various/list-view.test.js index 81de5745693b9f..4cfa7857ea7238 100644 --- a/packages/e2e-tests/specs/editor/various/list-view.test.js +++ b/packages/e2e-tests/specs/editor/various/list-view.test.js @@ -86,10 +86,10 @@ describe( 'List view', () => { // Select the image block in the canvas. await page.keyboard.press( 'Enter' ); - const canvasImageBlock = await page.waitForSelector( - 'button.block-editor-media-placeholder__upload-button' + const uploadButton = await page.waitForXPath( + '//button[contains( text(), "Upload" ) ]' ); - expect( canvasImageBlock ).toHaveFocus(); + expect( uploadButton ).toHaveFocus(); // Delete the image block in the canvas. await page.keyboard.press( 'ArrowUp' ); From 5007b54b5e38449714b68553afeecd8689e8d2e3 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 21 Feb 2022 19:23:47 -0500 Subject: [PATCH 16/33] Fix snapshot. --- .../various/__snapshots__/multi-block-selection.test.js.snap | 2 +- 1 file changed, 1 insertion(+), 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 25c58b84fdbc35..9cb97feb6c068b 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,7 +118,7 @@ exports[`Multi-block selection should not multi select single block 1`] = ` exports[`Multi-block selection should only trigger multi-selection when at the end 1`] = ` " -

4.

+

1.

From cc894c419124f1fbd684cc64557ec6de60b8c89a Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 21 Feb 2022 21:46:27 -0500 Subject: [PATCH 17/33] Maybe fix another snapshot. --- .../__snapshots__/block-hierarchy-navigation.test.js.snap | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap index 75dd24a83136f7..b28efa38cc7d57 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap @@ -13,9 +13,7 @@ exports[`Navigating the block hierarchy should appear and function even without exports[`Navigating the block hierarchy should navigate block hierarchy using only the keyboard 1`] = ` "
-
-

First column

-
+
From 643e470655f7785e111380e8cd60dca6cc3fa470 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 21 Feb 2022 23:36:06 -0500 Subject: [PATCH 18/33] Make sure returning early doesn't prevent false positive of still having a chance to focus. --- .../use-focus-first-element.js | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 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 f3448652716596..0b82f6b435a718 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 @@ -100,17 +100,16 @@ export function useFocusFirstElement( clientId ) { if ( ! ref.current.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( ref.current ); // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. - if ( ! ref.current.contains( focusElement ) ) { - return; - } - // Need to try our best to detect child Blocks and not focus them straight away. - const disallowedTags = [ 'div', 'figure' ]; - const checkDisallowedTags = disallowedTags.some( - ( tag ) => focusElement.tagName.toLowerCase() === tag - ); - if ( ! checkDisallowedTags && focusElement ) { - focusElement.focus(); - return; + if ( ref.current.contains( focusElement ) ) { + // Need to try our best to detect child Blocks and not focus them straight away. + const disallowedTags = [ 'div', 'figure' ]; + const checkDisallowedTags = disallowedTags.some( + ( tag ) => focusElement.tagName.toLowerCase() === tag + ); + if ( ! checkDisallowedTags && focusElement ) { + focusElement.focus(); + return; + } } } From df7bf85718d405e82fbd78877be06377304e428e Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 22 Feb 2022 00:12:50 -0500 Subject: [PATCH 19/33] Update snapshots. --- .../block-hierarchy-navigation.test.js.snap | 4 +++- .../multi-block-selection.test.js.snap | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap index b28efa38cc7d57..75dd24a83136f7 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/block-hierarchy-navigation.test.js.snap @@ -13,7 +13,9 @@ exports[`Navigating the block hierarchy should appear and function even without exports[`Navigating the block hierarchy should navigate block hierarchy using only the keyboard 1`] = ` "
-
+
+

First column

+
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 9cb97feb6c068b..e2c78857d930d4 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 @@ -73,21 +73,19 @@ exports[`Multi-block selection should cut and paste 2`] = ` exports[`Multi-block selection should gradually multi-select 1`] = ` "
-
- - - -
-
- - - +

1

2

-" +
+ + + +
+
+" `; exports[`Multi-block selection should gradually multi-select 2`] = ` @@ -118,7 +116,7 @@ exports[`Multi-block selection should not multi select single block 1`] = ` exports[`Multi-block selection should only trigger multi-selection when at the end 1`] = ` " -

1.

+

4.

From 51d8e7ac1645a67b47636d454e52fd85a0512aea Mon Sep 17 00:00:00 2001 From: tellthemachines Date: Wed, 23 Feb 2022 16:53:05 +1100 Subject: [PATCH 20/33] Fix test snapshot. --- .../various/__snapshots__/multi-block-selection.test.js.snap | 2 +- 1 file changed, 1 insertion(+), 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 e2c78857d930d4..401d953fc222c9 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 @@ -116,7 +116,7 @@ exports[`Multi-block selection should not multi select single block 1`] = ` exports[`Multi-block selection should only trigger multi-selection when at the end 1`] = ` " -

4.

+

1.

From 316b9cc7a2acfb0ea45a878cdb43b05aba6ebad2 Mon Sep 17 00:00:00 2001 From: tellthemachines Date: Mon, 28 Feb 2022 15:34:58 +1100 Subject: [PATCH 21/33] Update tests with focus order changes. --- .../__snapshots__/multi-block-selection.test.js.snap | 10 +++++----- .../various/__snapshots__/writing-flow.test.js.snap | 2 +- .../specs/editor/various/multi-block-selection.test.js | 1 - .../specs/editor/various/writing-flow.test.js | 3 +-- 4 files changed, 7 insertions(+), 9 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 401d953fc222c9..1c85d83cb4b1d2 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 @@ -72,8 +72,8 @@ exports[`Multi-block selection should cut and paste 2`] = ` exports[`Multi-block selection should gradually multi-select 1`] = ` " -
-
+
+

1

@@ -82,8 +82,8 @@ exports[`Multi-block selection should gradually multi-select 1`] = `
- -
+ +
" `; @@ -116,7 +116,7 @@ exports[`Multi-block selection should not multi select single block 1`] = ` exports[`Multi-block selection should only trigger multi-selection when at the end 1`] = ` " -

1.

+

4.

diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap index 407fcff1297c18..71943003cf923f 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/writing-flow.test.js.snap @@ -280,7 +280,7 @@ exports[`Writing Flow should not prematurely multi-select 1`] = ` exports[`Writing Flow should only consider the content as one tab stop 1`] = ` " -
2
+
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 9e3ba489b9e864..b242912d1c1c42 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 @@ -655,7 +655,6 @@ describe( 'Multi-block selection', () => { await page.keyboard.press( 'Enter' ); // Select two columns. await page.keyboard.press( 'ArrowRight' ); - await page.keyboard.press( 'ArrowRight' ); await page.keyboard.press( 'Enter' ); // Navigate to appender. await page.keyboard.press( 'ArrowRight' ); diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index df94913912c0ec..ed97013203a351 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -631,6 +631,7 @@ describe( 'Writing Flow', () => { // Select the previous block. await page.keyboard.press( 'ArrowUp' ); + await page.keyboard.press( 'ArrowUp' ); // Confirm correct setup. expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -666,8 +667,6 @@ describe( 'Writing Flow', () => { await page.keyboard.press( 'Enter' ); await page.keyboard.type( '/table' ); await page.keyboard.press( 'Enter' ); - // Move into the placeholder UI. - await page.keyboard.press( 'ArrowDown' ); // Tab to the "Create table" button. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); From 8969d2355425e35449e5d2e282874ddfd92cd892 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 1 Mar 2022 01:42:04 -0500 Subject: [PATCH 22/33] Make the implementation more stable. Fix the List View focus bug. --- .../use-focus-first-element.js | 27 ++++++++++--------- .../src/components/list-view/block.js | 1 + 2 files changed, 16 insertions(+), 12 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 0b82f6b435a718..b7c8fd0abe03ff 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 @@ -52,6 +52,16 @@ function useInitialPosition( clientId ) { ); } +function isFormElement( element ) { + const { tagName } = element; + return ( + tagName === 'INPUT' || + tagName === 'BUTTON' || + tagName === 'SELECT' || + tagName === 'TEXTAREA' + ); +} + /** * Transitions focus to the block or inner tabbable when the block becomes * selected and an initial position is set. @@ -97,19 +107,12 @@ export function useFocusFirstElement( clientId ) { } // Check to see if Block contains focussable element before a generic caret insert. - if ( ! ref.current.getAttribute( 'contenteditable' ) ) { + if ( ! target.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( ref.current ); - // Need to make sure we're still in the current Block, if not, we could run in to trouble if focus is placed in a Block further down the page. - if ( ref.current.contains( focusElement ) ) { - // Need to try our best to detect child Blocks and not focus them straight away. - const disallowedTags = [ 'div', 'figure' ]; - const checkDisallowedTags = disallowedTags.some( - ( tag ) => focusElement.tagName.toLowerCase() === tag - ); - if ( ! checkDisallowedTags && focusElement ) { - focusElement.focus(); - return; - } + // Make sure focusElement is valid, form field, and in current ref. + if ( focusElement && isFormElement( focusElement ) && ref.current.contains( focusElement ) ) { + focusElement.focus(); + return; } } diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 0fe5e529391a2d..112dbf477366fa 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -106,6 +106,7 @@ function ListViewBlock( { const selectEditorBlock = useCallback( ( event ) => { selectBlock( event, clientId ); + event.preventDefault(); }, [ clientId, selectBlock ] ); From ce52e45fd598ad601924c9e596ed0a55ddd300b9 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 1 Mar 2022 02:03:00 -0500 Subject: [PATCH 23/33] Use target everywhere. Add a check for Block Inserter trigger. --- .../use-block-props/use-focus-first-element.js | 13 +++++++++++-- 1 file changed, 11 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 b7c8fd0abe03ff..7bf332838fe80b 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 @@ -108,9 +108,18 @@ export function useFocusFirstElement( clientId ) { // Check to see if Block contains focussable element before a generic caret insert. if ( ! target.getAttribute( 'contenteditable' ) ) { - const focusElement = focus.tabbable.findNext( ref.current ); + const focusElement = focus.tabbable.findNext( target ); + // Ensure is not Add block/Block Inserter trigger + const isBlockInserterTrigger = target.getAttribute( 'aria-label' ) + ? target.getAttribute( 'aria-label' ) === 'Add block' + : false; // Make sure focusElement is valid, form field, and in current ref. - if ( focusElement && isFormElement( focusElement ) && ref.current.contains( focusElement ) ) { + if ( + focusElement && + isFormElement( focusElement ) && + ! isBlockInserterTrigger && + target.contains( focusElement ) + ) { focusElement.focus(); return; } From c0027c40a17d8442bf987e086765b106d079e716 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 1 Mar 2022 17:18:08 -0500 Subject: [PATCH 24/33] Use Tab to exit the toolbar so blocks could be navigated. Only seems to be a partial fix for this E2E test failure. --- packages/e2e-tests/specs/editor/various/writing-flow.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index ed97013203a351..6bfe7c166f22f0 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -628,6 +628,8 @@ describe( 'Writing Flow', () => { `//button[contains(@class,'components-dropdown-menu__menu-item')]//span[contains(text(), 'Wide width')]` ); await wideButton.click(); + // Focus the block content + await page.keyboard.press( 'Tab' ); // Select the previous block. await page.keyboard.press( 'ArrowUp' ); From 41ffcec5eb48fadd884076ffb74d7111f120cf4c Mon Sep 17 00:00:00 2001 From: tellthemachines Date: Fri, 4 Mar 2022 15:37:14 +1100 Subject: [PATCH 25/33] Fix failing test and update snapshot. --- .../__snapshots__/multi-block-selection.test.js.snap | 2 +- .../e2e-tests/specs/editor/various/writing-flow.test.js | 6 +++++- 2 files changed, 6 insertions(+), 2 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 1c85d83cb4b1d2..3ede4a98a20c0b 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 @@ -116,7 +116,7 @@ exports[`Multi-block selection should not multi select single block 1`] = ` exports[`Multi-block selection should only trigger multi-selection when at the end 1`] = ` " -

4.

+

1.

diff --git a/packages/e2e-tests/specs/editor/various/writing-flow.test.js b/packages/e2e-tests/specs/editor/various/writing-flow.test.js index 6bfe7c166f22f0..1389d3bc770e15 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -653,13 +653,17 @@ describe( 'Writing Flow', () => { const inserter = await page.$( '.block-editor-block-list__insertion-point' ); + // Find the space between the inserter and the image block. const inserterRect = await inserter.boundingBox(); const lowerInserterY = inserterRect.y + ( 2 * inserterRect.height ) / 3; + // Clicking that in-between space should select the image block. await page.mouse.click( x, lowerInserterY ); const type = await page.evaluate( () => - document.activeElement.getAttribute( 'data-type' ) + document.activeElement + .closest( '[data-block]' ) + .getAttribute( 'data-type' ) ); expect( type ).toBe( 'core/image' ); From cdea00f3e0e4cd18bda468e919e40377592a0007 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 4 Mar 2022 14:18:13 -0500 Subject: [PATCH 26/33] Fix group block still places focus on Add block button. Add some E2E tests. --- .../use-focus-first-element.js | 6 +++-- .../various/keyboard-navigable-blocks.test.js | 24 +++++++++++++++++++ 2 files changed, 28 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 7bf332838fe80b..b3dd0a3eade8c9 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 @@ -110,8 +110,10 @@ export function useFocusFirstElement( clientId ) { if ( ! target.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( target ); // Ensure is not Add block/Block Inserter trigger - const isBlockInserterTrigger = target.getAttribute( 'aria-label' ) - ? target.getAttribute( 'aria-label' ) === 'Add block' + const isBlockInserterTrigger = focusElement.hasAttribute( + 'aria-label' + ) + ? focusElement.getAttribute( 'aria-label' ) === 'Add block' : false; // Make sure focusElement is valid, form field, and in current ref. if ( diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js index b42f90b2df0151..9b009bb3e2aec7 100644 --- a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js @@ -207,4 +207,28 @@ describe( 'Order of block keyboard navigation', () => { await page.keyboard.press( 'ArrowRight' ); await expect( await getActiveLabel() ).toBe( 'Move up' ); } ); + + it( 'allows the first element within a block to receive focus', async () => { + // Insert a columns block. + await insertBlock( 'Image' ); + + const uploadButton = await page.waitForXPath( + '//button[contains( text(), "Upload" ) ]' + ); + + // Make sure the button has focus. + await expect( uploadButton ).toHaveFocus(); + + // Try to focus the block wrapper. + await page.keyboard.press( 'ArrowUp' ); + await expect( await getActiveLabel() ).toBe( 'Block: Image' ); + } ); + + it( 'allows the block wrapper to gain focus for a group block instead of the first element', async () => { + // Insert a group block. + await insertBlock( 'Group' ); + + // If active label matches, that means focus did not change from block wrapper. + await expect( await getActiveLabel() ).toBe( 'Block: Group' ); + } ); } ); From 72a59330b82fddbd7e641a90f95af6520775c9df Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 4 Mar 2022 14:21:16 -0500 Subject: [PATCH 27/33] Fix code comment. --- .../specs/editor/various/keyboard-navigable-blocks.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js index 9b009bb3e2aec7..2d80daf42af1a3 100644 --- a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js @@ -209,7 +209,7 @@ describe( 'Order of block keyboard navigation', () => { } ); it( 'allows the first element within a block to receive focus', async () => { - // Insert a columns block. + // Insert a group block. await insertBlock( 'Image' ); const uploadButton = await page.waitForXPath( From 065ba6f2ad46493c8d3556e8f10b170f2143f82a Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 4 Mar 2022 23:10:30 -0500 Subject: [PATCH 28/33] Fix bugs. --- .../use-block-props/use-focus-first-element.js | 18 +++++++++--------- .../various/keyboard-navigable-blocks.test.js | 6 +++--- 2 files changed, 12 insertions(+), 12 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 b3dd0a3eade8c9..d3136ed75bef97 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 @@ -106,21 +106,21 @@ export function useFocusFirstElement( clientId ) { return; } - // Check to see if Block contains focussable element before a generic caret insert. + // Check to see if element is focussable before a generic caret insert. if ( ! target.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( target ); - // Ensure is not Add block/Block Inserter trigger - const isBlockInserterTrigger = focusElement.hasAttribute( - 'aria-label' - ) - ? focusElement.getAttribute( 'aria-label' ) === 'Add block' - : false; + // Ensure is not block inserter trigger, don't want to focus that in the event of the group block which doesn't contain any other focussable elements. + let skipForInserterTrigger = false; + + if ( focusElement.getAttribute( 'aria-label' ) === 'Add block' ) { + skipForInserterTrigger = true; + } // Make sure focusElement is valid, form field, and in current ref. if ( focusElement && isFormElement( focusElement ) && - ! isBlockInserterTrigger && - target.contains( focusElement ) + target.contains( focusElement ) && + skipForInserterTrigger === false ) { focusElement.focus(); return; diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js index 2d80daf42af1a3..166ecf3dcdee44 100644 --- a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js @@ -209,17 +209,17 @@ describe( 'Order of block keyboard navigation', () => { } ); it( 'allows the first element within a block to receive focus', async () => { - // Insert a group block. + // Insert a image block. await insertBlock( 'Image' ); const uploadButton = await page.waitForXPath( '//button[contains( text(), "Upload" ) ]' ); - // Make sure the button has focus. + // Make sure the upload button has focus. await expect( uploadButton ).toHaveFocus(); - // Try to focus the block wrapper. + // Try to focus the image block wrapper. await page.keyboard.press( 'ArrowUp' ); await expect( await getActiveLabel() ).toBe( 'Block: Image' ); } ); From c2a8ba9f42e482cae58ac8f9b70ef86ce40c0904 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 4 Mar 2022 23:13:22 -0500 Subject: [PATCH 29/33] Formatting fixes. --- .../block-list/use-block-props/use-focus-first-element.js | 1 - .../specs/editor/various/keyboard-navigable-blocks.test.js | 5 ++--- 2 files changed, 2 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 d3136ed75bef97..d47a3c296a1100 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 @@ -111,7 +111,6 @@ export function useFocusFirstElement( clientId ) { const focusElement = focus.tabbable.findNext( target ); // Ensure is not block inserter trigger, don't want to focus that in the event of the group block which doesn't contain any other focussable elements. let skipForInserterTrigger = false; - if ( focusElement.getAttribute( 'aria-label' ) === 'Add block' ) { skipForInserterTrigger = true; } diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js index 166ecf3dcdee44..bac9e5c5a09279 100644 --- a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js @@ -212,11 +212,10 @@ describe( 'Order of block keyboard navigation', () => { // Insert a image block. await insertBlock( 'Image' ); + // Make sure the upload button has focus. const uploadButton = await page.waitForXPath( '//button[contains( text(), "Upload" ) ]' ); - - // Make sure the upload button has focus. await expect( uploadButton ).toHaveFocus(); // Try to focus the image block wrapper. @@ -228,7 +227,7 @@ describe( 'Order of block keyboard navigation', () => { // Insert a group block. await insertBlock( 'Group' ); - // If active label matches, that means focus did not change from block wrapper. + // If active label matches, that means focus did not change from group block wrapper. await expect( await getActiveLabel() ).toBe( 'Block: Group' ); } ); } ); From 300c3024a2154f3ed2f0aeb93decf9b50bd56373 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 5 Mar 2022 00:30:35 -0500 Subject: [PATCH 30/33] Update to use class name. --- .../use-block-props/use-focus-first-element.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 d47a3c296a1100..c38390fde14681 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 @@ -110,16 +110,15 @@ export function useFocusFirstElement( clientId ) { if ( ! target.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( target ); // Ensure is not block inserter trigger, don't want to focus that in the event of the group block which doesn't contain any other focussable elements. - let skipForInserterTrigger = false; - if ( focusElement.getAttribute( 'aria-label' ) === 'Add block' ) { - skipForInserterTrigger = true; - } + const skipBlockInserterTrigger = target.classList.contains( + 'block-editor-button-block-appender' + ); // Make sure focusElement is valid, form field, and in current ref. if ( focusElement && isFormElement( focusElement ) && target.contains( focusElement ) && - skipForInserterTrigger === false + ! skipBlockInserterTrigger ) { focusElement.focus(); return; From b66fbd2c9f9fad9b8dad5310a04b4da08797da71 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 5 Mar 2022 01:29:47 -0500 Subject: [PATCH 31/33] Try again to fix my own test. --- .../use-block-props/use-focus-first-element.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 c38390fde14681..2175567afc35d7 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 @@ -109,16 +109,15 @@ export function useFocusFirstElement( clientId ) { // Check to see if element is focussable before a generic caret insert. if ( ! target.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( target ); + // Make sure focusElement is valid, form field, and within the current target element. // Ensure is not block inserter trigger, don't want to focus that in the event of the group block which doesn't contain any other focussable elements. - const skipBlockInserterTrigger = target.classList.contains( - 'block-editor-button-block-appender' - ); - // Make sure focusElement is valid, form field, and in current ref. if ( focusElement && isFormElement( focusElement ) && target.contains( focusElement ) && - ! skipBlockInserterTrigger + ! focusElement.classList.contains( + 'block-editor-button-block-appender' + ) ) { focusElement.focus(); return; From 3f4df149690618b56ed550a33b30351795b28931 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 14 Mar 2022 16:07:43 -0400 Subject: [PATCH 32/33] Try again to rewrite toolbar roving tabindex now that E2E is working. --- .../various/toolbar-roving-tabindex.test.js | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js b/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js index 61cbf9ffd577d0..e05f1b595ec9eb 100644 --- a/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js +++ b/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js @@ -13,23 +13,30 @@ async function focusBlockToolbar() { } async function expectLabelToHaveFocus( label ) { - await expect( - await page.evaluate( () => + let ariaLabel = await page.evaluate( () => + document.activeElement.getAttribute( 'aria-label' ) + ); + if ( ariaLabel !== label ) { + await page.keyboard.press( 'ArrowUp' ); + ariaLabel = await page.evaluate( () => document.activeElement.getAttribute( 'aria-label' ) - ) - ).toBe( label ); + ); + } + await expect( ariaLabel ).toBe( label ); } -async function testBlockToolbarKeyboardNavigation( currentBlockTitle ) { +async function testBlockToolbarKeyboardNavigation( + currentBlockLabel, + currentBlockTitle +) { await focusBlockToolbar(); await expectLabelToHaveFocus( currentBlockTitle ); await page.keyboard.press( 'ArrowRight' ); await expectLabelToHaveFocus( 'Move up' ); await page.keyboard.press( 'Tab' ); + await expectLabelToHaveFocus( currentBlockLabel ); await pressKeyWithModifier( 'shift', 'Tab' ); await expectLabelToHaveFocus( 'Move up' ); - await page.keyboard.press( 'ArrowLeft' ); - await expectLabelToHaveFocus( currentBlockTitle ); } async function wrapCurrentBlockWithGroup( currentBlockTitle ) { @@ -62,7 +69,10 @@ describe( 'Toolbar roving tabindex', () => { it( 'ensures paragraph block toolbar uses roving tabindex', async () => { await insertBlock( 'Paragraph' ); await page.keyboard.type( 'Paragraph' ); - await testBlockToolbarKeyboardNavigation( 'Paragraph' ); + await testBlockToolbarKeyboardNavigation( + 'Paragraph block', + 'Paragraph' + ); await wrapCurrentBlockWithGroup( 'Paragraph' ); await testGroupKeyboardNavigation( 'Paragraph block', 'Paragraph' ); } ); @@ -70,7 +80,7 @@ describe( 'Toolbar roving tabindex', () => { it( 'ensures heading block toolbar uses roving tabindex', async () => { await insertBlock( 'Heading' ); await page.keyboard.type( 'Heading' ); - await testBlockToolbarKeyboardNavigation( 'Heading' ); + await testBlockToolbarKeyboardNavigation( 'Block: Heading', 'Heading' ); await wrapCurrentBlockWithGroup( 'Heading' ); await testGroupKeyboardNavigation( 'Block: Heading', 'Heading' ); } ); @@ -78,34 +88,34 @@ describe( 'Toolbar roving tabindex', () => { it( 'ensures list block toolbar uses roving tabindex', async () => { await insertBlock( 'List' ); await page.keyboard.type( 'List' ); - await testBlockToolbarKeyboardNavigation( 'List' ); + await testBlockToolbarKeyboardNavigation( 'Block: List', 'List' ); await wrapCurrentBlockWithGroup( 'List' ); await testGroupKeyboardNavigation( 'Block: List', 'List' ); } ); it( 'ensures image block toolbar uses roving tabindex', async () => { await insertBlock( 'Image' ); - await testBlockToolbarKeyboardNavigation( 'Image' ); + await testBlockToolbarKeyboardNavigation( 'Block: Image', 'Image' ); await wrapCurrentBlockWithGroup( 'Image' ); await testGroupKeyboardNavigation( 'Block: Image', 'Image' ); } ); it( 'ensures table block toolbar uses roving tabindex', async () => { await insertBlock( 'Table' ); - await testBlockToolbarKeyboardNavigation( 'Table' ); + await testBlockToolbarKeyboardNavigation( 'Block: Table', 'Table' ); // Move focus to the first toolbar item. await page.keyboard.press( 'Home' ); await expectLabelToHaveFocus( 'Table' ); await page.click( '.blocks-table__placeholder-button' ); await page.keyboard.press( 'Tab' ); - await expectLabelToHaveFocus( 'Body cell text' ); + await testBlockToolbarKeyboardNavigation( 'Body cell text', 'Table' ); await wrapCurrentBlockWithGroup( 'Table' ); await testGroupKeyboardNavigation( 'Block: Table', 'Table' ); } ); it( 'ensures custom html block toolbar uses roving tabindex', async () => { await insertBlock( 'Custom HTML' ); - await testBlockToolbarKeyboardNavigation( 'Custom HTML' ); + await testBlockToolbarKeyboardNavigation( 'HTML', 'Custom HTML' ); await wrapCurrentBlockWithGroup( 'Custom HTML' ); await testGroupKeyboardNavigation( 'Block: Custom HTML', From 948f325dd7ff94b838db9413d3c3b4e7dd0a3930 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 14 Mar 2022 16:08:44 -0400 Subject: [PATCH 33/33] Better code commenting. --- .../specs/editor/various/toolbar-roving-tabindex.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js b/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js index e05f1b595ec9eb..ddc36ff4a4770b 100644 --- a/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js +++ b/packages/e2e-tests/specs/editor/various/toolbar-roving-tabindex.test.js @@ -16,6 +16,7 @@ async function expectLabelToHaveFocus( label ) { let ariaLabel = await page.evaluate( () => document.activeElement.getAttribute( 'aria-label' ) ); + // If the labels don't match, try pressing Up Arrow to focus the block wrapper in non-content editable block. if ( ariaLabel !== label ) { await page.keyboard.press( 'ArrowUp' ); ariaLabel = await page.evaluate( () =>