[docs] Add a11y section to Tabs #20965
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eps1lon
added
docs
1 task
1 task
Details of bundle changes.Comparing: b0d6b80...426d177 Details of page changes
|
| import Typography from '@material-ui/core/Typography'; | ||
| import Box from '@material-ui/core/Box'; | ||
|
|
||
| function TabPanel(props) { |
There was a problem hiding this comment.
Should it use the lab API instead? I will help to gain more feedback.
There was a problem hiding this comment.
I lean more towards this now, yes. For a11y attributes on the stable API we can refer to the previous demos.
mbrookes
requested changes
May 10, 2020
| ### 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). |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Good points well made, thanks!
Co-authored-by: Olivier Tassinari <[email protected]> Co-authored-by: Matt <[email protected]>
|
What do you guys think about this layout?
The incentives being:
the diffdiff --git a/docs/src/pages/components/tabs/AccessibleTabs.tsx b/docs/src/pages/components/tabs/AccessibleTabs.tsx
index aa55fa51c..24053867c 100644
--- a/docs/src/pages/components/tabs/AccessibleTabs.tsx
+++ b/docs/src/pages/components/tabs/AccessibleTabs.tsx
@@ -1,5 +1,5 @@
import React from 'react';
-import { makeStyles } from '@material-ui/core/styles';
+import { makeStyles, Theme, createStyles } from '@material-ui/core/styles';
import AppBar from '@material-ui/core/AppBar';
import Tabs from '@material-ui/core/Tabs';
import Tab from '@material-ui/core/Tab';
@@ -33,18 +33,16 @@ function TabPanel(props: TabPanelProps) {
}
interface DemoTabsProps {
- labelId: string;
onChange: (event: unknown, value: number) => void;
selectionFollowsFocus?: boolean;
value: number;
}
function DemoTabs(props: DemoTabsProps) {
- const { labelId, onChange, selectionFollowsFocus, value } = props;
+ const { onChange, selectionFollowsFocus, value } = props;
return (
<AppBar position="static">
<Tabs
- aria-labelledby={labelId}
onChange={onChange}
selectionFollowsFocus={selectionFollowsFocus}
value={value}
@@ -57,11 +55,12 @@ function DemoTabs(props: DemoTabsProps) {
);
}
-const useStyles = makeStyles({
+const useStyles = makeStyles((theme: Theme) => createStyles({
root: {
flexGrow: 1,
+ backgroundColor: theme.palette.background.paper,
},
-});
+}));
export default function AccessibleTabs() {
const classes = useStyles();
@@ -73,19 +72,13 @@ export default function AccessibleTabs() {
return (
<div className={classes.root}>
- <Typography id="demo-a11y-tabs-automatic-label">
- Tabs where selection follows focus
- </Typography>
<DemoTabs
- labelId="demo-a11y-tabs-automatic-label"
selectionFollowsFocus
onChange={handleChange}
value={value}
/>
- <Typography id="demo-a11y-tabs-manual-label">
- Tabs where each tab needs to be selected manually
- </Typography>
- <DemoTabs labelId="demo-a11y-tabs-manual-label" onChange={handleChange} value={value} />
+ <br />
+ <DemoTabs onChange={handleChange} value={value} />
<TabPanel value={value} index={0}>
Item One
</TabPanel>
diff --git a/docs/src/pages/components/tabs/tabs.md b/docs/src/pages/components/tabs/tabs.md
index 41e0a6759..d79931599 100644
--- a/docs/src/pages/components/tabs/tabs.md
+++ b/docs/src/pages/components/tabs/tabs.md
@@ -101,22 +101,26 @@ The following steps are needed in order to provide necessary information for ass
2. `Tab`s need to be connected to their
corresponding `[role="tabpanel"]` by setting the correct `id`, `aria-controls` and `aria-labelledby`.
-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.
+An example for the current implementation can be found in the demos on this page. We've also published [an experimental API](#experimental-api) in the 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 when to make selection automatically follow focus](https://www.w3.org/TR/wai-aria-practices/#kbd_selection_follows_focus).
-#### `selectionFollowsFocus` Demo
-
The following two demos only differ in their keyboard navigation behavior.
Focus a tab and navigate with arrow keys to notice the difference.
-{{"demo": "pages/components/tabs/AccessibleTabs.js", "bg": true}}
+```jsx
+/* 1. Tabs where selection follows focus */
+<Tabs selectionFollowsFocus />
+/* 2. Tabs where each tab needs to be selected manually */
+<Tabs />
+```
+
+{{"demo": "pages/components/tabs/AccessibleTabs.js", "bg": true, "defaultCodeOpen": false}}
-## Experimental Tabs API
+## Experimental API
`@material-ui/lab` offers utility components that inject props to implement accessible tabs
following [WAI-ARIA authoring practices](https://www.w3.org/TR/wai-aria-practices/#tabpanel). |
Already there by naming the prop
Post-hoc rationalization. Headers don't make content harder to follow. Their purpose is to structure content.
Not a priority. It's more important to move description and behavior as close as possible together rather than backreference. |
oliviertassinari
approved these changes
May 11, 2020
| selectionFollowsFocus={selectionFollowsFocus} | ||
| value={value} | ||
| > | ||
| <Tab label="Item One" aria-controls="simple-tabpanel-0" id="simple-tab-0" /> |
There was a problem hiding this comment.
It seems to duplicate with another id on the page https://validator.w3.org/nu/?doc=https%3A%2F%2Fmaster--material-ui.netlify.app%2Fcomponents%2Ftabs%2F.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #6955, part of #20600
Rendered preview