Add useTabs / Tabs Component #837

so99ynoodles · 2020-07-24T10:51:50Z

Closes #784

Implement useTabs
Implement useTab
Add TabsKeyboardDelegate
Change on useSelectableList or create useTabState

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Added dummy storybook / components those are using useTabs / useTab at react-spectrum. But there is a problem I'm getting stuck on. You can ignore those components since they are for demo.
I'll write some tests if they are necessary, mainly they will be the Keyboard.

🧢 Your Project:

snowystinger · 2020-07-24T17:21:02Z

packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts

+}
+
+
+export class TabsKeyboardDelegate<T> implements KeyboardDelegate {


looks like this delegate won't do wrapping, should tabs keyboard navigation do wrapping? @devongovett do you know the answer to this?
If so, you can probably borrow the logic from ActionButtons, https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/actiongroup/src/ActionGroupKeyboardDelegate.ts

+1 to the above. Looking at https://www.w3.org/TR/wai-aria-practices-1.2/#keyboard-interaction-20, focus wrapping should be supported IMO. The ActionGroupKeyboardDelegate logic will also solve the direction flipping that happens in RTL locales

I don't think that needs to be up to the delegate. We have the shouldFocusWrap option to useSelectableList that should handle it.

we could also consider merging this into ListKeyboardDelegate since it's pretty similar. We'd need to add the orientation option, as well as options to disable typeahead and page up/page down I guess.

That should work too, I guess we would need to add the existing shouldFocusWrap logic in useSelectableCollection to ArrowLeft and ArrowRight right?

@so99ynoodles If I am interpreting @devongovett comment correctly (feel free to correct me if I am misunderstanding @devongovett ) I think the focus wrapping should solely be handled by useSelectableCollection via the shouldFocusWrap option. As of now, this keyboard delegate actually handles focus wrapping independently of useSelectableCollection due to how getNextKey and getPreviousKey work (namely

react-spectrum/packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts

Lines 84 to 86 in e56351a

if (!key) {

key = this.collection.getFirstKey();

}

and

react-spectrum/packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts

Lines 94 to 96 in e56351a

if (!key) {

key = this.collection.getLastKey();

}

).

Thinking out loud, I think the next change would be to do the following:

remove getNextKey and getPreviousKey from TabsKeyboardDelegate

replace this.getNextKey(key) with this.collection.getKeyAfter(key) and this.getPreviousKey(key) with `this.collection.getKeyBefore(key)

Add the same shouldFocusWrap logic that exists in useSelectableCollection for ArrowUp and ArrowDown to ArrowLeft and ArrowRight

Ugh, some of the directional stuff gets hairy with getFirstKey and getLastKey here, perhaps useSelectableCollection needs flipDirection communicated to it? I'll need to think about this more

Heh, well... appears all that was copied from ActionGroupKeyboardDelegate so we'd need to change that one too. Perhaps this could be done as a separate task from this PR? Combine all the keyboard delegates into ListKeyboardDelegate.

LFDanLu

Added some comments regarding the aria hook parts of your code. Can comment on the spectrum component parts if you are interested but figured we should focus on the aria bits first. Great work so far btw :)

packages/@react-aria/selection/src/useSelectableList.ts

packages/@react-aria/selection/src/useSelectableCollection.ts

packages/@react-aria/tabs/src/useTabs.ts

packages/@react-spectrum/tabs/src/Tabs.tsx

packages/@react-spectrum/tabs/src/TabList.tsx

devongovett · 2020-07-24T23:46:52Z

packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts

+}
+
+
+export class TabsKeyboardDelegate<T> implements KeyboardDelegate {


I don't think that needs to be up to the delegate. We have the shouldFocusWrap option to useSelectableList that should handle it.

devongovett · 2020-07-25T00:03:03Z

packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts

+}
+
+
+export class TabsKeyboardDelegate<T> implements KeyboardDelegate {


we could also consider merging this into ListKeyboardDelegate since it's pretty similar. We'd need to add the orientation option, as well as options to disable typeahead and page up/page down I guess.

so99ynoodles · 2020-07-25T05:40:21Z

@devongovett @LFDanLu @snowystinger
Thank you for reviewing! 😄 I really appreciate it.

Fixed

Add @react-types/tabs and fix some props.
Combine TabList to Tabs component and move Tab component to the same file.
Replicate selectOnFocus logic of useSelectableCollection under the shouldFocusWrap if statement.
Fix TabsKeyboardDelegate logic. (merging it to ListKeyboardDelegate is also considerable, I thought having independent file is simpler.)
Fix useTabs to provide tabPanelProps instead of useTab.
Create util that generate id for tab and tabpanel.

Testing (Want advice)

Add Picker component to implement overflowMode: dropdown
- Picker calls useSelectState inside, so it uses similar hooks multiple times now...

packages/@react-spectrum/tabs/src/Tabs.tsx

LFDanLu · 2020-07-27T23:58:04Z

packages/@react-spectrum/tabs/src/Tabs.tsx

+  useEffect(() => {
+    let tabs: HTMLElement[] = Array.from(ref.current.querySelectorAll('.' + styles['spectrum-Tabs-item']));
+    if (overflowMode === 'scrolling') {
+      setSelectedTab(tabs.find(tab => tab.dataset.key === state.selectedKey));
+    }
+    if (overflowMode === 'dropdown') {
+      setSelectedTab(tabs[0]);
+    }
+  }, [props.children, state.selectedKey, overflowMode]);


With regards to the overflow mode you've implemented here is some code that might be of interest: https://github.com/adobe/react-spectrum/blob/main/packages/@react-spectrum/breadcrumbs/src/Breadcrumbs.tsx#L56-L120. Breadcrumbs does some overflow handling that maybe of use. Not a hard requirement for this pull, just putting it here if you are interested, you can play around with how it works here: https://react-spectrum.adobe.com/react-spectrum/Breadcrumbs.html#visible-items-overflow-behavior

Tabs is a bit different though. As soon as any tab doesn't fit, then all tabs collapse into a dropdown, whereas with breadcrumbs, they collapse one by one. We'll need to verify this behavior with the Spectrum design team though.

I'll remove this from this PR and cover it in another PR! Thank you.

devongovett

Looking good! 😄

packages/@react-aria/tabs/src/useTabs.ts

packages/@react-aria/tabs/src/index.ts

packages/@react-spectrum/tabs/src/Tabs.tsx

devongovett · 2020-07-28T02:58:05Z

packages/@react-spectrum/tabs/src/Tabs.tsx

+  useEffect(() => {
+    let tabs: HTMLElement[] = Array.from(ref.current.querySelectorAll('.' + styles['spectrum-Tabs-item']));
+    if (overflowMode === 'scrolling') {
+      setSelectedTab(tabs.find(tab => tab.dataset.key === state.selectedKey));
+    }
+    if (overflowMode === 'dropdown') {
+      setSelectedTab(tabs[0]);
+    }
+  }, [props.children, state.selectedKey, overflowMode]);


Tabs is a bit different though. As soon as any tab doesn't fit, then all tabs collapse into a dropdown, whereas with breadcrumbs, they collapse one by one. We'll need to verify this behavior with the Spectrum design team though.

packages/@react-spectrum/tabs/src/Tabs.tsx

packages/@react-aria/tabs/src/useTabs.ts

packages/@react-types/tabs/src/index.d.ts

packages/@react-aria/tabs/src/useTabs.ts

devongovett · 2020-07-28T03:14:04Z

@so99ynoodles awesome work! What do you think about merging an initial implementation and then following up in separate PRs for additional features like overflow mode? Seems like that could help keep the PR to a good size.

The next step would be adding some tests. We use react-testing-library and Jest. You can check out some of the existing tests for other components to see how they're written. They don't have to be 100% complete for the first merge, but it would be good to start with a few basic ones. Let us know if you have questions!

so99ynoodles · 2020-07-28T10:00:33Z

@devongovett
I think it's a nice idea to separate PR!

The next step would be adding some tests

Do you want me to add tests to this PR?

devongovett · 2020-07-28T17:01:40Z

@so99ynoodles a few tests to cover the basics would be great! We can add more later on.

so99ynoodles · 2020-07-30T11:49:43Z

@devongovett
Added some tests!

devongovett

Looks great! Thanks so much. 😄

darekkay · 2020-09-18T20:56:34Z

Hey @devongovett ,
I am missing the Tabs component from the Adobe React Spectrum demo page. Are there any plans to include it?

devongovett · 2020-09-18T22:34:37Z

@darekkay its still in alpha right now, but we will have docs once a stable versions ships (hopefully soon!).

so99ynoodles added 3 commits July 24, 2020 11:35

Add useTabs hook

26804ce

Add basic tabs example (WIP)

7b7f277

Switch useListState to useSingleSelectListState

15e81f9

so99ynoodles requested review from dannify, devongovett and snowystinger as code owners July 24, 2020 10:51

so99ynoodles added 2 commits July 24, 2020 20:07

Add fixme tabpanel props

7d8d139

Add role

f3a4321

so99ynoodles changed the title ~~[WIP] Add useTabs / Tabs Component~~ Add useTabs / Tabs Component Jul 24, 2020

snowystinger reviewed Jul 24, 2020

View reviewed changes

LFDanLu reviewed Jul 24, 2020

View reviewed changes

devongovett reviewed Jul 25, 2020

View reviewed changes

so99ynoodles added 6 commits July 25, 2020 09:15

Fix TabsKeyboardDelegate

b70265e

Replace useSelectableList to useSelectableCollection

1e13858

Fix comment for selectOnFocus prop

7f4af67

Replicate selectOnFocus logic under shouldFocusWrap if statement

c7c1739

Add @react-types/tabs and merge TabList to Tabs

1bd0f2b

Fix useTabs to provide tabPanelProps and fix types

407d7c6

so99ynoodles and others added 4 commits July 25, 2020 17:26

Remove unnecessary props

6b0c127

Add overflowMode: dropdown

5de083b

Fix

a5a4632

Add tabs.css and fix stories

e56351a

LFDanLu reviewed Jul 27, 2020

View reviewed changes

packages/@react-spectrum/tabs/src/Tabs.tsx Outdated Show resolved Hide resolved

LFDanLu reviewed Jul 27, 2020

View reviewed changes

devongovett requested changes Jul 28, 2020

View reviewed changes

Fix commented parts

92b5f36

so99ynoodles requested a review from devongovett July 28, 2020 09:44

Add tests

968c39b

so99ynoodles and others added 2 commits July 31, 2020 09:58

Fix tests

9291bef

Merge branch 'main' into feature/tabs

f288971

devongovett approved these changes Jul 31, 2020

View reviewed changes

devongovett merged commit 48fa5dd into adobe:main Jul 31, 2020

This was referenced Aug 10, 2025

useTab() does not adhere to W3C ARIA APG regarding aria-controls #8699

Open

Feat: Add support for horizontal orientation to GridList & ListBox #8533

Open

		}


		export class TabsKeyboardDelegate<T> implements KeyboardDelegate {

Add useTabs / Tabs Component #837

Add useTabs / Tabs Component #837

Uh oh!

Conversation

so99ynoodles commented Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

so99ynoodles commented Jul 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixed

Testing (Want advice)

Uh oh!

Uh oh!

LFDanLu Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devongovett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devongovett commented Jul 28, 2020

Uh oh!

so99ynoodles commented Jul 28, 2020

Uh oh!

devongovett commented Jul 28, 2020

Uh oh!

so99ynoodles commented Jul 30, 2020

Uh oh!

so99ynoodles commented Jul 24, 2020 •

edited

Loading

LFDanLu Jul 24, 2020 •

edited

Loading

so99ynoodles commented Jul 25, 2020 •

edited

Loading

LFDanLu Jul 27, 2020 •

edited

Loading