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

[Autocomplete] Better synchronize the highlight with the value #19923

Merged
merged 7 commits into from
Mar 3, 2020
Merged

[Autocomplete] Better synchronize the highlight with the value #19923

merged 7 commits into from
Mar 3, 2020

Conversation

captain-yossarian
Copy link
Contributor

@captain-yossarian captain-yossarian commented Mar 1, 2020 •
edited by oliviertassinari
Loading

Closes #19779
Closes #19637
Closes #19705

@captain-yossarian captain-yossarian changed the title [Autocomplete] Select option scroll drop list to defaultValue [wip][Autocomplete] Select option scroll drop list to defaultValue Mar 1, 2020
@mui-pr-bot
Copy link

mui-pr-bot commented Mar 1, 2020 •
edited
Loading

Details of bundle changes.

Comparing: 33a7c26...a33ae0f

bundle Size Change Size Gzip Change Gzip
@material-ui/lab ▲ +214 B (+0.11% ) 199 kB ▲ +74 B (+0.13% ) 58.9 kB
Autocomplete ▲ +214 B (+0.16% ) 134 kB ▲ +68 B (+0.16% ) 42.1 kB
useAutocomplete ▲ +214 B (+1.47% ) 14.7 kB ▲ +68 B (+1.30% ) 5.32 kB
ButtonBase -- 77.3 kB ▼ -5 B (-0.02% ) 24.2 kB
ButtonGroup -- 86.5 kB ▼ -5 B (-0.02% ) 26.6 kB
FilledInput -- 76.9 kB ▼ -5 B (-0.02% ) 23.9 kB
InputBase -- 74 kB ▼ -5 B (-0.02% ) 23.2 kB
MenuItem -- 81.4 kB ▼ -5 B (-0.02% ) 25.4 kB
Pagination -- 87.6 kB ▼ -5 B (-0.02% ) 27 kB
Tooltip -- 105 kB ▼ -5 B (-0.02% ) 33.1 kB
Drawer -- 87.9 kB ▼ -4 B (-0.01% ) 26.8 kB
Fab -- 80.1 kB ▼ -4 B (-0.02% ) 24.9 kB
Fade -- 27.1 kB ▼ -4 B (-0.04% ) 9.05 kB
IconButton -- 79.4 kB ▼ -4 B (-0.02% ) 24.8 kB
Input -- 75.9 kB ▼ -4 B (-0.02% ) 23.7 kB
ListItem -- 80.3 kB ▼ -4 B (-0.02% ) 25.1 kB
OutlinedInput -- 77.9 kB ▼ -4 B (-0.02% ) 24.2 kB
Snackbar -- 78.6 kB ▼ -4 B (-0.02% ) 24.5 kB
Button -- 83 kB ▼ -3 B (-0.01% ) 25.4 kB
Chip -- 85.8 kB ▼ -3 B (-0.01% ) 26.3 kB
Dialog -- 86.2 kB ▼ -3 B (-0.01% ) 26.9 kB
Menu -- 91.8 kB ▼ -3 B (-0.01% ) 28.3 kB
StepButton -- 85.5 kB ▼ -3 B (-0.01% ) 27 kB
SwipeableDrawer -- 95.3 kB ▼ -3 B (-0.01% ) 29.9 kB
Tab -- 79.6 kB ▼ -3 B (-0.01% ) 25.3 kB
TextareaAutosize -- 5.19 kB ▲ +3 B (+0.14% ) 2.17 kB
ToggleButton -- 79.4 kB ▼ -3 B (-0.01% ) 25.1 kB
Alert -- 86.6 kB ▼ -2 B (-0.01% ) 27.3 kB
Backdrop -- 71.2 kB ▼ -2 B (-0.01% ) 22 kB
Breadcrumbs -- 83.6 kB ▼ -2 B (-0.01% ) 26.5 kB
Checkbox -- 85.4 kB ▼ -2 B (-0.01% ) 27 kB
Grow -- 27.7 kB ▼ -2 B (-0.02% ) 9.26 kB
Popper -- 28.8 kB ▲ +2 B (+0.02% ) 10.3 kB
SpeedDial -- 89.4 kB ▼ -2 B (-0.01% ) 28.3 kB
Switch -- 84.6 kB ▼ -2 B (-0.01% ) 26.6 kB
Tabs -- 88.6 kB ▼ -2 B (-0.01% ) 28.3 kB
@material-ui/core -- 359 kB ▲ +1 B (0.00% ) 98.8 kB
BottomNavigationAction -- 78.8 kB ▼ -1 B (-0.00% ) 24.9 kB
CardActionArea -- 78.4 kB ▼ -1 B (-0.00% ) 24.8 kB
ExpansionPanelSummary -- 81.4 kB ▼ -1 B (-0.00% ) 25.7 kB
Link -- 69.9 kB ▼ -1 B (-0.00% ) 22.1 kB
MenuList -- 69.3 kB ▲ +1 B (0.00% ) 21.7 kB
NativeSelect -- 80.1 kB ▼ -1 B (-0.00% ) 25.3 kB
PaginationItem -- 84 kB ▼ -1 B (-0.00% ) 25.9 kB
Radio -- 86.4 kB ▼ -1 B (-0.00% ) 27.3 kB
RadioGroup -- 67.1 kB ▼ -1 B (-0.00% ) 20.9 kB
Rating -- 73.7 kB ▼ -1 B (-0.00% ) 23.7 kB
SpeedDialAction -- 121 kB ▼ -1 B (-0.00% ) 38.4 kB
TablePagination -- 145 kB ▲ +1 B (0.00% ) 42.6 kB
TableSortLabel -- 80.6 kB ▼ -1 B (-0.00% ) 25.5 kB
TreeItem -- 78.5 kB ▼ -1 B (-0.00% ) 24.8 kB
Zoom -- 27.1 kB ▲ +1 B (+0.01% ) 9.18 kB
@material-ui/core[umd] -- 318 kB -- 92.1 kB
@material-ui/styles -- 51.4 kB -- 15.4 kB
@material-ui/system -- 16.5 kB -- 4.31 kB
AlertTitle -- 67.5 kB -- 21.2 kB
AppBar -- 67.4 kB -- 21.1 kB
Avatar -- 68.5 kB -- 21.7 kB
AvatarGroup -- 65.7 kB -- 20.6 kB
Badge -- 68.7 kB -- 21.3 kB
BottomNavigation -- 65.7 kB -- 20.6 kB
Box -- 72.3 kB -- 21.8 kB
Card -- 66.2 kB -- 20.7 kB
CardActions -- 65.4 kB -- 20.5 kB
CardContent -- 65.3 kB -- 20.4 kB
CardHeader -- 68.4 kB -- 21.5 kB
CardMedia -- 65.7 kB -- 20.6 kB
CircularProgress -- 67.4 kB -- 21.2 kB
ClickAwayListener -- 3.84 kB -- 1.54 kB
Collapse -- 71.3 kB -- 22.1 kB
colorManipulator -- 3.88 kB -- 1.52 kB
Container -- 66.5 kB -- 20.8 kB
CssBaseline -- 65.3 kB -- 20.5 kB
DialogActions -- 65.4 kB -- 20.5 kB
DialogContent -- 65.6 kB -- 20.5 kB
DialogContentText -- 67.4 kB -- 21.1 kB
DialogTitle -- 67.6 kB -- 21.2 kB
Divider -- 66 kB -- 20.7 kB
docs.landing -- 56.6 kB -- 15.6 kB
docs.main -- 602 kB -- 195 kB
ExpansionPanel -- 74.9 kB -- 23.5 kB
ExpansionPanelActions -- 65.4 kB -- 20.5 kB
ExpansionPanelDetails -- 65.3 kB -- 20.4 kB
FormControl -- 67.7 kB -- 21.1 kB
FormControlLabel -- 68.8 kB -- 21.6 kB
FormGroup -- 65.4 kB -- 20.5 kB
FormHelperText -- 66.7 kB -- 20.7 kB
FormLabel -- 66.8 kB -- 20.7 kB
Grid -- 68.4 kB -- 21.4 kB
GridList -- 65.8 kB -- 20.6 kB
GridListTile -- 67.1 kB -- 21 kB
GridListTileBar -- 66.6 kB -- 20.8 kB
Hidden -- 69.3 kB -- 21.7 kB
Icon -- 66.1 kB -- 20.7 kB
InputAdornment -- 68.4 kB -- 21.6 kB
InputLabel -- 68.6 kB -- 21.2 kB
LinearProgress -- 68.7 kB -- 21.2 kB
List -- 65.7 kB -- 20.5 kB
ListItemAvatar -- 65.4 kB -- 20.5 kB
ListItemIcon -- 65.5 kB -- 20.5 kB
ListItemSecondaryAction -- 65.4 kB -- 20.5 kB
ListItemText -- 68.3 kB -- 21.5 kB
ListSubheader -- 66.1 kB -- 20.8 kB
MobileStepper -- 71.2 kB -- 22.3 kB
Modal -- 14.3 kB -- 5.04 kB
NoSsr -- 2.17 kB -- 1.03 kB
Paper -- 65.7 kB -- 20.5 kB
Popover -- 86.2 kB -- 26.7 kB
Portal -- 2.87 kB -- 1.29 kB
RootRef -- 4.21 kB -- 1.64 kB
ScopedCssBaseline -- 66.2 kB -- 20.7 kB
Select -- 119 kB -- 35.3 kB
Skeleton -- 66.3 kB -- 20.9 kB
Slide -- 29.1 kB -- 9.78 kB
Slider -- 79.1 kB -- 25.2 kB
SnackbarContent -- 66.9 kB -- 21 kB
SpeedDialIcon -- 67.9 kB -- 21.3 kB
Step -- 66 kB -- 20.7 kB
StepConnector -- 66.1 kB -- 20.8 kB
StepContent -- 72.5 kB -- 22.6 kB
StepIcon -- 67.9 kB -- 21.2 kB
StepLabel -- 71.9 kB -- 22.3 kB
Stepper -- 68.2 kB -- 21.5 kB
styles/createMuiTheme -- 20.9 kB -- 7.27 kB
SvgIcon -- 66.4 kB -- 20.7 kB
Table -- 65.9 kB -- 20.6 kB
TableBody -- 65.4 kB -- 20.5 kB
TableCell -- 67.4 kB -- 21.2 kB
TableContainer -- 65.3 kB -- 20.4 kB
TableFooter -- 65.5 kB -- 20.5 kB
TableHead -- 65.4 kB -- 20.5 kB
TableRow -- 65.8 kB -- 20.6 kB
TextField -- 127 kB -- 37.4 kB
ToggleButtonGroup -- 66.5 kB -- 20.9 kB
Toolbar -- 65.7 kB -- 20.6 kB
TreeView -- 71.5 kB -- 22.4 kB
Typography -- 67 kB -- 20.9 kB
useMediaQuery -- 2.56 kB -- 1.06 kB

