From 60be3fae21761981f4c981c1074b0e866b3bff0b Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 11 Apr 2024 12:39:50 -0500 Subject: [PATCH 1/5] Fallback to first enabled tab if no active tab Id If there is no selectedTabId, focus will be placed on the tab container which requires an arrow keypress to move focus to the first tab. This behavior isn't as user friendly as focus should be placed on a tab. To prevent focus on the wrapper, if no element is focused and there is no activeId, fallback to the first enabled tab. --- packages/components/src/tabs/index.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/components/src/tabs/index.tsx b/packages/components/src/tabs/index.tsx index 5ffa8e97a651b0..b7f1a00f4d900c 100644 --- a/packages/components/src/tabs/index.tsx +++ b/packages/components/src/tabs/index.tsx @@ -166,7 +166,12 @@ function Tabs( { ! focusedElement || ! items.some( ( item ) => focusedElement === item.element ) ) { - return; // Return early if no tabs are focused. + // If there is no focused or active tab, fallback to place focus on the first enabled tab + // so there is always an active element + if ( ! activeId && firstEnabledTab?.id ) { + setActiveId( firstEnabledTab.id ); + } + return; } // If, after ariakit re-computes the active tab, that tab doesn't match @@ -177,7 +182,7 @@ function Tabs( { setActiveId( focusedElement.id ); } } ); - }, [ activeId, isControlled, items, setActiveId ] ); + }, [ activeId, isControlled, items, setActiveId, firstEnabledTab ] ); const contextValue = useMemo( () => ( { From aa1a1b19c15bd4e773b4fbbb1f7b55b2fe4c6cc6 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 13 Apr 2024 00:55:55 +0900 Subject: [PATCH 2/5] Add test --- packages/components/src/tabs/test/index.tsx | 38 +++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/components/src/tabs/test/index.tsx b/packages/components/src/tabs/test/index.tsx index 39c860de62c144..bbb7f591d0ffc7 100644 --- a/packages/components/src/tabs/test/index.tsx +++ b/packages/components/src/tabs/test/index.tsx @@ -242,6 +242,44 @@ describe( 'Tabs', () => { await press.Tab(); expect( alphaButton ).toHaveFocus(); } ); + + it( 'should focus on the first enabled tab when pressing the Tab key if no tab is selected', async () => { + const TABS_WITH_ALPHA_DISABLED = TABS.map( ( tabObj ) => + tabObj.tabId === 'alpha' + ? { + ...tabObj, + tab: { + ...tabObj.tab, + disabled: true, + }, + } + : tabObj + ); + + render( + + ); + + await sleep(); + await press.Tab(); + expect( + await screen.findByRole( 'tab', { name: 'Beta' } ) + ).toHaveFocus(); + + await press.ArrowRight(); + expect( + await screen.findByRole( 'tab', { name: 'Gamma' } ) + ).toHaveFocus(); + + await press.Tab(); + await press.ShiftTab(); + expect( + await screen.findByRole( 'tab', { name: 'Gamma' } ) + ).toHaveFocus(); + } ); } ); describe( 'Tab Attributes', () => { From d44e6aeda082046dd390e806f9eebc32a2707bbc Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 13 Apr 2024 00:58:29 +0900 Subject: [PATCH 3/5] Move to designated hook and limit to null selected tab --- packages/components/src/tabs/index.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/components/src/tabs/index.tsx b/packages/components/src/tabs/index.tsx index b7f1a00f4d900c..64a924f28e5c02 100644 --- a/packages/components/src/tabs/index.tsx +++ b/packages/components/src/tabs/index.tsx @@ -153,6 +153,14 @@ function Tabs( { } }, [ isControlled, selectedTab, selectedTabId, setSelectedId ] ); + useEffect( () => { + // If there is no active tab, fallback to place focus on the first enabled tab + // so there is always an active element + if ( selectedTabId === null && ! activeId && firstEnabledTab?.id ) { + setActiveId( firstEnabledTab.id ); + } + }, [ selectedTabId, activeId, firstEnabledTab?.id, setActiveId ] ); + useEffect( () => { if ( ! isControlled ) { return; @@ -166,11 +174,6 @@ function Tabs( { ! focusedElement || ! items.some( ( item ) => focusedElement === item.element ) ) { - // If there is no focused or active tab, fallback to place focus on the first enabled tab - // so there is always an active element - if ( ! activeId && firstEnabledTab?.id ) { - setActiveId( firstEnabledTab.id ); - } return; } @@ -182,7 +185,7 @@ function Tabs( { setActiveId( focusedElement.id ); } } ); - }, [ activeId, isControlled, items, setActiveId, firstEnabledTab ] ); + }, [ activeId, isControlled, items, setActiveId ] ); const contextValue = useMemo( () => ( { From 7b6a6486e9304265604aab201e95f0191cd6462b Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 13 Apr 2024 01:04:52 +0900 Subject: [PATCH 4/5] Add changelog --- packages/components/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 28475ded0efb0b..b3d146623f5c3b 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -13,6 +13,10 @@ - `ProgressBar`: Fix CSS variable with invalid value ([#60576](https://github.com/WordPress/gutenberg/pull/60576)). +### Experimental + +- `Tabs`: Fallback to first enabled tab if no active tab id ([#60681](https://github.com/WordPress/gutenberg/pull/60681)). + ## 27.3.0 (2024-04-03) ### Bug Fix From 30b0447f38a2b8bfd3cdca8d31ccf9fa170a415c Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 13 Apr 2024 01:05:55 +0900 Subject: [PATCH 5/5] Restore original comment --- packages/components/src/tabs/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/tabs/index.tsx b/packages/components/src/tabs/index.tsx index 64a924f28e5c02..aaae3ed2527504 100644 --- a/packages/components/src/tabs/index.tsx +++ b/packages/components/src/tabs/index.tsx @@ -174,7 +174,7 @@ function Tabs( { ! focusedElement || ! items.some( ( item ) => focusedElement === item.element ) ) { - return; + return; // Return early if no tabs are focused. } // If, after ariakit re-computes the active tab, that tab doesn't match