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.

Sign up for GitHub

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

Jump to bottom

[Breadcrumbs] Fix expand/collapsed Breadcrumbs via keyboard #19724

Merged
merged 11 commits into from
Feb 20, 2020
Merged

[Breadcrumbs] Fix expand/collapsed Breadcrumbs via keyboard #19724

merged 11 commits into from
Feb 20, 2020

Conversation

captain-yossarian
Copy link
Contributor

@captain-yossarian captain-yossarian commented Feb 15, 2020 •
edited by oliviertassinari
Loading

Closes #19708

@mui-pr-bot
Copy link

mui-pr-bot commented Feb 15, 2020 •
edited
Loading

Details of bundle changes.

Comparing: f2d74e9...dc883e6

bundle Size Change Size Gzip Change Gzip
Breadcrumbs ▲ +12.7 kB (+18.63% ) 80.8 kB ▲ +4.06 kB (+19.01% ) 25.4 kB
@material-ui/core ▲ +107 B (+0.03% ) 362 kB ▲ +47 B (+0.05% ) 98.9 kB
@material-ui/core[umd] ▲ +98 B (+0.03% ) 318 kB ▲ +36 B (+0.04% ) 92 kB
docs.main ▲ +48 B (+0.01% ) 604 kB ▲ +34 B (+0.02% ) 196 kB
ButtonBase ▼ -2 B (-0.00% ) 74.4 kB ▼ -2 B (-0.01% ) 23.3 kB
Paper ▲ +1 B (0.00% ) 62.7 kB ▼ -1 B (-0.01% ) 19.6 kB
SvgIcon ▲ +1 B (0.00% ) 63.4 kB ▼ -1 B (-0.01% ) 19.8 kB
StepButton -- 82.7 kB ▼ -11 B (-0.04% ) 26.1 kB
TextField -- 126 kB ▲ +8 B (+0.02% ) 36.6 kB
Autocomplete -- 132 kB ▼ -6 B (-0.01% ) 41.4 kB
Rating -- 70.8 kB ▼ -6 B (-0.03% ) 22.8 kB
TablePagination -- 144 kB ▲ +6 B (+0.01% ) 42 kB
Snackbar -- 75.7 kB ▼ -4 B (-0.02% ) 23.7 kB
Button -- 80.1 kB ▼ -3 B (-0.01% ) 24.5 kB
Drawer -- 85.2 kB ▼ -3 B (-0.01% ) 25.9 kB
ExpansionPanelActions -- 62.4 kB ▼ -3 B (-0.02% ) 19.6 kB
ExpansionPanelSummary -- 78.5 kB ▼ -3 B (-0.01% ) 24.8 kB
Menu -- 89.1 kB ▼ -3 B (-0.01% ) 27.4 kB
MenuItem -- 78.6 kB ▼ -3 B (-0.01% ) 24.5 kB
RadioGroup -- 64.8 kB ▼ -3 B (-0.01% ) 20.1 kB
SpeedDial -- 86.6 kB ▼ -3 B (-0.01% ) 27.3 kB
Tabs -- 85.8 kB ▼ -3 B (-0.01% ) 27.2 kB
Backdrop -- 68.2 kB ▼ -2 B (-0.01% ) 21.1 kB
BottomNavigation -- 62.7 kB ▼ -2 B (-0.01% ) 19.7 kB
ButtonGroup -- 83.6 kB ▼ -2 B (-0.01% ) 25.6 kB
CardActionArea -- 75.5 kB ▼ -2 B (-0.01% ) 23.8 kB
CardHeader -- 65.4 kB ▲ +2 B (+0.01% ) 20.6 kB
CardMedia -- 62.7 kB ▼ -2 B (-0.01% ) 19.7 kB
Checkbox -- 83.4 kB ▼ -2 B (-0.01% ) 26.3 kB
Chip -- 83 kB ▼ -2 B (-0.01% ) 25.4 kB
Collapse -- 68.4 kB ▼ -2 B (-0.01% ) 21.2 kB
Fab -- 77.2 kB ▼ -2 B (-0.01% ) 24 kB
IconButton -- 76.5 kB ▼ -2 B (-0.01% ) 23.9 kB
ListItem -- 77.5 kB ▼ -2 B (-0.01% ) 24.2 kB
Pagination -- 85.5 kB ▼ -2 B (-0.01% ) 26.3 kB
PaginationItem -- 81.2 kB ▼ -2 B (-0.01% ) 25 kB
Popover -- 83.5 kB ▼ -2 B (-0.01% ) 25.8 kB
Slider -- 77 kB ▼ -2 B (-0.01% ) 24.2 kB
SwipeableDrawer -- 92.6 kB ▼ -2 B (-0.01% ) 28.9 kB
Tab -- 76.7 kB ▼ -2 B (-0.01% ) 24.2 kB
TableSortLabel -- 77.8 kB ▼ -2 B (-0.01% ) 24.4 kB
Tooltip -- 103 kB ▼ -2 B (-0.01% ) 32.4 kB
TreeItem -- 74.3 kB ▲ +2 B (+0.01% ) 23.5 kB
Alert -- 83.8 kB ▼ -1 B (-0.00% ) 26.3 kB
AlertTitle -- 64.5 kB ▼ -1 B (-0.00% ) 20.3 kB
AppBar -- 64.4 kB ▼ -1 B (-0.00% ) 20.2 kB
Avatar -- 65.6 kB ▼ -1 B (-0.00% ) 20.7 kB
AvatarGroup -- 62.6 kB ▼ -1 B (-0.01% ) 19.7 kB
BottomNavigationAction -- 75.9 kB ▼ -1 B (-0.00% ) 24 kB
Box -- 72.2 kB ▼ -1 B (-0.00% ) 21.9 kB
Card -- 63.2 kB ▼ -1 B (-0.01% ) 19.8 kB
CardActions -- 62.4 kB ▼ -1 B (-0.01% ) 19.6 kB
CardContent -- 62.3 kB ▼ -1 B (-0.01% ) 19.5 kB
CircularProgress -- 64.4 kB ▼ -1 B (-0.00% ) 20.3 kB
Container -- 63.5 kB ▼ -1 B (-0.01% ) 19.9 kB
CssBaseline -- 62.3 kB ▼ -1 B (-0.01% ) 19.6 kB
Dialog -- 83.4 kB ▼ -1 B (-0.00% ) 26 kB
DialogActions -- 62.4 kB ▼ -1 B (-0.01% ) 19.6 kB
DialogTitle -- 64.6 kB ▼ -1 B (-0.00% ) 20.3 kB
ExpansionPanel -- 72.7 kB ▼ -1 B (-0.00% ) 22.7 kB
ExpansionPanelDetails -- 62.3 kB ▼ -1 B (-0.01% ) 19.5 kB
FilledInput -- 73.9 kB ▼ -1 B (-0.00% ) 23 kB
FormControl -- 64.8 kB ▼ -1 B (-0.00% ) 20.2 kB
FormGroup -- 62.3 kB ▼ -1 B (-0.01% ) 19.6 kB
FormHelperText -- 63.7 kB ▼ -1 B (-0.00% ) 20 kB
FormLabel -- 63.8 kB ▼ -1 B (-0.01% ) 19.8 kB
Grid -- 65.4 kB ▼ -1 B (-0.00% ) 20.5 kB
GridListTile -- 64.1 kB ▼ -1 B (-0.00% ) 20.1 kB
GridListTileBar -- 63.6 kB ▼ -1 B (-0.01% ) 19.9 kB
Hidden -- 66.3 kB ▲ +1 B (0.00% ) 20.8 kB
Icon -- 63.1 kB ▼ -1 B (-0.01% ) 19.8 kB
Input -- 72.9 kB ▼ -1 B (-0.00% ) 22.8 kB
InputBase -- 71 kB ▼ -1 B (-0.00% ) 22.3 kB
InputLabel -- 65.7 kB ▼ -1 B (-0.00% ) 20.5 kB
LinearProgress -- 65.7 kB ▼ -1 B (-0.00% ) 20.5 kB
Link -- 67 kB ▼ -1 B (-0.00% ) 21.1 kB
ListItemIcon -- 62.5 kB ▼ -1 B (-0.01% ) 19.6 kB
ListItemSecondaryAction -- 62.3 kB ▼ -1 B (-0.01% ) 19.5 kB
ListSubheader -- 63.1 kB ▼ -1 B (-0.01% ) 19.8 kB
MobileStepper -- 68.2 kB ▼ -1 B (-0.00% ) 21.4 kB
NoSsr -- 2.19 kB ▲ +1 B (+0.10% ) 1.04 kB
OutlinedInput -- 74.9 kB ▼ -1 B (-0.00% ) 23.3 kB
Popper -- 28.8 kB ▼ -1 B (-0.01% ) 10.3 kB
RootRef -- 4.24 kB ▲ +1 B (+0.06% ) 1.64 kB
ScopedCssBaseline -- 63.2 kB ▼ -1 B (-0.01% ) 19.9 kB
Select -- 117 kB ▲ +1 B (0.00% ) 34.6 kB
Slide -- 25.7 kB ▲ +1 B (+0.01% ) 8.75 kB
SnackbarContent -- 63.9 kB ▼ -1 B (-0.00% ) 20.1 kB
SpeedDialAction -- 119 kB ▼ -1 B (-0.00% ) 37.6 kB
SpeedDialIcon -- 64.9 kB ▲ +1 B (0.00% ) 20.3 kB
StepConnector -- 63.1 kB ▼ -1 B (-0.01% ) 19.9 kB
StepContent -- 69.5 kB ▼ -1 B (-0.00% ) 21.8 kB
StepIcon -- 65 kB ▼ -1 B (-0.00% ) 20.3 kB
StepLabel -- 69 kB ▼ -1 B (-0.00% ) 21.7 kB
Stepper -- 65.2 kB ▼ -1 B (-0.00% ) 20.6 kB
styles/createMuiTheme -- 16.6 kB ▼ -1 B (-0.02% ) 5.85 kB
TableCell -- 64.4 kB ▼ -1 B (-0.00% ) 20.3 kB
TableContainer -- 62.3 kB ▼ -1 B (-0.01% ) 19.5 kB
TableFooter -- 62.5 kB ▼ -1 B (-0.01% ) 19.5 kB
TableHead -- 62.4 kB ▼ -1 B (-0.01% ) 19.5 kB
TableRow -- 62.8 kB ▼ -1 B (-0.01% ) 19.7 kB
ToggleButton -- 76.5 kB ▼ -1 B (-0.00% ) 24.2 kB
Toolbar -- 62.7 kB ▼ -1 B (-0.01% ) 19.7 kB
Typography -- 64 kB ▼ -1 B (-0.00% ) 20 kB
@material-ui/lab -- 194 kB -- 57.2 kB
@material-ui/styles -- 51.4 kB -- 15.4 kB
@material-ui/system -- 16.5 kB -- 4.32 kB
Badge -- 65.7 kB -- 20.4 kB
ClickAwayListener -- 3.91 kB -- 1.55 kB
colorManipulator -- 3.88 kB -- 1.52 kB
DialogContent -- 62.6 kB -- 19.6 kB
DialogContentText -- 64.4 kB -- 20.2 kB
Divider -- 63 kB -- 19.8 kB
docs.landing -- 56.8 kB -- 15.6 kB
Fade -- 23.6 kB -- 8.01 kB
FormControlLabel -- 65.9 kB -- 20.6 kB
GridList -- 62.8 kB -- 19.7 kB
Grow -- 24.2 kB -- 8.22 kB
InputAdornment -- 65.4 kB -- 20.6 kB
List -- 62.7 kB -- 19.6 kB
ListItemAvatar -- 62.5 kB -- 19.5 kB
ListItemText -- 65.3 kB -- 20.5 kB
MenuList -- 66.4 kB -- 20.8 kB
Modal -- 14.5 kB -- 5.04 kB
NativeSelect -- 77.2 kB -- 24.3 kB
Portal -- 2.92 kB -- 1.3 kB
Radio -- 84.4 kB -- 26.6 kB
Skeleton -- 63.3 kB -- 20 kB
Step -- 63 kB -- 19.8 kB
Switch -- 82.5 kB -- 26 kB
Table -- 62.9 kB -- 19.7 kB
TableBody -- 62.4 kB -- 19.5 kB
TextareaAutosize -- 5.12 kB -- 2.14 kB
ToggleButtonGroup -- 63.5 kB -- 20 kB
TreeView -- 67 kB -- 21.1 kB
useAutocomplete -- 14.7 kB -- 5.31 kB
useMediaQuery -- 2.58 kB -- 1.06 kB
Zoom -- 23.6 kB -- 8.12 kB