Generated by 🚫 dangerJS against a33ae0f

@captain-yossarian
Copy link
Contributor Author

@jonatanklosko Thanks for the test.

@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work labels Mar 1, 2020
oliviertassinari
oliviertassinari approved these changes Mar 1, 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.

I must say, the logic we are changing here is complex, I have rarely seen this in the repository. I have tried to update the tests to better cover the issues. I have also merged two effects.
The only part I'm not very happy about is that we ignore 3 dependencies in the effect: options, getOptionSelected, getOptionLabel but I think that it's a tradeoff for the best. I would like to avoid a case where the users rerender at each keystroke, leading to a stuck highlight. Maybe we can reconsider it later. At least we have these two great test cases.

@oliviertassinari
Copy link
Member

@captain-yossarian If you could double-check the changes, it would be highly appreciated, I'm not very confident.

@oliviertassinari oliviertassinari changed the title [wip][Autocomplete] Select option scroll drop list to defaultValue [Autocomplete] Better synchronize the highlight with the value Mar 1, 2020
@captain-yossarian
Copy link
Contributor Author

@oliviertassinari I will thoroughly check it and let you know

P.S. I did not know about #19705 issue

@captain-yossarian captain-yossarian mentioned this pull request Mar 1, 2020
Closed
@captain-yossarian
Copy link
Contributor Author

