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

[Table] Add aria-label & caption in table demos #17696

Merged
merged 9 commits into from Oct 10, 2019

Conversation

adeelibr
Copy link
Contributor

@adeelibr adeelibr commented Oct 3, 2019

Closes #17679

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 3, 2019

Are aria- and caption meant to be used at the same time? Which one should be preferred? cc @ahayes91

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 3, 2019

The difference that I was able to find between <caption/> tag & aria-label attribute is;

  • Use caption element to provide an accessible name for a data table.
  • Use aria-label attribute to provide an accessible name for a data table (NOTE: inconsistent browser/AT support).

Reference: https://fae.disability.illinois.edu/rulesets/TABLE_2/

So i ended up using both, which I am now thinking is not the right approach maybe. What do you suggest? @oliviertassinari

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 3, 2019

Details of bundle changes.

Comparing: daadbca...4a2532a

bundle Size Change Size Gzip Change Gzip
LinearProgress -- 65.3 kB -- 20.3 kB
@material-ui/core[umd] ▲ +148 B (+0.05% ) 306 kB ▲ +33 B (+0.04% ) 88 kB
Link -- 66.6 kB -- 21 kB
@material-ui/core ▲ +158 B (+0.05% ) 347 kB ▲ +50 B (+0.05% ) 94.9 kB
@material-ui/lab -- 144 kB -- 45 kB
@material-ui/styles -- 51.8 kB -- 15.6 kB
@material-ui/system -- 15.7 kB -- 4.35 kB
AppBar -- 63.9 kB -- 19.9 kB
Avatar -- 62.7 kB -- 19.6 kB
Backdrop -- 67.7 kB -- 20.9 kB
Badge -- 65.3 kB -- 20.2 kB
BottomNavigation -- 62.4 kB -- 19.5 kB
BottomNavigationAction -- 75.3 kB -- 23.7 kB
Box -- 70.8 kB -- 21.3 kB
Breadcrumbs -- 68 kB -- 21.3 kB
Button -- 79.2 kB -- 24.5 kB
ButtonBase -- 73.8 kB -- 23.1 kB
ButtonGroup -- 64.2 kB -- 20 kB
Card -- 62.9 kB -- 19.6 kB
CardActionArea -- 74.9 kB -- 23.6 kB
CardActions -- 62 kB -- 19.3 kB
CardContent -- 62 kB -- 19.3 kB
CardHeader -- 65.1 kB -- 20.4 kB
CardMedia -- 62.4 kB -- 19.5 kB
Checkbox -- 81.6 kB -- 25.6 kB
Chip -- 70.6 kB -- 21.8 kB
CircularProgress -- 64.1 kB -- 20.1 kB
ClickAwayListener -- 3.85 kB -- 1.55 kB
Collapse -- 67.8 kB -- 20.9 kB
colorManipulator -- 3.83 kB -- 1.52 kB
Container -- 63.1 kB -- 19.6 kB
CssBaseline -- 57.6 kB -- 18 kB
Dialog -- 82.5 kB -- 25.5 kB
DialogActions -- 62.1 kB -- 19.4 kB
DialogContent -- 62.2 kB -- 19.4 kB
DialogContentText -- 64 kB -- 20 kB
DialogTitle -- 64.3 kB -- 20.1 kB
Divider -- 62.6 kB -- 19.6 kB
docs.landing -- 54.3 kB -- 14.3 kB
docs.main -- 598 kB -- 191 kB
Drawer -- 84.3 kB -- 26 kB
ExpansionPanel -- 71.1 kB -- 22.1 kB
ExpansionPanelActions -- 62.1 kB -- 19.4 kB
ExpansionPanelDetails -- 61.9 kB -- 19.3 kB
ExpansionPanelSummary -- 77.9 kB -- 24.5 kB
Fab -- 76.6 kB -- 23.8 kB
Fade -- 23.1 kB -- 8.03 kB
FilledInput -- 72.9 kB -- 22.6 kB
FormControl -- 64.3 kB -- 19.9 kB
FormControlLabel -- 65.5 kB -- 20.5 kB
FormGroup -- 62 kB -- 19.3 kB
FormHelperText -- 63.3 kB -- 19.7 kB
FormLabel -- 63.2 kB -- 19.5 kB
Grid -- 65.1 kB -- 20.3 kB
GridList -- 62.5 kB -- 19.5 kB
GridListTile -- 63.7 kB -- 19.9 kB
GridListTileBar -- 63.2 kB -- 19.7 kB
Grow -- 23.7 kB -- 8.15 kB
Hidden -- 66.1 kB -- 20.6 kB
Icon -- 62.8 kB -- 19.6 kB
IconButton -- 76 kB -- 23.6 kB
Input -- 72 kB -- 22.4 kB
InputAdornment -- 65.1 kB -- 20.4 kB
InputBase -- 70.2 kB -- 22 kB
InputLabel -- 65.1 kB -- 20.2 kB
List -- 62.4 kB -- 19.3 kB
ListItem -- 77 kB -- 24 kB
ListItemAvatar -- 62.1 kB -- 19.3 kB
ListItemIcon -- 62.2 kB -- 19.4 kB
ListItemSecondaryAction -- 62 kB -- 19.3 kB
ListItemText -- 65 kB -- 20.3 kB
ListSubheader -- 62.8 kB -- 19.6 kB
Menu -- 88.2 kB -- 27.6 kB
MenuItem -- 78 kB -- 24.3 kB
MenuList -- 66 kB -- 20.5 kB
MobileStepper -- 67.7 kB -- 21 kB
Modal -- 14.3 kB -- 4.96 kB
NativeSelect -- 76.4 kB -- 24 kB
NoSsr -- 2.19 kB -- 1.04 kB
OutlinedInput -- 73.5 kB -- 22.8 kB
Paper -- 62.4 kB -- 19.3 kB
Popover -- 82.6 kB -- 25.4 kB
Popper -- 28.3 kB -- 10.2 kB
Portal -- 2.87 kB -- 1.29 kB
Radio -- 82.5 kB -- 25.9 kB
RadioGroup -- 63.2 kB -- 19.7 kB
Rating -- 69.8 kB -- 22.2 kB
RootRef -- 4.43 kB -- 1.67 kB
Select -- 113 kB -- 33.5 kB
Skeleton -- 62.5 kB -- 19.5 kB
Slide -- 25.1 kB -- 8.66 kB
Slider -- 74.9 kB -- 23.5 kB
Snackbar -- 77.1 kB -- 24 kB
SnackbarContent -- 65.8 kB -- 20.6 kB
SpeedDial -- 85.7 kB -- 26.9 kB
SpeedDialAction -- 115 kB -- 36.4 kB
SpeedDialIcon -- 64.6 kB -- 20.2 kB
Step -- 62.6 kB -- 19.5 kB
StepButton -- 82.1 kB -- 25.7 kB
StepConnector -- 62.7 kB -- 19.6 kB
StepContent -- 68.9 kB -- 21.4 kB
StepIcon -- 64.7 kB -- 20.1 kB
StepLabel -- 68.6 kB -- 21.4 kB
Stepper -- 64.9 kB -- 20.3 kB
styles/createMuiTheme -- 16.2 kB -- 5.78 kB
SvgIcon -- 63 kB -- 19.6 kB
SwipeableDrawer -- 90.7 kB -- 28 kB
Switch -- 80.8 kB -- 25.1 kB
Tab -- 76.2 kB -- 24.1 kB
TableBody -- 62.1 kB -- 19.3 kB
TableCell -- 64.1 kB -- 20 kB
TableFooter -- 62.1 kB -- 19.3 kB
TableHead -- 62.1 kB -- 19.3 kB
TablePagination -- 139 kB -- 40.6 kB
TableRow -- 62.5 kB -- 19.5 kB
TableSortLabel -- 77.2 kB -- 24.4 kB
Tabs -- 85.3 kB -- 27.1 kB
TextareaAutosize -- 5.06 kB -- 2.11 kB
TextField -- 121 kB -- 35.4 kB
ToggleButton -- 76 kB -- 23.9 kB
ToggleButtonGroup -- 63.2 kB -- 19.8 kB
Toolbar -- 62.3 kB -- 19.4 kB
Table ▲ +158 B (+0.25% ) 62.5 kB ▲ +75 B (+0.39% ) 19.5 kB
Tooltip -- 99 kB -- 31.3 kB
TreeItem -- 73.4 kB -- 23 kB
TreeView -- 66 kB -- 20.6 kB
Typography -- 63.6 kB -- 19.8 kB
useMediaQuery -- 2.49 kB -- 1.05 kB
Zoom -- 23.1 kB -- 8.04 kB

