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

[docs] Add a11y section to Tabs #20965

Merged
merged 4 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions docs/src/pages/components/tabs/AccessibleTabs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import React from 'react';
import PropTypes from 'prop-types';
import { makeStyles } from '@material-ui/core/styles';
import AppBar from '@material-ui/core/AppBar';
import FormControlLabel from '@material-ui/core/FormControlLabel';
import Switch from '@material-ui/core/Switch';
import Tabs from '@material-ui/core/Tabs';
import Tab from '@material-ui/core/Tab';
import Typography from '@material-ui/core/Typography';
import Box from '@material-ui/core/Box';

function TabPanel(props) {
Copy link
Member

@oliviertassinari oliviertassinari May 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it use the lab API instead? I will help to gain more feedback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lean more towards this now, yes. For a11y attributes on the stable API we can refer to the previous demos.

const { children, value, index, ...other } = props;

return (
<div
role="tabpanel"
hidden={value !== index}
id={`simple-tabpanel-${index}`}
aria-labelledby={`simple-tab-${index}`}
{...other}
>
{value === index && (
<Box p={3}>
<Typography>{children}</Typography>
</Box>
)}
</div>
);
}

TabPanel.propTypes = {
children: PropTypes.node,
index: PropTypes.any.isRequired,
value: PropTypes.any.isRequired,
};

const useStyles = makeStyles((theme) => ({
root: {
flexGrow: 1,
backgroundColor: theme.palette.background.paper,
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
},
}));

export default function AccessibleTabs() {
const classes = useStyles();

const [value, setValue] = React.useState(0);
const handleChange = (event, newValue) => {
setValue(newValue);
};

const [selectionFollowsFocus, toggleSelectionFollowsFocus] = React.useReducer(
(flag) => !flag,
true,
);

return (
<div className={classes.root}>
<FormControlLabel
control={
<Switch
checked={selectionFollowsFocus}
onChange={toggleSelectionFollowsFocus}
color="secondary"
/>
}
label="Selection follows focus"
/>
<AppBar position="static">
<Tabs
selectionFollowsFocus={selectionFollowsFocus}
value={value}
onChange={handleChange}
aria-label="simple tabs example"
>
<Tab label="Item One" aria-controls="simple-tabpanel-0" id="simple-tab-0" />
<Tab label="Item Two" aria-controls="simple-tabpanel-1" id="simple-tab-1" />
<Tab label="Item Three" aria-controls="simple-tabpanel-2" id="simple-tab-2" />
</Tabs>
</AppBar>
<TabPanel value={value} index={0}>
Item One
</TabPanel>
<TabPanel value={value} index={1}>
Item Two
</TabPanel>
<TabPanel value={value} index={2}>
Item Three
</TabPanel>
</div>
);
}
92 changes: 92 additions & 0 deletions docs/src/pages/components/tabs/AccessibleTabs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import React from 'react';
import { makeStyles, Theme } from '@material-ui/core/styles';
import AppBar from '@material-ui/core/AppBar';
import FormControlLabel from '@material-ui/core/FormControlLabel';
import Switch from '@material-ui/core/Switch';
import Tabs from '@material-ui/core/Tabs';
import Tab from '@material-ui/core/Tab';
import Typography from '@material-ui/core/Typography';
import Box from '@material-ui/core/Box';

interface TabPanelProps {
children?: React.ReactNode;
index: any;
value: any;
}

function TabPanel(props: TabPanelProps) {
const { children, value, index, ...other } = props;

return (
<div
role="tabpanel"
hidden={value !== index}
id={`simple-tabpanel-${index}`}
aria-labelledby={`simple-tab-${index}`}
{...other}
>
{value === index && (
<Box p={3}>
<Typography>{children}</Typography>
</Box>
)}
</div>
);
}

const useStyles = makeStyles((theme: Theme) => ({
root: {
flexGrow: 1,
backgroundColor: theme.palette.background.paper,
},
}));