@oliviertassinari I have tested #19779 and #19637 and it works as expected.
As for #19705 I've asked here @jdoklovic to test it because his demo is little bit complicated to reproduce on my own

@oliviertassinari oliviertassinari merged commit 0ac8d41 into mui:master Mar 3, 2020
@oliviertassinari
Copy link
Member

@captain-yossarian Thanks

This was referenced Mar 20, 2020
Merged
Merged
Merged
Merged
Merged
Merged
@snyk-bot snyk-bot mentioned this pull request Mar 21, 2020
Merged
@oliviertassinari oliviertassinari mentioned this pull request Mar 28, 2020
Merged
EsoterikStare pushed a commit to EsoterikStare/material-ui that referenced this pull request Mar 30, 2020
@captain-yossarian @EsoterikStare
[Autocomplete] Better synchronize the highlight with the value (mui#1…
fb17e69 
…9923)
This was referenced Apr 10, 2020
Closed
Merged
Merged
Merged
@snyk-bot snyk-bot mentioned this pull request Apr 19, 2020
Closed
@snyk-bot snyk-bot mentioned this pull request May 4, 2020
Closed
@oliviertassinari oliviertassinari mentioned this pull request Sep 20, 2020
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eps1lon eps1lon eps1lon left review comments

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
4 participants
@captain-yossarian @mui-pr-bot @oliviertassinari @eps1lon