Generated by 🚫 dangerJS against 4a2532a

@eps1lon
Copy link
Member

eps1lon commented Oct 3, 2019

It's basically the difference between label and description. caption works as an implicit reference via aria-describedby while aria-label is just a label.

If we want to include this we should provide explicit examples not just repeating the words.

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 3, 2019

If we want to include this we should provide explicit examples not just repeating the words.

So just aria-label or use caption (with more details) with aria-describedby or both at the same time? 😅

@eps1lon
Copy link
Member

eps1lon commented Oct 3, 2019

So just aria-label or use caption (with more details) with aria-describedby or both at the same time? sweat_smile

Can't give you a clear prescription here but at least something more specific than "label" and "label" + caption.

@ahayes91
Copy link

ahayes91 commented Oct 4, 2019

Are aria- and caption meant to be used at the same time? Which one should be preferred? cc @ahayes91

I'm definitely not an expert or regular user of these to be honest. I've done some testing on both options both separately and together (<Table aria-label="My table">....</Table> and <Table><caption>My table</caption>....</Table>) as below.

I've applied CSS to visually hide the <caption> element:

position: fixed;
bottom: 200vh;

Screenreader testing:

Screenreader Browser OS Results for aria-label Results for caption Results for using both
VoiceOver Chrome Mac OS Announced both with regular tab navigation and table navigation Announced both with regular tab navigation and table navigation Announced only once with both with regular tab navigation and table navigation
VoiceOver Safari Mac OS Announced both with regular tab navigation and table navigation Announced both with regular tab navigation and table navigation Announced only once with both with regular tab navigation and table navigation
ChromeVox Chrome Mac OS Not announced Not announced with regular tab navigation but is announced with table navigation - this does shift focus to the caption though, even when it is visually hidden. Not sure what can be done about that. Not announced with regular tab navigation but is announced once with table navigation - this does shift focus to the caption though, even when it is visually hidden. Not sure what can be done about that.
NVDA Firefox Windows 10 Announced both with regular tab navigation and table navigation Announced both with regular tab navigation and table navigation (caption is announced for table navigation twice but once at the start and once at the end, probably nothing that can be done about that) Announced once both with regular tab navigation and table navigation (caption is announced for table navigation twice but once at the start and once at the end, probably nothing that can be done about that)
NVDA Chrome Windows 10 Announced both with regular tab navigation and table navigation Announced both with regular tab navigation and table navigation (caption is announced for table navigation twice but once at the start and once at the end, probably nothing that can be done about that) Announced once both with regular tab navigation and table navigation (caption is announced for table navigation twice but once at the start and once at the end, probably nothing that can be done about that)
JAWS Chrome Windows 10 Announced both with regular tab navigation and table navigation Announced both with regular tab navigation and table navigation (this is really buggy though for React - page needs to be refreshed for JAWS to pick up the markup correctly) Not announced with regular tab navigation but announced once with table navigation
JAWS Firefox Windows 10 Announced with regular tab navigation and table navigation Announced both with regular tab navigation and table navigation Announced once both with regular tab navigation and table navigation

