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

[Autocomplete] Fix hidden items during keyboard navigation #19305

Merged

Conversation

aisamu
Copy link
Contributor

@aisamu aisamu commented Jan 20, 2020

This PR implements @oliviertassinari's suggestion for #19217 with a couple tweaks

The differences were mainly to preserve the existing behaviour on lists without groups and on the downward scrolling of lists with groups.

Original Original suggestion This PR

comparison-small

While scrolling up with the keyboard, position the top part of the
option so that the both the group label and the row are fully visible.
No changes were made for the scroll down behaviour, which already
handled skipping the group label.

Doesn't interfere with the current behaviour of autoselects without
groups.

The 1.3 ratio was selected through visual inspection, and is the value
that minimizes misaligments and/or cropping of the topmost option.

Fixes #19217

Some guidance is definitely needed on how to test this behaviour given its dynamic nature. Would it be possible to send keystrokes to the element before taking a visual regression snapshot, for example?

While scrolling up with the keyboard, position the top part of the
option so that the both the group label and the row are fully visible.
No changes were made for the scroll down behaviour, which already
handled skipping the group label.

Doesn't interfere with the current behaviour of autoselects without
groups.

The 1.3 ratio was selected through visual inspection, and is the value
that minimizes misaligments and/or cropping of the topmost option.
@mui-pr-bot
Copy link

Details of bundle changes.

Comparing: f1080b6...0671abb

bundle Size Change Size Gzip Change Gzip
@material-ui/lab ▲ +50 B (+0.03% ) 185 kB ▲ +16 B (+0.03% ) 55.2 kB
useAutocomplete ▲ +50 B (+0.34% ) 14.6 kB ▲ +15 B (+0.29% ) 5.26 kB
Autocomplete ▲ +50 B (+0.04% ) 132 kB ▲ +14 B (+0.03% ) 41.3 kB
@material-ui/core -- 361 kB -- 98.7 kB
@material-ui/core[umd] -- 317 kB -- 91.9 kB
@material-ui/styles -- 51.2 kB -- 15.4 kB
@material-ui/system -- 14.8 kB -- 4.07 kB
Alert -- 83.9 kB -- 26.3 kB
AlertTitle -- 64.3 kB -- 20.2 kB
AppBar -- 64.1 kB -- 20.1 kB
Avatar -- 65.4 kB -- 20.6 kB
AvatarGroup -- 62.4 kB -- 19.6 kB
Backdrop -- 68 kB -- 21 kB
Badge -- 65.4 kB -- 20.4 kB
BottomNavigation -- 62.5 kB -- 19.6 kB
BottomNavigationAction -- 75.7 kB -- 23.9 kB
Box -- 71 kB -- 21.7 kB
Breadcrumbs -- 67.9 kB -- 21.3 kB
Button -- 79.9 kB -- 24.5 kB
ButtonBase -- 74.2 kB -- 23.3 kB
ButtonGroup -- 83.4 kB -- 25.5 kB
Card -- 63 kB -- 19.7 kB
CardActionArea -- 75.3 kB -- 23.7 kB
CardActions -- 62.2 kB -- 19.5 kB
CardContent -- 62.1 kB -- 19.4 kB
CardHeader -- 65.2 kB -- 20.5 kB
CardMedia -- 62.5 kB -- 19.7 kB
Checkbox -- 83.2 kB -- 26.3 kB
Chip -- 82.8 kB -- 25.4 kB
CircularProgress -- 64.2 kB -- 20.3 kB
ClickAwayListener -- 3.91 kB -- 1.55 kB
Collapse -- 68.2 kB -- 21.1 kB
colorManipulator -- 3.88 kB -- 1.52 kB
Container -- 63.4 kB -- 19.8 kB
CssBaseline -- 57.7 kB -- 18.1 kB
Dialog -- 83.2 kB -- 25.9 kB
DialogActions -- 62.2 kB -- 19.5 kB
DialogContent -- 62.3 kB -- 19.5 kB
DialogContentText -- 64.2 kB -- 20.2 kB
DialogTitle -- 64.4 kB -- 20.2 kB
Divider -- 62.7 kB -- 19.7 kB
docs.landing -- 51 kB -- 13.5 kB
docs.main -- 617 kB -- 196 kB
Drawer -- 85 kB -- 25.8 kB
ExpansionPanel -- 72.5 kB -- 22.7 kB
ExpansionPanelActions -- 62.2 kB -- 19.5 kB
ExpansionPanelDetails -- 62.1 kB -- 19.4 kB
ExpansionPanelSummary -- 78.3 kB -- 24.7 kB
Fab -- 77 kB -- 24 kB
Fade -- 23.6 kB -- 8 kB
FilledInput -- 73.7 kB -- 22.9 kB
FormControl -- 64.5 kB -- 20.1 kB
FormControlLabel -- 65.6 kB -- 20.6 kB
FormGroup -- 62.1 kB -- 19.5 kB
FormHelperText -- 63.5 kB -- 20 kB
FormLabel -- 63.6 kB -- 19.7 kB
Grid -- 65.2 kB -- 20.4 kB
GridList -- 62.6 kB -- 19.7 kB
GridListTile -- 63.9 kB -- 20 kB
GridListTileBar -- 63.3 kB -- 19.9 kB
Grow -- 24.2 kB -- 8.21 kB
Hidden -- 66.1 kB -- 20.7 kB
Icon -- 62.9 kB -- 19.7 kB
IconButton -- 76.3 kB -- 23.8 kB
Input -- 72.6 kB -- 22.7 kB
InputAdornment -- 65.2 kB -- 20.5 kB
InputBase -- 70.8 kB -- 22.2 kB
InputLabel -- 65.5 kB -- 20.2 kB
LinearProgress -- 65.4 kB -- 20.5 kB
Link -- 66.7 kB -- 21.1 kB
List -- 62.5 kB -- 19.5 kB
ListItem -- 77.3 kB -- 24.2 kB
ListItemAvatar -- 62.2 kB -- 19.5 kB
ListItemIcon -- 62.3 kB -- 19.5 kB
ListItemSecondaryAction -- 62.1 kB -- 19.5 kB
ListItemText -- 65.1 kB -- 20.4 kB
ListSubheader -- 62.9 kB -- 19.8 kB
Menu -- 88.9 kB -- 27.4 kB
MenuItem -- 78.4 kB -- 24.5 kB
MenuList -- 66.2 kB -- 20.7 kB
MobileStepper -- 68 kB -- 21.3 kB
Modal -- 14.5 kB -- 5.05 kB
NativeSelect -- 77 kB -- 24.3 kB
NoSsr -- 2.19 kB -- 1.04 kB
OutlinedInput -- 74.1 kB -- 23.1 kB
Paper -- 62.5 kB -- 19.5 kB
Popover -- 83.3 kB -- 25.8 kB
Popper -- 28.8 kB -- 10.3 kB
Portal -- 2.92 kB -- 1.3 kB
Radio -- 84.3 kB -- 26.6 kB
RadioGroup -- 64.6 kB -- 20 kB
Rating -- 70.6 kB -- 22.7 kB
RootRef -- 4.24 kB -- 1.64 kB
Select -- 116 kB -- 34.4 kB
Skeleton -- 63 kB -- 19.9 kB
Slide -- 25.6 kB -- 8.73 kB
Slider -- 76.8 kB -- 24.3 kB
Snackbar -- 75.5 kB -- 23.6 kB
SnackbarContent -- 63.7 kB -- 20.1 kB
SpeedDial -- 86.4 kB -- 27.2 kB
SpeedDialAction -- 118 kB -- 37.5 kB
SpeedDialIcon -- 64.7 kB -- 20.3 kB
Step -- 62.8 kB -- 19.7 kB
StepButton -- 82.5 kB -- 26.1 kB
StepConnector -- 62.8 kB -- 19.8 kB
StepContent -- 69.3 kB -- 21.7 kB
StepIcon -- 64.8 kB -- 20.2 kB
StepLabel -- 68.7 kB -- 21.7 kB
Stepper -- 65 kB -- 20.5 kB
styles/createMuiTheme -- 16.6 kB -- 5.83 kB
SvgIcon -- 63.2 kB -- 19.8 kB
SwipeableDrawer -- 92.4 kB -- 28.9 kB
Switch -- 82.4 kB -- 26 kB
Tab -- 76.5 kB -- 24.2 kB
Table -- 62.7 kB -- 19.7 kB
TableBody -- 62.2 kB -- 19.5 kB
TableCell -- 64.2 kB -- 20.2 kB
TableContainer -- 62.1 kB -- 19.4 kB
TableFooter -- 62.2 kB -- 19.5 kB
TableHead -- 62.2 kB -- 19.5 kB
TablePagination -- 143 kB -- 41.8 kB
TableRow -- 62.6 kB -- 19.6 kB
TableSortLabel -- 77.6 kB -- 24.4 kB
Tabs -- 85.8 kB -- 27.2 kB
TextareaAutosize -- 5.12 kB -- 2.14 kB
TextField -- 125 kB -- 36.5 kB
ToggleButton -- 76.3 kB -- 24.2 kB
ToggleButtonGroup -- 63.3 kB -- 19.9 kB
Toolbar -- 62.5 kB -- 19.6 kB
Tooltip -- 102 kB -- 32.3 kB
TreeItem -- 74.1 kB -- 23.5 kB
TreeView -- 66.8 kB -- 21 kB
Typography -- 63.8 kB -- 20 kB
useMediaQuery -- 2.51 kB -- 1.06 kB
Zoom -- 23.6 kB -- 8.12 kB