export default function AccessibleTabs() {
const classes = useStyles();

const [value, setValue] = React.useState(0);
const handleChange = (event: React.ChangeEvent<{}>, newValue: number) => {
setValue(newValue);
};

const [selectionFollowsFocus, toggleSelectionFollowsFocus] = React.useReducer(
(flag) => !flag,
true,
);

return (
<div className={classes.root}>
<FormControlLabel
control={
<Switch
checked={selectionFollowsFocus}
onChange={toggleSelectionFollowsFocus}
color="secondary"
/>
}
label="Selection follows focus"
/>
<AppBar position="static">
<Tabs
selectionFollowsFocus={selectionFollowsFocus}
value={value}
onChange={handleChange}
aria-label="simple tabs example"
>
<Tab label="Item One" aria-controls="simple-tabpanel-0" id="simple-tab-0" />
<Tab label="Item Two" aria-controls="simple-tabpanel-1" id="simple-tab-1" />
<Tab label="Item Three" aria-controls="simple-tabpanel-2" id="simple-tab-2" />
</Tabs>
</AppBar>
<TabPanel value={value} index={0}>
Item One
</TabPanel>
<TabPanel value={value} index={1}>
Item Two
</TabPanel>
<TabPanel value={value} index={2}>
Item Three
</TabPanel>
</div>
);
}
23 changes: 23 additions & 0 deletions docs/src/pages/components/tabs/tabs.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,29 @@ Tab labels may be either all icons or all text.

{{"demo": "pages/components/tabs/IconLabelTabs.js", "bg": true}}

## Accessibility

(WAI-ARIA: https://www.w3.org/TR/wai-aria-practices/#tabpanel)

This section lists necessary steps to provide necessary information for assistive technology:
eps1lon marked this conversation as resolved.
Show resolved Hide resolved

1. label `Tabs` via `aria-label` or `aria-labelledby`
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
2. `Tab`s need to be connected to their
corresponding `[role="tabpanel"]` by setting the correct `id`, `aria-controls` and `aria-labelledby`
eps1lon marked this conversation as resolved.
Show resolved Hide resolved

An example for the current implementation can be found in the demos on this page. We've also published an experimental API in `@material-ui/lab` that does not require
extra work.

### Keyboard navigation

The components implement keyboard navigation using the "manual activation" behavior. If you want to switch to the
"selection automatically follows focus" behavior you have pass `selectionFollowsFocus` to the `Tabs` component. The WAI-ARIA authoring practices have a detailed guide on [how to decide then to make selection automatically follow focus](https://www.w3.org/TR/wai-aria-practices/#kbd_selection_follows_focus).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"selection automatically follows focus" behavior you have pass `selectionFollowsFocus` to the `Tabs` component. The WAI-ARIA authoring practices have a detailed guide on [how to decide then to make selection automatically follow focus](https://www.w3.org/TR/wai-aria-practices/#kbd_selection_follows_focus).
"selection automatically follows focus" behavior you have pass `selectionFollowsFocus` to the `Tabs` component. The WAI-ARIA authoring practices have a detailed guide on [how to decide when to make selection automatically follow focus](https://www.w3.org/TR/wai-aria-practices/#kbd_selection_follows_focus).

To which the answer seems to be "always", since there should be no reason why an asynchronous network request for tabpanel content should block tab navigation.

I recall a comment that the default for v5 is up for discussion, and that current behaviour is retained to avoid a breaking change, but I think the wording in this section should more strongly advocate the use of this prop.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now I would agree with this. However, it's likely that this could be harmful for certain UIs. I think promise cancellation isn't a thing right now. Automatic activation could kick of a bunch of network requests that each have the same priority. You'd switch 5 tabs and now the first 4 tabs are starving the fifth request.

There's also the issue of render-blocking. Keyboard navigation is user-input and therefore any slight lag is noticed immediately. Since concurrent react is not stable yet, switching could introduce a sluggish UI if the corresponding panel takes more than 16ms to render.

It's a tough decission. I'd say in concurrent react (and if the Offscreen API lands) selectionFollowsFocus is better for UX. Everyone else should test this in production first.

We can extend the section a bit more with things to consider (too many requests, render blocking etc).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points well made, thanks!


### Demo
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved

{{"demo": "pages/components/tabs/AccessibleTabs.js", "bg": true}}


## Experimental Tabs API
eps1lon marked this conversation as resolved.
Show resolved Hide resolved

`@material-ui/lab` offers utility components that inject props to implement accessible tabs
Expand Down