If you have any doc examples ready I can do some testing on those too - the tests above were performed on my own implementation of an MUI table.

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 4, 2019

I have added an aria-describedby along with better captions (I hope 😅)
is it better now? 😅😅

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 4, 2019

I've applied CSS to visually hide the element:

position: fixed;
bottom: 200vh;

@ahayes91 I have not worked with accessibility that much, but does display: none have the same effect in terms of screen-readers like VoiceOver etc as that of position: fixed; bottom: 200vh 🤔

@ahayes91
Copy link

ahayes91 commented Oct 4, 2019

I've applied CSS to visually hide the element:
position: fixed;
bottom: 200vh;

@ahayes91 I have not worked with accessibility that much, but does display: none have the same effect in terms of screen-readers like VoiceOver etc as that of position: fixed; bottom: 200vh 🤔

Quite the opposite 😄 display: none will hide the region from screenreaders, same for visibility: hidden for most screenreaders. https://webaim.org/techniques/css/invisiblecontent/

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 4, 2019

thank you for the explanation @ahayes91.

Should I add .hidden class to the <caption /> element which has position: fixed; bottom: 200vh styled apply to it, to hide it from the demo? @eps1lon

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 5, 2019

What do you guys think of using, and only, using caption?

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 5, 2019

According to this if we are using <caption /> we don't actually need aria-describedby property & this property is only used when

The element containing the summary doesn’t need to be in front of the table in the document, but it helps users to discover the summary more quickly if the summary is near the table, especially if they are not using a screen reader. i.e,

<p id="tblDesc">Column one has the location and size of accommodation, other columns show the type and number of properties available.</p>
<table aria-describedby="tblDesc">

So would a simple table with caption work like below;

<table>
    <caption>lorem ipsum lorem ipsum lorem ipsum</caption>
    <!-- TABLE CONTENT -->
</table>

@eps1lon
Copy link
Member

eps1lon commented Oct 6, 2019

Should I add .hidden class to the element which has position: fixed; bottom: 200vh styled apply to it, to hide it from the demo? @eps1lon