Generated by 🚫 dangerJS against 0671abb

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.

Could you slow down the GIF? I can't see any difference.

Would it be possible to send keystrokes to the element before taking a visual regression snapshot, for example?

I don't know of any and wouldn't spend too much time on it. At some point I want to improve how we do visual regression tests. So far they are good enough.

@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work labels Jan 20, 2020
@aisamu
Copy link
Contributor Author

aisamu commented Jan 20, 2020

Could you slow down the GIF? I can't see any difference.

Sure!

Original Original suggestion This PR

comparison-small

Issue on the first one:

  • While scrolling up, the selection goes behind the groups' labels

Issues on the second one:

  • While scrolling up, the selected item is slightly cropped by the label (see screenshot below)
  • While scrolling down, the selected item is not at the bottom, but on the second-to-last position

Screen Shot 2020-01-20 at 09 06 10

@oliviertassinari oliviertassinari changed the title [Autocomplete] Fix group labels hiding items during keyboard navigation [Autocomplete] Fix hidden items during keyboard navigation Jan 20, 2020
@oliviertassinari oliviertassinari merged commit 0d56ff2 into mui:master Jan 20, 2020
@oliviertassinari
Copy link
Member

@aisamu Thank you!

@aisamu
Copy link
Contributor Author

aisamu commented Jan 20, 2020

@oliviertassinari oh, don't mention it! With all the hand holding and a suggestion that took it almost all the way, you're the one that should be thanked 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: autocomplete 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.

[Autocomplete] Group labels hide the selection when navigating with keyboard
4 participants