Generated by 🚫 dangerJS against dc883e6

@oliviertassinari oliviertassinari added the component: breadcrumbs This is the name of the generic UI component, not the React module! label Feb 16, 2020
eps1lon
eps1lon reviewed Feb 16, 2020
View reviewed changes
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Shouldn't we change the icon to implement the button role? I'm not sure about this implementation concerning a11y.

packages/material-ui/src/Breadcrumbs/Breadcrumbs.js Outdated Show resolved Hide resolved
@captain-yossarian
Copy link
Contributor Author

@oliviertassinari @eps1lon Could you please check a logic? If it is ok, I will change the tests.

oliviertassinari
oliviertassinari reviewed Feb 18, 2020
View reviewed changes
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.

We are getting closer. I still think that we need an aria label on the expand icon, otherwise, how can screenreaders know about it's impact?

packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.js Outdated Show resolved Hide resolved
packages/material-ui/src/Breadcrumbs/BreadcrumbCollapsed.test.js Outdated Show resolved Hide resolved
oliviertassinari
oliviertassinari reviewed Feb 18, 2020
View reviewed changes
packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
@@ -52,6 +52,7 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {
classes,
className,
component: Component = 'nav',
expandText = 'Show path',
Copy link
Member

@oliviertassinari oliviertassinari Feb 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Taken from the aria-label Google Drive uses.

This was referenced Feb 19, 2020
Closed
Merged
@oliviertassinari
Copy link
Member

I have rebased on top of #19784 and improved the ripple + hover style.

@oliviertassinari oliviertassinari merged commit 22ce678 into mui:master Feb 20, 2020
@oliviertassinari
Copy link
Member

@captain-yossarian Thank you for spending time on it! It's a really great improvement for accessibility :)

This was referenced Mar 10, 2020
Merged
Closed
Open
This was referenced Mar 21, 2020
Merged
Open
Closed
EsoterikStare pushed a commit to EsoterikStare/material-ui that referenced this pull request Mar 30, 2020
@captain-yossarian @EsoterikStare
[Breadcrumbs] Fix expand/collapsed Breadcrumbs via keyboard (mui#19724)
a336ae8
This was referenced Apr 16, 2020
Merged
Closed
@snyk-bot snyk-bot mentioned this pull request May 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

@eps1lon eps1lon Awaiting requested review from eps1lon

Assignees
No one assigned
Labels
accessibility a11y component: breadcrumbs This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Breadcrumbs] Can't expand collapsed Breadcrumbs via keyboard
4 participants
@captain-yossarian @mui-pr-bot @oliviertassinari @eps1lon