caption should be visible for sighted users.

Either use caption or aria-describedby. An aria-labe is good in any case.

@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 6, 2019

Either use caption or aria-describedby. An aria-labe is good in any case.

I have made the changes, can you kindly review again. 😄 😅

@adeelibr

This comment has been minimized.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 7, 2019

@adeelibr Great work so far. Sorry, I have commented too early. After more thoughts, I come to the following conclusion:

  1. We need to handle the CSS for the caption element. I have pushed a commit in this direction: ddf5401. Looking at the screenshot diffs, It might not be enough, adding the typography body2 variant style sounds even better.
  2. Because the demos, we have, use the "Paper" approach, rendering a caption inside them doesn't look good. I don't think that it's an option we can keep. We need something else (the aria-label is great).
  3. If we are willing to accept the "caption" overhead in all the demos (my personal inclination would be slightly against for simplicity, but if @eps1lon think that we should, it's fine by me) we would need to replace the caption with a Typography color="secondary" at the bottom, outside the paper, with a aria-describedby link with the table. This is related to point 2.
  4. Because accessibility is important, I recommend adding an Accessibility documentation section at the bottom of the page. It's something we try to do in all the component pages. We could cover the description part of the problem, linking relevant resources.
  5. I think that we need at least one demo with a caption and one with a aria-describedby in the page. If we don't have them, the new proposed accessibility section in 4. would be a great place to host them.

@oliviertassinari oliviertassinari added the component: table This is the name of the generic UI component, not the React module! label Oct 7, 2019
@adeelibr
Copy link
Contributor Author

adeelibr commented Oct 7, 2019

I honestly love it, how 4 lines of CSS can make it so pretty (still amazes me) 😄 😺 But if you don't find it elegant, I can remove the caption tag from all of the tables & only keep aria-label & add a new section for `Accessibility with 2 demos, 1 with caption & the other with aria-describedby, if that's okay?

@attilavago
Copy link

@ahayes91 this bit might bite us in the behind later...

position: fixed;
bottom: 200vh;

This is perfect for desktop but on mobile you might end up with unexpected focusing behaviour. It might very well work fine, but worth testing before merging.

@eps1lon
Copy link
Member

eps1lon commented Oct 8, 2019

The styling looks fine to me. What is the problem currently?

Quick summary as to label, caption and description:

  • accessible name derivde from: aria-labelledby then aria-label then caption. First "hit" being prioritized.
  • accessible description derived from: aria-describedby then caption. First "hit" being prioritized.

-- https://w3c.github.io/html-aam/#table-element

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I have updated the pull request with a new commit.

  • I have added the body2 typography to the caption
  • I have added a small accessibility and caption section in the documentation
  • I have removed the caption from the other table demos for simplicity. I could only find a design system that shows the caption support: Bootstrap and even then, it's scoped to a single demo. I don't think that it's something developers want in the majority of the cases (outside of the morality of it). I would expect the majority to use the component to build large interactive tables.

Do you guys any concerns left to be addressed?

@eps1lon
Copy link
Member

eps1lon commented Oct 9, 2019

I don't think that it's something developers want in the majority of the cases (outside of the morality of it).

I'd argue that aria-label is perfectly fine if you have a single table per section. For screen reader users caption and aria-label should be equivalent and for other visual impairments the heading of that section is another visual anchor.

Captions are a fine thing either way. I hope more people would to add them to their tables since that's what you're supposed to do with your tables. Just like you label your diagrams. Or eat your vitamins 😄

@ahayes91
Copy link

ahayes91 commented Oct 9, 2019

@ahayes91 this bit might bite us in the behind later...

position: fixed;
bottom: 200vh;

This is perfect for desktop but on mobile you might end up with unexpected focusing behaviour. It might very well work fine, but worth testing before merging.

Dead right, VoiceOver on iPad focus was going off screen when navigating to the table. Changed the CSS to use clip instead and it also made the ChromeVox focus better in table navigation mode:

  position: absolute;
  clip: rect(1px, 1px, 1px, 1px);
  border: 0;
  height: 1px;
  margin: -1px;
  overflow: hidden;
  padding: 0;
  width: 1px;
  white-space: nowrap;

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. PR: ready to ship and removed good first issue Great for first contributions. Enable to learn the contribution process. labels Oct 9, 2019
@oliviertassinari oliviertassinari merged commit e3a6c73 into mui:master Oct 10, 2019
@oliviertassinari
Copy link
Member

@adeelibr Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: table This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A11y: Material UI tables should have option to provide caption/aria-labelledby for screen reader users
6 participants