From fea91043de49d96c2c7cc0a9c312de934bc1ab12 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 13 Sep 2023 16:17:42 -0500 Subject: [PATCH 1/4] Fix focus loss when returning from the block toolbar and block content is different. --- .../components/writing-flow/use-tab-nav.js | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) 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 616da1bc758136..0e293919f38545 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 @@ -15,9 +15,10 @@ import { isInSameBlock, isInsideRootBlock } from '../../utils/dom'; export default function useTabNav() { const container = useRef(); + const lastFocus = useRef(); + const lastBlock = useRef(); const focusCaptureBeforeRef = useRef(); const focusCaptureAfterRef = useRef(); - const lastFocus = useRef(); const { hasMultiSelection, getSelectedBlockClientId, getBlockCount } = useSelect( blockEditorStore ); const { setNavigationMode } = useDispatch( blockEditorStore ); @@ -40,7 +41,35 @@ export default function useTabNav() { } else if ( hasMultiSelection() ) { container.current.focus(); } else if ( getSelectedBlockClientId() ) { + // Try to focus the last element. lastFocus.current.focus(); + // Check to see if focus worked. + if ( + lastFocus.current.ownerDocument.activeElement === + lastFocus.current + ) { + // Looks like yes, return early. + return; + } + // Access the last known block and try to find a new area to focus. + const selectedBlock = lastBlock.current; + if ( ! selectedBlock ) { + // TODO: Should this be handled? + return; + } + // Get first tabbable in the block if one exists. + const firstBlockTabbable = focus.tabbable.findNext( selectedBlock ); + // Check to ensure tabbable element is inside the current block. + const tabbableInSelectedBlock = firstBlockTabbable + ? isInSameBlock( firstBlockTabbable, selectedBlock ) + : false; + if ( tabbableInSelectedBlock ) { + // Focus the found tabbable in the selected block. + firstBlockTabbable.focus(); + } else { + // Focus the block wrapper if no tabbable was found. + selectedBlock.focus(); + } } else { setNavigationMode( true ); @@ -158,7 +187,10 @@ export default function useTabNav() { } function onFocusOut( event ) { + // Capture the last element with focus. lastFocus.current = event.target; + // Capture the last known block before focus leaves writing flow. + lastBlock.current = event.target.closest( '[data-block]' ); const { ownerDocument } = node; From 167046c0e4b59dfdbc7a78bab83f16c3e4636029 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Wed, 13 Sep 2023 23:41:09 -0500 Subject: [PATCH 2/4] Cleanup code. --- .../components/writing-flow/use-tab-nav.js | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 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 0e293919f38545..44410f640dc5b6 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 @@ -41,7 +41,7 @@ export default function useTabNav() { } else if ( hasMultiSelection() ) { container.current.focus(); } else if ( getSelectedBlockClientId() ) { - // Try to focus the last element. + // Try to focus the last element which had focus. lastFocus.current.focus(); // Check to see if focus worked. if ( @@ -51,24 +51,17 @@ export default function useTabNav() { // Looks like yes, return early. return; } - // Access the last known block and try to find a new area to focus. - const selectedBlock = lastBlock.current; - if ( ! selectedBlock ) { - // TODO: Should this be handled? - return; - } - // Get first tabbable in the block if one exists. - const firstBlockTabbable = focus.tabbable.findNext( selectedBlock ); - // Check to ensure tabbable element is inside the current block. - const tabbableInSelectedBlock = firstBlockTabbable - ? isInSameBlock( firstBlockTabbable, selectedBlock ) - : false; - if ( tabbableInSelectedBlock ) { - // Focus the found tabbable in the selected block. + // Last element focus did not work. Now try to find the first tabbable in the last block to focus. + const firstBlockTabbable = focus.tabbable.findNext( + lastBlock.current + ); + // Check to ensure tabbable is inside the last block and that it is a form element. + if ( isInSameBlock( lastBlock.current, firstBlockTabbable ) ) { + // Focus the found tabbable in the last block. firstBlockTabbable.focus(); } else { - // Focus the block wrapper if no tabbable was found. - selectedBlock.focus(); + // Focus the last block wrapper if no tabbable was found. + lastBlock.current.focus(); } } else { setNavigationMode( true ); From 2f562836c28280a4495fc98f7374bb548d9d0251 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Thu, 14 Sep 2023 00:15:14 -0500 Subject: [PATCH 3/4] If last focus defaults to block wrapper, try to find a better element to focus instead. The fallback is still wrapper but its last in the list. --- .../components/writing-flow/use-tab-nav.js | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 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 44410f640dc5b6..0202f98fb5d9a0 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 @@ -41,22 +41,25 @@ export default function useTabNav() { } else if ( hasMultiSelection() ) { container.current.focus(); } else if ( getSelectedBlockClientId() ) { - // Try to focus the last element which had focus. - lastFocus.current.focus(); - // Check to see if focus worked. - if ( - lastFocus.current.ownerDocument.activeElement === - lastFocus.current - ) { - // Looks like yes, return early. - return; + // If last focus position matches last block, this likely means we're on the last block wrapper. Let's try to find a better place to focus before defaulting to the wrapper. + if ( lastBlock.current !== lastFocus.current ) { + // Try to focus the last element which had focus. + lastFocus.current.focus(); + // Check to see if focus worked. + if ( + lastFocus.current.ownerDocument.activeElement === + lastFocus.current + ) { + // Looks like yes, return early. + return; + } } // Last element focus did not work. Now try to find the first tabbable in the last block to focus. const firstBlockTabbable = focus.tabbable.findNext( lastBlock.current ); // Check to ensure tabbable is inside the last block and that it is a form element. - if ( isInSameBlock( lastBlock.current, firstBlockTabbable ) ) { + if ( isInsideRootBlock( lastBlock.current, firstBlockTabbable ) ) { // Focus the found tabbable in the last block. firstBlockTabbable.focus(); } else { From 87f37d85cf2fd7d66ddbb5821e16aaa2d52c4fc0 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Fri, 25 Aug 2023 08:54:12 -0500 Subject: [PATCH 4/4] Resolve conflict. --- .../data/data-core-block-editor.md | 24 +++++++++++++++++++ .../components/writing-flow/use-tab-nav.js | 11 ++++++--- packages/block-editor/src/store/actions.js | 15 ++++++++++++ packages/block-editor/src/store/reducer.js | 17 +++++++++++++ packages/block-editor/src/store/selectors.js | 11 +++++++++ 5 files changed, 75 insertions(+), 3 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 7b0bd386daaf48..38a93552bcbef2 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -588,6 +588,18 @@ _Properties_ - _isDisabled_ `boolean`: Whether or not the user should be prevented from inserting this item. - _frecency_ `number`: Heuristic that combines frequency and recency. +### getLastFocus + +Returns the element of the last element that had focus when focus left the editor canvas. + +_Parameters_ + +- _state_ `Object`: Block editor state. + +_Returns_ + +- `Object`: Element. + ### getLastMultiSelectedBlockClientId Returns the client ID of the last block in the multi-selection set, or null if there is no multi-selection. @@ -1651,6 +1663,18 @@ _Parameters_ - _clientId_ `string`: The block's clientId. - _hasControlledInnerBlocks_ `boolean`: True if the block's inner blocks are controlled. +### setLastFocus + +Action that sets the element that had focus when focus leaves the editor canvas. + +_Parameters_ + +- _lastFocus_ `Object`: The last focused element. + +_Returns_ + +- `Object`: Action object. + ### setNavigationMode Action that enables or disables the navigation mode. 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 0202f98fb5d9a0..5914db7f21fd36 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 @@ -15,18 +15,23 @@ import { isInSameBlock, isInsideRootBlock } from '../../utils/dom'; export default function useTabNav() { const container = useRef(); - const lastFocus = useRef(); const lastBlock = useRef(); const focusCaptureBeforeRef = useRef(); const focusCaptureAfterRef = useRef(); + const { hasMultiSelection, getSelectedBlockClientId, getBlockCount } = useSelect( blockEditorStore ); - const { setNavigationMode } = useDispatch( blockEditorStore ); + const { setNavigationMode, setLastFocus } = useDispatch( blockEditorStore ); const isNavigationMode = useSelect( ( select ) => select( blockEditorStore ).isNavigationMode(), [] ); + const lastFocus = useSelect( + ( select ) => select( blockEditorStore ).getLastFocus(), + [] + ); + // Don't allow tabbing to this element in Navigation mode. const focusCaptureTabIndex = ! isNavigationMode ? '0' : undefined; @@ -184,7 +189,7 @@ export default function useTabNav() { function onFocusOut( event ) { // Capture the last element with focus. - lastFocus.current = event.target; + setLastFocus( { ...lastFocus, current: event.target } ); // Capture the last known block before focus leaves writing flow. lastBlock.current = event.target.closest( '[data-block]' ); diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 1740543744aaa2..b0ad2d179f8d9b 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -1967,3 +1967,18 @@ export function unsetBlockEditingMode( clientId = '' ) { clientId, }; } + +/** + * Action that sets the element that had focus when focus leaves the editor canvas. + * + * @param {Object} lastFocus The last focused element. + * + * + * @return {Object} Action object. + */ +export function setLastFocus( lastFocus = null ) { + return { + type: 'LAST_FOCUS', + lastFocus, + }; +} diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 18048ce138eb23..8fc4bbe381ab10 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -1946,6 +1946,22 @@ export function styleOverrides( state = new Map(), action ) { return newState; } } +} + +/** + * Reducer setting last focused element + * + * @param {boolean} state Current state. + * @param {Object} action Dispatched action. + * + * @return {boolean} Updated state. + */ +export function lastFocus( state = false, action ) { + switch ( action.type ) { + case 'LAST_FOCUS': + return action.lastFocus; + } + return state; } @@ -1965,6 +1981,7 @@ const combinedReducers = combineReducers( { settings, preferences, lastBlockAttributesChange, + lastFocus, editorMode, hasBlockMovingClientId, highlightedBlock, diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index c618a354bcd10a..57ec5c86b562e4 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -3018,3 +3018,14 @@ export const isGroupable = createRegistrySelector( ); } ); + +/* + * Returns the element of the last element that had focus when focus left the editor canvas. + * + * @param {Object} state Block editor state. + * + * @return {Object} Element. + */ +export function getLastFocus( state ) { + return state.lastFocus; +}