From e5c36dc5f2e91206a912044d39be7af1a4082841 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 15 Apr 2022 22:16:34 -0400 Subject: [PATCH 1/8] Fix focus after table creation. --- packages/block-library/src/table/edit.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/table/edit.js b/packages/block-library/src/table/edit.js index dea27caa360498..abf99ca987f0b7 100644 --- a/packages/block-library/src/table/edit.js +++ b/packages/block-library/src/table/edit.js @@ -6,7 +6,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useEffect, useState } from '@wordpress/element'; +import { useEffect, useRef, useState } from '@wordpress/element'; import { InspectorControls, BlockControls, @@ -105,6 +105,9 @@ function TableEdit( { const colorProps = useColorProps( attributes ); const borderProps = useBorderProps( attributes ); + const tableRef = useRef(); + const [ hasTableCreated, setHasTableCreated ] = useState( false ); + /** * Updates the initial column count used for table creation. * @@ -137,6 +140,7 @@ function TableEdit( { columnCount: parseInt( initialColumnCount, 10 ) || 2, } ) ); + setHasTableCreated( true ); } /** @@ -341,6 +345,15 @@ function TableEdit( { } }, [ isSelected ] ); + useEffect( () => { + if ( hasTableCreated ) { + tableRef?.current + ?.querySelector( 'td[contentEditable="true"]' ) + ?.focus(); + setHasTableCreated( false ); + } + }, [ hasTableCreated ] ); + const sections = [ 'head', 'body', 'foot' ].filter( ( name ) => ! isEmptyTableSection( attributes[ name ] ) ); @@ -426,7 +439,7 @@ function TableEdit( { const isEmpty = ! sections.length; return ( -
+
{ ! isEmpty && ( <> From 3fb4f6467307c93f6e72a354a60883ea74ca6b19 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 15 Apr 2022 22:42:25 -0400 Subject: [PATCH 2/8] Fix E2E test failures. Add new E2E test. --- .../e2e-tests/specs/editor/blocks/table.test.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/blocks/table.test.js b/packages/e2e-tests/specs/editor/blocks/table.test.js index 7afb8c9cfbbba6..bc9d806a39cc23 100644 --- a/packages/e2e-tests/specs/editor/blocks/table.test.js +++ b/packages/e2e-tests/specs/editor/blocks/table.test.js @@ -269,7 +269,6 @@ describe( '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' ); @@ -280,4 +279,18 @@ describe( 'Table', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should not have focus loss after creation', async () => { + // Insert table block. + await insertBlock( 'Table' ); + + // Create the table. + await clickButton( createButtonLabel ); + + // Focus should be in first td. + const isInTd = await page.waitForSelector( + 'td[contentEditable="true"]' + ); + await expect( isInTd ).toHaveFocus(); + } ); } ); From b09ea5af207e4d46813bc505296afb90e7890b58 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 16 Apr 2022 14:36:07 -0400 Subject: [PATCH 3/8] isFormElement now supports option for contentEditable attribute. --- packages/dom/README.md | 1 + packages/dom/src/dom/is-form-element.js | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/dom/README.md b/packages/dom/README.md index 41af65673bff07..3ad0e79d52c769 100644 --- a/packages/dom/README.md +++ b/packages/dom/README.md @@ -191,6 +191,7 @@ Detects if element is a form element. _Parameters_ - _element_ `Element`: The element to check. +- _includeContentEditable_ `boolean`: Rather or not to include elements with contentEditable attribute. _Returns_ diff --git a/packages/dom/src/dom/is-form-element.js b/packages/dom/src/dom/is-form-element.js index c54c47b6215b5e..dae6e94a101c9c 100644 --- a/packages/dom/src/dom/is-form-element.js +++ b/packages/dom/src/dom/is-form-element.js @@ -7,14 +7,28 @@ import isInputOrTextArea from './is-input-or-text-area'; * * Detects if element is a form element. * - * @param {Element} element The element to check. + * @param {Element} element The element to check. + * @param {boolean} includeContentEditable Rather or not to include elements with contentEditable attribute. * * @return {boolean} True if form element and false otherwise. */ -export default function isFormElement( element ) { +export default function isFormElement( + element, + includeContentEditable = false +) { const { tagName } = element; const checkForInputTextarea = isInputOrTextArea( element ); + let checkContentEditable = false; + if ( + includeContentEditable && + element.getAttribute( 'contentEditable' ) === 'true' + ) { + checkContentEditable = true; + } return ( - checkForInputTextarea || tagName === 'BUTTON' || tagName === 'SELECT' + checkForInputTextarea || + tagName === 'BUTTON' || + tagName === 'SELECT' || + checkContentEditable ); } From e4455e0cc1328916ac2b1b2da62c5e8045bc41bd Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 16 Apr 2022 14:36:35 -0400 Subject: [PATCH 4/8] Fix E2E test. --- .../specs/editor/various/writing-flow.test.js | 16 ---------------- 1 file changed, 16 deletions(-) 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 1389d3bc770e15..9211d0e9bbf3b9 100644 --- a/packages/e2e-tests/specs/editor/various/writing-flow.test.js +++ b/packages/e2e-tests/specs/editor/various/writing-flow.test.js @@ -678,27 +678,11 @@ describe( 'Writing Flow', () => { await page.keyboard.press( 'Tab' ); // Create the table. await page.keyboard.press( 'Space' ); - // Return focus after focus loss. This should be fixed. - await page.keyboard.press( 'Tab' ); // Navigate to the second cell. await page.keyboard.press( 'ArrowRight' ); await page.keyboard.type( '2' ); // Confirm correct setup. expect( await getEditedPostContent() ).toMatchSnapshot(); - // The content should only have one tab stop. - await page.keyboard.press( 'Tab' ); - expect( - await page.evaluate( () => - document.activeElement.getAttribute( 'aria-label' ) - ) - ).toBe( 'Post' ); - await pressKeyWithModifier( 'shift', 'Tab' ); - await pressKeyWithModifier( 'shift', 'Tab' ); - expect( - await page.evaluate( () => - document.activeElement.getAttribute( 'aria-label' ) - ) - ).toBe( 'Table' ); } ); it( 'Should unselect all blocks when hitting double escape', async () => { From b2af579e9104d7fd78762e8c296c30180bcb6380 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 16 Apr 2022 14:37:26 -0400 Subject: [PATCH 5/8] Allow Tab key to work for writing flow contentEditable elements. Allow contentEditable elements for focus first. --- .../block-list/use-block-props/use-focus-first-element.js | 2 +- .../src/components/writing-flow/use-tab-nav.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 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 8b95f19f3c9ff2..2ceb42b67210b0 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 @@ -112,7 +112,7 @@ export function useFocusFirstElement( clientId ) { if ( focusElement && isInsideRootBlock( ref.current, focusElement ) && - isFormElement( focusElement ) + isFormElement( focusElement, true ) ) { focusElement.focus(); return; diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index 61697a4c0d2beb..c22d2150d0528f 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -115,10 +115,13 @@ export default function useTabNav() { // these are not rendered in the content and perhaps in the // future they can be rendered in an iframe or shadow DOM. if ( - ( isFormElement( event.target ) || + ( isFormElement( event.target, true ) || event.target.getAttribute( 'data-block' ) === getSelectedBlockClientId() ) && - isFormElement( focus.tabbable[ direction ]( event.target ) ) + isFormElement( + focus.tabbable[ direction ]( event.target ), + true + ) ) { return; } From 5ccae83c6313de1b87cb5de07e7931236ea4c9c9 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 16 Apr 2022 14:52:09 -0400 Subject: [PATCH 6/8] Update API docs/function comment with better structure. --- packages/dom/README.md | 2 +- packages/dom/src/dom/is-form-element.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/dom/README.md b/packages/dom/README.md index 3ad0e79d52c769..0db05e39bf36c8 100644 --- a/packages/dom/README.md +++ b/packages/dom/README.md @@ -191,7 +191,7 @@ Detects if element is a form element. _Parameters_ - _element_ `Element`: The element to check. -- _includeContentEditable_ `boolean`: Rather or not to include elements with contentEditable attribute. +- _includeContentEditable_ `boolean`: Should include elements with contentEditable attribute. _Returns_ diff --git a/packages/dom/src/dom/is-form-element.js b/packages/dom/src/dom/is-form-element.js index dae6e94a101c9c..d6776f6f9c3969 100644 --- a/packages/dom/src/dom/is-form-element.js +++ b/packages/dom/src/dom/is-form-element.js @@ -8,7 +8,7 @@ import isInputOrTextArea from './is-input-or-text-area'; * Detects if element is a form element. * * @param {Element} element The element to check. - * @param {boolean} includeContentEditable Rather or not to include elements with contentEditable attribute. + * @param {boolean} includeContentEditable Should include elements with contentEditable attribute. * * @return {boolean} True if form element and false otherwise. */ From 62e0e69e3de194d9ea797d273a1262bf3ee11dc5 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Sat, 16 Apr 2022 16:21:24 -0400 Subject: [PATCH 7/8] Only find contentEditable fields within a block for writing flow tab nav. --- .../block-editor/src/components/writing-flow/use-tab-nav.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index c22d2150d0528f..90a741ad94db0e 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -118,10 +118,7 @@ export default function useTabNav() { ( isFormElement( event.target, true ) || event.target.getAttribute( 'data-block' ) === getSelectedBlockClientId() ) && - isFormElement( - focus.tabbable[ direction ]( event.target ), - true - ) + isFormElement( focus.tabbable[ direction ]( event.target ) ) ) { return; } From de235a997b428b8d741a76f5d9115ca0eac76b03 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 20 Apr 2022 13:43:58 -0400 Subject: [PATCH 8/8] Revert changes to dom/block-editor packages for now. Will split these in to another PR. --- .../use-focus-first-element.js | 2 +- .../components/writing-flow/use-tab-nav.js | 2 +- packages/dom/README.md | 1 - packages/dom/src/dom/is-form-element.js | 20 +++---------------- 4 files changed, 5 insertions(+), 20 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 2ceb42b67210b0..8b95f19f3c9ff2 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 @@ -112,7 +112,7 @@ export function useFocusFirstElement( clientId ) { if ( focusElement && isInsideRootBlock( ref.current, focusElement ) && - isFormElement( focusElement, true ) + isFormElement( focusElement ) ) { focusElement.focus(); return; diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index 90a741ad94db0e..61697a4c0d2beb 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -115,7 +115,7 @@ export default function useTabNav() { // these are not rendered in the content and perhaps in the // future they can be rendered in an iframe or shadow DOM. if ( - ( isFormElement( event.target, true ) || + ( isFormElement( event.target ) || event.target.getAttribute( 'data-block' ) === getSelectedBlockClientId() ) && isFormElement( focus.tabbable[ direction ]( event.target ) ) diff --git a/packages/dom/README.md b/packages/dom/README.md index 0db05e39bf36c8..41af65673bff07 100644 --- a/packages/dom/README.md +++ b/packages/dom/README.md @@ -191,7 +191,6 @@ Detects if element is a form element. _Parameters_ - _element_ `Element`: The element to check. -- _includeContentEditable_ `boolean`: Should include elements with contentEditable attribute. _Returns_ diff --git a/packages/dom/src/dom/is-form-element.js b/packages/dom/src/dom/is-form-element.js index d6776f6f9c3969..c54c47b6215b5e 100644 --- a/packages/dom/src/dom/is-form-element.js +++ b/packages/dom/src/dom/is-form-element.js @@ -7,28 +7,14 @@ import isInputOrTextArea from './is-input-or-text-area'; * * Detects if element is a form element. * - * @param {Element} element The element to check. - * @param {boolean} includeContentEditable Should include elements with contentEditable attribute. + * @param {Element} element The element to check. * * @return {boolean} True if form element and false otherwise. */ -export default function isFormElement( - element, - includeContentEditable = false -) { +export default function isFormElement( element ) { const { tagName } = element; const checkForInputTextarea = isInputOrTextArea( element ); - let checkContentEditable = false; - if ( - includeContentEditable && - element.getAttribute( 'contentEditable' ) === 'true' - ) { - checkContentEditable = true; - } return ( - checkForInputTextarea || - tagName === 'BUTTON' || - tagName === 'SELECT' || - checkContentEditable + checkForInputTextarea || tagName === 'BUTTON' || tagName === 'SELECT' ); }