From 2e95c2067d94dc2ff2e577fe3a4b45b88c6a97f1 Mon Sep 17 00:00:00 2001 From: Chad Chadbourne <13856531+chad1008@users.noreply.github.com> Date: Fri, 8 Dec 2023 13:18:38 -0500 Subject: [PATCH] Tabs: sync browser focus to selected tab in controlled mode (#56658) * link browser focus to selected tab changes in controlled mode * remove unnecessary memoization * remove duplicate condition * dont apply when selectOnMove is false * use `items` to find active element, instead of a ref * CHANGELOG * add `selectOnMove` unit tests * convert new tests to ariakit/test * remove redundant tests --- packages/components/CHANGELOG.md | 4 + packages/components/src/tabs/index.tsx | 23 +++- packages/components/src/tabs/test/index.tsx | 111 +++++++++++++++++++- 3 files changed, 136 insertions(+), 2 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index ed4b88d000f88..33e4e58d29e7c 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -26,6 +26,10 @@ - `Tabs`: implement new `tabId` prop ([#56883](https://github.com/WordPress/gutenberg/pull/56883)). +### Experimental + +- `Tabs`: improve focus handling in controlled mode ([#56658](https://github.com/WordPress/gutenberg/pull/56658)). + ## 25.13.0 (2023-11-29) ### Enhancements diff --git a/packages/components/src/tabs/index.tsx b/packages/components/src/tabs/index.tsx index 12dd0b4fcc83f..7f738cb9f08a9 100644 --- a/packages/components/src/tabs/index.tsx +++ b/packages/components/src/tabs/index.tsx @@ -45,7 +45,7 @@ function Tabs( { const isControlled = selectedTabId !== undefined; const { items, selectedId } = store.useState(); - const { setSelectedId } = store; + const { setSelectedId, move } = store; // Keep track of whether tabs have been populated. This is used to prevent // certain effects from firing too early while tab data and relevant @@ -154,6 +154,27 @@ function Tabs( { setSelectedId, ] ); + // In controlled mode, make sure browser focus follows the selected tab if + // the selection is changed while a tab is already being focused. + useLayoutEffect( () => { + if ( ! isControlled || ! selectOnMove ) { + return; + } + const currentItem = items.find( ( item ) => item.id === selectedId ); + const activeElement = currentItem?.element?.ownerDocument.activeElement; + const tabsHasFocus = items.some( ( item ) => { + return activeElement && activeElement === item.element; + } ); + + if ( + activeElement && + tabsHasFocus && + selectedId !== activeElement.id + ) { + move( selectedId ); + } + }, [ isControlled, items, move, selectOnMove, selectedId ] ); + const contextValue = useMemo( () => ( { store, diff --git a/packages/components/src/tabs/test/index.tsx b/packages/components/src/tabs/test/index.tsx index 7e2d467122485..70ad3c1c18ae5 100644 --- a/packages/components/src/tabs/test/index.tsx +++ b/packages/components/src/tabs/test/index.tsx @@ -7,7 +7,7 @@ import { press, click } from '@ariakit/test'; /** * WordPress dependencies */ -import { useState } from '@wordpress/element'; +import { useEffect, useState } from '@wordpress/element'; /** * Internal dependencies @@ -102,6 +102,10 @@ const ControlledTabs = ( { string | undefined | null >( props.selectedTabId ); + useEffect( () => { + setSelectedTabId( props.selectedTabId ); + }, [ props.selectedTabId ] ); + return ( { ).not.toBeInTheDocument(); } ); } ); + + describe( 'When `selectOnMove` is `true`', () => { + it( 'should automatically select a newly focused tab', async () => { + render( ); + + await press.Tab(); + + // Tab key should focus the currently selected tab, which is Beta. + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveFocus(); + + // Arrow keys should select and move focus to the next tab. + await press.ArrowRight(); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveFocus(); + } ); + it( 'should automatically update focus when the selected tab is changed by the controlling component', async () => { + const { rerender } = render( + + ); + + // Tab key should focus the currently selected tab, which is Beta. + await press.Tab(); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + expect( await getSelectedTab() ).toHaveFocus(); + + rerender( + + ); + + // When the selected tab is changed, it should automatically receive focus. + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( await getSelectedTab() ).toHaveFocus(); + } ); + } ); + describe( 'When `selectOnMove` is `false`', () => { + it( 'should apply focus without automatically changing the selected tab', async () => { + render( + + ); + + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + + // Tab key should focus the currently selected tab, which is Beta. + await press.Tab(); + expect( + await screen.findByRole( 'tab', { name: 'Beta' } ) + ).toHaveFocus(); + + // Arrow key should move focus but not automatically change the selected tab. + await press.ArrowRight(); + expect( + screen.getByRole( 'tab', { name: 'Gamma' } ) + ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + + // Pressing the spacebar should select the focused tab. + await press.Space(); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + + // Arrow key should move focus but not automatically change the selected tab. + await press.ArrowRight(); + expect( + screen.getByRole( 'tab', { name: 'Alpha' } ) + ).toHaveFocus(); + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + + // Pressing the enter/return should select the focused tab. + await press.Enter(); + expect( await getSelectedTab() ).toHaveTextContent( 'Alpha' ); + } ); + it( 'should not automatically update focus when the selected tab is changed by the controlling component', async () => { + const { rerender } = render( + + ); + + expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); + + // Tab key should focus the currently selected tab, which is Beta. + await press.Tab(); + expect( await getSelectedTab() ).toHaveFocus(); + + rerender( + + ); + + // When the selected tab is changed, it should not automatically receive focus. + expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); + expect( + screen.getByRole( 'tab', { name: 'Beta' } ) + ).toHaveFocus(); + } ); + } ); } ); } );