From fac5b0347072e62b464bab213ab70bad0fe075d6 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 15 Mar 2022 09:07:33 -0400 Subject: [PATCH 01/10] Improve the way focus first element works for non-content editable blocks. --- .../use-block-props/use-focus-first-element.js | 10 +++------- packages/block-editor/src/utils/dom.js | 6 +++++- 2 files changed, 8 insertions(+), 8 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 2175567afc35d7..fa5659afe1ab5c 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,15 +109,11 @@ 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. + // Make sure focusElement is valid, contained in the same block, and a form field. if ( focusElement && - isFormElement( focusElement ) && - target.contains( focusElement ) && - ! focusElement.classList.contains( - 'block-editor-button-block-appender' - ) + isInsideRootBlock( ref.current, target ) && + isFormElement( focusElement ) ) { focusElement.focus(); return; diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 6de80b24dac4bc..b4a9d388b44907 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -1,5 +1,6 @@ const BLOCK_SELECTOR = '.block-editor-block-list__block'; const APPENDER_SELECTOR = '.block-list-appender'; +const BLOCK_APPENDER_CLASS = '.block-editor-button-block-appender'; /** * Returns true if two elements are contained within the same block. @@ -27,7 +28,10 @@ export function isInsideRootBlock( blockElement, element ) { const parentBlock = element.closest( [ BLOCK_SELECTOR, APPENDER_SELECTOR ].join( ',' ) ); - return parentBlock === blockElement; + return ( + parentBlock === blockElement && + element.classList.contains( BLOCK_APPENDER_CLASS ) + ); } /** From 33bdfc5674873fd925af77bbed30b073d9bc40fd Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 15 Mar 2022 09:23:20 -0400 Subject: [PATCH 02/10] Some code fixes. --- .../block-list/use-block-props/use-focus-first-element.js | 2 +- packages/block-editor/src/utils/dom.js | 2 +- 2 files changed, 2 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 fa5659afe1ab5c..f4733144bfbb1e 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 ) { // Make sure focusElement is valid, contained in the same block, and a form field. if ( focusElement && - isInsideRootBlock( ref.current, target ) && + isInsideRootBlock( ref.current, focusElement ) && isFormElement( focusElement ) ) { focusElement.focus(); diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index b4a9d388b44907..713b3c1b17fb5a 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -30,7 +30,7 @@ export function isInsideRootBlock( blockElement, element ) { ); return ( parentBlock === blockElement && - element.classList.contains( BLOCK_APPENDER_CLASS ) + ! element.classList.contains( BLOCK_APPENDER_CLASS ) ); } From e454097234608b1113096e2a8b3aa4814b35bd21 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 15 Mar 2022 09:53:37 -0400 Subject: [PATCH 03/10] Break off isFormElement function in to utils.js for reuse. --- .../use-focus-first-element.js | 11 +---------- .../components/writing-flow/use-tab-nav.js | 11 +---------- .../src/components/writing-flow/utils.js | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 packages/block-editor/src/components/writing-flow/utils.js 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 f4733144bfbb1e..f35d5633a19c02 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 @@ -13,6 +13,7 @@ import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ +import { isFormElement } from '../../writing-flow/utils'; import { isInsideRootBlock } from '../../../utils/dom'; import { store as blockEditorStore } from '../../../store'; import { setContentEditableWrapper } from './use-multi-selection'; @@ -52,16 +53,6 @@ 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. 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 452a6f76a6d506..84668d64b346a0 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 @@ -10,18 +10,9 @@ import { useRef } from '@wordpress/element'; /** * Internal dependencies */ +import { isFormElement } from './utils'; import { store as blockEditorStore } from '../../store'; -function isFormElement( element ) { - const { tagName } = element; - return ( - tagName === 'INPUT' || - tagName === 'BUTTON' || - tagName === 'SELECT' || - tagName === 'TEXTAREA' - ); -} - export default function useTabNav() { const container = useRef(); const focusCaptureBeforeRef = useRef(); diff --git a/packages/block-editor/src/components/writing-flow/utils.js b/packages/block-editor/src/components/writing-flow/utils.js new file mode 100644 index 00000000000000..1d02898a78d7bb --- /dev/null +++ b/packages/block-editor/src/components/writing-flow/utils.js @@ -0,0 +1,19 @@ +/** @typedef {import('@wordpress/element').RefObject} RefObject */ + +/** + * + * Detects if element is a form element. + * + * @param {RefObject} element The element to check. + * + * @return {boolean} True if form element and false otherwise. + */ +export function isFormElement( element ) { + const { tagName } = element; + return ( + tagName === 'INPUT' || + tagName === 'BUTTON' || + tagName === 'SELECT' || + tagName === 'TEXTAREA' + ); +} From cbd79e6fc6b80d49e4f8feb2127af78a7c65523d Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 15 Mar 2022 13:04:07 -0400 Subject: [PATCH 04/10] Fix element class detection error. --- packages/block-editor/src/utils/dom.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 713b3c1b17fb5a..510ce30fe33e7c 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -25,13 +25,13 @@ export function isInSameBlock( a, b ) { * its inner blocks or appender. */ export function isInsideRootBlock( blockElement, element ) { + if ( element.classList.contains( BLOCK_APPENDER_CLASS ) ) { + return false; + } const parentBlock = element.closest( [ BLOCK_SELECTOR, APPENDER_SELECTOR ].join( ',' ) ); - return ( - parentBlock === blockElement && - ! element.classList.contains( BLOCK_APPENDER_CLASS ) - ); + return parentBlock === blockElement; } /** From 14fd4ad527fc1f44b014969e7e41b3ff86bd668d Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 15 Mar 2022 14:49:27 -0400 Subject: [PATCH 05/10] Try to fix test failures. --- packages/block-editor/src/utils/dom.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 510ce30fe33e7c..dda5a701e66c5f 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -25,13 +25,13 @@ export function isInSameBlock( a, b ) { * its inner blocks or appender. */ export function isInsideRootBlock( blockElement, element ) { - if ( element.classList.contains( BLOCK_APPENDER_CLASS ) ) { - return false; - } const parentBlock = element.closest( [ BLOCK_SELECTOR, APPENDER_SELECTOR ].join( ',' ) ); - return parentBlock === blockElement; + return ( + parentBlock === blockElement && + parentBlock.classList.contains( BLOCK_APPENDER_CLASS ) === false + ); } /** From 42ddba7f19c4f87d398bae7d1e8bc47efbcfa9e0 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Tue, 15 Mar 2022 15:35:41 -0400 Subject: [PATCH 06/10] See if tests pass. --- packages/block-editor/src/utils/dom.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index dda5a701e66c5f..99466de6f304bc 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -1,6 +1,6 @@ const BLOCK_SELECTOR = '.block-editor-block-list__block'; const APPENDER_SELECTOR = '.block-list-appender'; -const BLOCK_APPENDER_CLASS = '.block-editor-button-block-appender'; +//const BLOCK_APPENDER_CLASS = '.block-editor-button-block-appender'; /** * Returns true if two elements are contained within the same block. @@ -28,10 +28,7 @@ export function isInsideRootBlock( blockElement, element ) { const parentBlock = element.closest( [ BLOCK_SELECTOR, APPENDER_SELECTOR ].join( ',' ) ); - return ( - parentBlock === blockElement && - parentBlock.classList.contains( BLOCK_APPENDER_CLASS ) === false - ); + return parentBlock === blockElement; } /** From bb7a855d2fc4c457d02f51fc5184b30a21a7e2fb Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 16 Mar 2022 14:00:00 -0400 Subject: [PATCH 07/10] Now that tests are working again, try again with some fixes. JSDoc improvements. Type improvements. Add back the ability to skip block specific inserter. Add custom option to toggle this ability. --- .../use-focus-first-element.js | 9 ++++++++- .../src/components/writing-flow/utils.js | 4 +--- packages/block-editor/src/utils/dom.js | 20 +++++++++++++++---- 3 files changed, 25 insertions(+), 8 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 f35d5633a19c02..dca22adc8ed98d 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,10 +100,17 @@ 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 ); + const rootBlockOptions = { + skipBlockInserter: true, + }; // Make sure focusElement is valid, contained in the same block, and a form field. if ( focusElement && - isInsideRootBlock( ref.current, focusElement ) && + isInsideRootBlock( + ref.current, + focusElement, + rootBlockOptions + ) && isFormElement( focusElement ) ) { focusElement.focus(); diff --git a/packages/block-editor/src/components/writing-flow/utils.js b/packages/block-editor/src/components/writing-flow/utils.js index 1d02898a78d7bb..d116053ea045bb 100644 --- a/packages/block-editor/src/components/writing-flow/utils.js +++ b/packages/block-editor/src/components/writing-flow/utils.js @@ -1,10 +1,8 @@ -/** @typedef {import('@wordpress/element').RefObject} RefObject */ - /** * * Detects if element is a form element. * - * @param {RefObject} element The element to check. + * @param {Element} element The element to check. * * @return {boolean} True if form element and false otherwise. */ diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index 99466de6f304bc..e42ec20c9400fa 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -1,6 +1,6 @@ const BLOCK_SELECTOR = '.block-editor-block-list__block'; const APPENDER_SELECTOR = '.block-list-appender'; -//const BLOCK_APPENDER_CLASS = '.block-editor-button-block-appender'; +const BLOCK_APPENDER_CLASS = '.block-editor-button-block-appender'; /** * Returns true if two elements are contained within the same block. @@ -18,16 +18,28 @@ export function isInSameBlock( a, b ) { * Returns true if an element is considered part of the block and not its inner * blocks or appender. * - * @param {Element} blockElement Block container element. - * @param {Element} element Element. + * @param {Element} blockElement Block container element. + * @param {Element} element Element. + * @param {{ skipBlockInserter: boolean }} options Custom options to set. * * @return {boolean} Whether an element is considered part of the block and not * its inner blocks or appender. */ -export function isInsideRootBlock( blockElement, element ) { +export function isInsideRootBlock( + blockElement, + element, + options = { skipBlockInserter: false } +) { const parentBlock = element.closest( [ BLOCK_SELECTOR, APPENDER_SELECTOR ].join( ',' ) ); + // Skip inserter for blocks such as group block. + if ( + options.skipBlockInserter && + parentBlock?.classList.contains( BLOCK_APPENDER_CLASS ) + ) { + return false; + } return parentBlock === blockElement; } From c5dae590d47d96b2005adf94dc752da146c3d7c9 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 16 Mar 2022 22:38:32 -0400 Subject: [PATCH 08/10] Fix failing E2E tests. --- .../block-list/use-block-props/use-focus-first-element.js | 4 ++-- 1 file changed, 2 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 dca22adc8ed98d..917e3430e2ee5a 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,8 +98,8 @@ 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 ); + if ( ! ref.current.getAttribute( 'contenteditable' ) ) { + const focusElement = focus.tabbable.findNext( ref.current ); const rootBlockOptions = { skipBlockInserter: true, }; From 6ed767d2810bff52ee755aa0b09bcf63d88a8691 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Thu, 17 Mar 2022 14:03:24 -0400 Subject: [PATCH 09/10] Move isFormElement to wordpress/dom. --- .../use-focus-first-element.js | 8 ++++++-- .../components/writing-flow/use-tab-nav.js | 3 +-- .../src/components/writing-flow/utils.js | 17 ---------------- packages/dom/README.md | 12 +++++++++++ packages/dom/src/dom/index.js | 1 + packages/dom/src/dom/is-form-element.js | 20 +++++++++++++++++++ 6 files changed, 40 insertions(+), 21 deletions(-) delete mode 100644 packages/block-editor/src/components/writing-flow/utils.js create mode 100644 packages/dom/src/dom/is-form-element.js 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 917e3430e2ee5a..55c2af58c5ed4c 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 @@ -7,13 +7,17 @@ import { first, last } from 'lodash'; * WordPress dependencies */ import { useEffect, useRef } from '@wordpress/element'; -import { focus, isTextField, placeCaretAtHorizontalEdge } from '@wordpress/dom'; +import { + focus, + isFormElement, + isTextField, + placeCaretAtHorizontalEdge, +} from '@wordpress/dom'; import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import { isFormElement } from '../../writing-flow/utils'; import { isInsideRootBlock } from '../../../utils/dom'; import { store as blockEditorStore } from '../../../store'; import { setContentEditableWrapper } from './use-multi-selection'; 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 84668d64b346a0..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 @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { focus } from '@wordpress/dom'; +import { focus, isFormElement } from '@wordpress/dom'; import { TAB, ESCAPE } from '@wordpress/keycodes'; import { useSelect, useDispatch } from '@wordpress/data'; import { useRefEffect, useMergeRefs } from '@wordpress/compose'; @@ -10,7 +10,6 @@ import { useRef } from '@wordpress/element'; /** * Internal dependencies */ -import { isFormElement } from './utils'; import { store as blockEditorStore } from '../../store'; export default function useTabNav() { diff --git a/packages/block-editor/src/components/writing-flow/utils.js b/packages/block-editor/src/components/writing-flow/utils.js deleted file mode 100644 index d116053ea045bb..00000000000000 --- a/packages/block-editor/src/components/writing-flow/utils.js +++ /dev/null @@ -1,17 +0,0 @@ -/** - * - * Detects if element is a form element. - * - * @param {Element} element The element to check. - * - * @return {boolean} True if form element and false otherwise. - */ -export function isFormElement( element ) { - const { tagName } = element; - return ( - tagName === 'INPUT' || - tagName === 'BUTTON' || - tagName === 'SELECT' || - tagName === 'TEXTAREA' - ); -} diff --git a/packages/dom/README.md b/packages/dom/README.md index a8ec7ed26547a2..41af65673bff07 100644 --- a/packages/dom/README.md +++ b/packages/dom/README.md @@ -184,6 +184,18 @@ _Returns_ - `boolean`: True if entirely selected, false if not. +### isFormElement + +Detects if element is a form element. + +_Parameters_ + +- _element_ `Element`: The element to check. + +_Returns_ + +- `boolean`: True if form element and false otherwise. + ### isHorizontalEdge Check whether the selection is horizontally at the edge of the container. diff --git a/packages/dom/src/dom/index.js b/packages/dom/src/dom/index.js index 5e8e44449bcea3..f21ec1e4e85e66 100644 --- a/packages/dom/src/dom/index.js +++ b/packages/dom/src/dom/index.js @@ -6,6 +6,7 @@ export { default as getRectangleFromRange } from './get-rectangle-from-range'; export { default as getScrollContainer } from './get-scroll-container'; export { default as getOffsetParent } from './get-offset-parent'; export { default as isEntirelySelected } from './is-entirely-selected'; +export { default as isFormElement } from './is-form-element'; export { default as isHorizontalEdge } from './is-horizontal-edge'; export { default as isNumberInput } from './is-number-input'; export { default as isTextField } from './is-text-field'; diff --git a/packages/dom/src/dom/is-form-element.js b/packages/dom/src/dom/is-form-element.js new file mode 100644 index 00000000000000..c54c47b6215b5e --- /dev/null +++ b/packages/dom/src/dom/is-form-element.js @@ -0,0 +1,20 @@ +/** + * Internal dependencies + */ +import isInputOrTextArea from './is-input-or-text-area'; + +/** + * + * Detects if element is a form element. + * + * @param {Element} element The element to check. + * + * @return {boolean} True if form element and false otherwise. + */ +export default function isFormElement( element ) { + const { tagName } = element; + const checkForInputTextarea = isInputOrTextArea( element ); + return ( + checkForInputTextarea || tagName === 'BUTTON' || tagName === 'SELECT' + ); +} From 007d8264763c2f654a696c8c43eb6c0fabf5e257 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Fri, 18 Mar 2022 09:26:58 -0400 Subject: [PATCH 10/10] Code cleanup. --- .../use-focus-first-element.js | 9 +-------- packages/block-editor/src/utils/dom.js | 20 ++++--------------- 2 files changed, 5 insertions(+), 24 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 55c2af58c5ed4c..fcdfe1c87663d7 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 @@ -104,17 +104,10 @@ export function useFocusFirstElement( clientId ) { // Check to see if element is focussable before a generic caret insert. if ( ! ref.current.getAttribute( 'contenteditable' ) ) { const focusElement = focus.tabbable.findNext( ref.current ); - const rootBlockOptions = { - skipBlockInserter: true, - }; // Make sure focusElement is valid, contained in the same block, and a form field. if ( focusElement && - isInsideRootBlock( - ref.current, - focusElement, - rootBlockOptions - ) && + isInsideRootBlock( ref.current, focusElement ) && isFormElement( focusElement ) ) { focusElement.focus(); diff --git a/packages/block-editor/src/utils/dom.js b/packages/block-editor/src/utils/dom.js index e42ec20c9400fa..6af35aff730155 100644 --- a/packages/block-editor/src/utils/dom.js +++ b/packages/block-editor/src/utils/dom.js @@ -18,28 +18,16 @@ export function isInSameBlock( a, b ) { * Returns true if an element is considered part of the block and not its inner * blocks or appender. * - * @param {Element} blockElement Block container element. - * @param {Element} element Element. - * @param {{ skipBlockInserter: boolean }} options Custom options to set. + * @param {Element} blockElement Block container element. + * @param {Element} element Element. * * @return {boolean} Whether an element is considered part of the block and not * its inner blocks or appender. */ -export function isInsideRootBlock( - blockElement, - element, - options = { skipBlockInserter: false } -) { +export function isInsideRootBlock( blockElement, element ) { const parentBlock = element.closest( - [ BLOCK_SELECTOR, APPENDER_SELECTOR ].join( ',' ) + [ BLOCK_SELECTOR, APPENDER_SELECTOR, BLOCK_APPENDER_CLASS ].join( ',' ) ); - // Skip inserter for blocks such as group block. - if ( - options.skipBlockInserter && - parentBlock?.classList.contains( BLOCK_APPENDER_CLASS ) - ) { - return false; - } return parentBlock === blockElement; }