Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a tab component to react-spectrum #784

Closed
dergroncki opened this issue Jul 18, 2020 · 6 comments · Fixed by #837
Closed

Add a tab component to react-spectrum #784

dergroncki opened this issue Jul 18, 2020 · 6 comments · Fixed by #837
Labels
enhancement New feature or request

Comments

@dergroncki
Copy link

🙋 Feature Request

Add a tab component as described in the https://www.w3.org/TR/wai-aria-practices/#tabpanel.

@so99ynoodles
Copy link
Contributor

Tabs and TabList component are under construction :)

@devongovett
Copy link
Member

Yep though they were made a long time ago as a POC and never finished. I think at this point I would re-start the implementation or at least heavily modify it. If anyone would like to work on that, I’d be happy to provide some guidance! 😊

@devongovett devongovett reopened this Jul 18, 2020
@so99ynoodles
Copy link
Contributor

I'm interested in implementing it. :)

@devongovett
Copy link
Member

@so99ynoodles awesome! Let me get back to you with some updated API specs and we can go from there. In the meantime, here’s the accessibility spec we’ll need to follow: https://w3c.github.io/aria-practices/#tabpanel

@devongovett devongovett added the enhancement New feature or request label Jul 19, 2020
@devongovett
Copy link
Member

Ok, here's the API spec we came up with.

import {CollectionBase, SingleSelection} from '@react-types/shared';
import {SingleSelectListState} from '@react-stately/list';

interface TabsProps<T> extends CollectionBase<T>, SingleSelection {
  /**
   * Whether tabs are activated automatically on focus or manually.
   * @default 'automatic'
   */
  keyboardActivation?: 'automatic' | 'manual',
  /**
   * The orientation of the tabs.
   * @default 'horizontal'
   */
  orientation?: 'horizontal' | 'vertical'
}

interface TabsAria {
  /** Props for the tablist container */
  tabListProps: HTMLAttributes<HTMLElement>
}

declare function useTabs<T>(props: TabsProps<T>, state: SingleSelectListState<T>): TabsAria;

interface TabAria {
  /** Props for the tab element */
  tabProps: HTMLAttributes<HTMLElement>,
  /** Props for the associated tabpanel element */
  tabPanelProps: HTMLAttributes<HTMLElement>
}

interface TabAriaProps<T> {
  /** Collection node for the tab. */
  item: Node<T>
}

declare function useTab<T>(props: TabAriaProps, state: SingleSelectListState<T>): TabAria;

There's also some spectrum specific props. I can provide those if you want to implement the Spectrum part and not just the Aria/Stately parts.

Here's an example of what constructing a tab component would look like for an end user:

<Tabs selectedKey="files">
  <Item key="files" title="Your files">
    <Files />
  </Item>
  <Item key="shared" title="Shared with you">
    <Shared />
  </Item>
</Tabs>

The API follows our collection components API, using <Item> to specify each tab item. In addition, it follows the same selection API that we use for other components.

In terms of implementation, there's a few pieces:

  • For state, we can use the useSingleSelectListState hook. That will handle converting the children from JSX to a Collection object, along with setting up selection state and a SelectionManager. I don't think any other state is needed for tabs beyond this, but if so, we can add a useTabState hook later on.
  • The useTabs hook should be created to manage the tab list and the associated tab panels. It should return ARIA props for the tablist element (see below). It should also call the useSelectableCollection hook, which will handle the keyboard navigation and focus management requirements. You'll need to either add a new KeyboardDelegate implementation to handle the left and right arrow keys, or add an orientation option to the existing ListKeyboardDelegate to allow switching from vertical to horizontal orientation.
  • The useTab hook should be created to handle an individual tab and its associated tabpanel element. It should return ARIA props for those elements, including generating ids and aria-labelledby attributes (see below). It should also call the useSelectableItem hook to handle selecting tabs and focusing accordingly. The hook should receive a Node object from the collection, and use the title and rendered properties for the tab and tabpanel contents respectively.

For accessibility, the spec is listed above in my previous comment. In addition, you can see the two examples from the WAI-ARIA Authoring Practices. They cover the two keyboardActivation modes.

I'd start by making a basic example component using the hooks and use that in the storybook/tests for now. I'm happy to also provide guidance on the Spectrum-specific parts, but they're not required if you'd like to focus on React Aria/React Stately.

Another helpful thing is to start by running the yarn plop command, which will help bootstrap the new packages you'll need to create with some templates. We've removed the existing implementation from the repo since it was old and probably harder to modify than to just start from scratch.

That was a lot. Hopefully I covered enough to get you started, but please let me know if you have any questions! 😄

@so99ynoodles
Copy link
Contributor

so99ynoodles commented Jul 24, 2020

@devongovett
Hi. I was creating react-spectrum/tabs for testing the behavior of newly created useTabs and useTab, I found something confusing.

// jsx
<Tabs selectedKey="files">
  <Item key="files" title="Your files">
    Files
  </Item>
  <Item key="shared" title="Shared with you">
    Shared
  </Item>
</Tabs>

// HTML created from jsx above
<div role="tabs">
  <div role="tablist">
    // could be a div, button is expected according to ↓. CSS changes are needed?
    // https://www.w3.org/TR/wai-aria-practices-1.2/examples/tabs/tabs-2/tabs.html
    <button role="tab">Your files</button>
    <button role="tab">Shared with you</button>
  </div>
  <div role="tabpanel">
    Files
  </div>
</div>

// Role based architecture
<Tabs>
  <TabList>
    <Tab />
    <Tab />
  </TabList>
  <TabPanel />
</Tabs>

tabPanelProps are returned by useTab, and useTab is called inside of Tab component,
ButTabPanel is rendered outside of TabList, which makes passing tabPanelProps to it difficult.
Since they are Spectrum-specific parts, if this looks fine to you, I'll just focus on aria parts 😅 .

Please checkout the PR, any advice would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants