[Autocomplete] Listen for click on the root element#36369
[Autocomplete] Listen for click on the root element#36369siriwatknp merged 19 commits intomui:masterfrom
Conversation
Netlify deploy previewhttps://deploy-preview-36369--material-ui.netlify.app/ Bundle size report |
zannager
added
the
scope: autocomplete
|
@sai6855 Your changes have introduced another bug: Grabacion.de.pantalla.2023-03-01.a.las.15.37.01.mov |
Nice catch, I'll check |
@marcpachecog Fixed the bug you found out and i've updated PR description with new video. @siriwatknp @ZeeshanTamboli PR is ready for review now |
| if (event.currentTarget === event.target) { | ||
| handleInputMouseDown(); | ||
| } |
There was a problem hiding this comment.
I think it should also focus on the input as well, otherwise I can't close the popup on blur in this case. I mouse down on the border and then mouse up on one of the options, after that I cannot close the popup when click outside of the autocomplete.
Screen.Recording.2566-03-11.at.15.03.18.mov
There was a problem hiding this comment.
@siriwatknp moved all logic to onClick, now bug you found is fixed.
| // Focus the input when interacting with the combobox | ||
| const handleClick = () => { | ||
| const handleClick = (event) => { | ||
| inputRef.current.focus(); |
There was a problem hiding this comment.
To see if it makes sense to use this bug as an opportunity to fix this line. Focusing on click no sense per #29381 (comment)
There was a problem hiding this comment.
@oliviertassinari i had changed logic, since previous logic wasn't working for material-ui Autocomplete. With updated logic, i don't know if the your proposed fix is relevant to fix in this PR.
| handleRootOnClick(event); | ||
| } | ||
| if (event.currentTarget === event.target && handleInputMouseDown) { | ||
| handleInputMouseDown(event as React.MouseEvent<HTMLInputElement, MouseEvent>); |
There was a problem hiding this comment.
handleInputMouseDown expects event to be coming from input but here we have access to event of div element. that's why had to cast the type
| ownerState, | ||
| getSlotProps: getRootProps, | ||
| additionalProps: { | ||
| onClick: (event: React.MouseEvent<HTMLDivElement, MouseEvent>) => { |
There was a problem hiding this comment.
this onClick overrides onClick from https://github.com/mui/material-ui/blob/master/packages/mui-base/src/useAutocomplete/useAutocomplete.js#L1084 , hence onClick coming from useAutoComplete is called again here.
| className: classes.inputRoot, | ||
| startAdornment, | ||
| onClick: (event) => { | ||
| if (event.target === event.currentTarget) { |
There was a problem hiding this comment.
Check to make sure, handleInputMouseDown is called only when user clicks on input element.
| if (handleRootOnClick) { | ||
| handleRootOnClick(event); | ||
| } | ||
| if (event.currentTarget === event.target && handleInputMouseDown) { |
There was a problem hiding this comment.
Check to make sure, handleInputMouseDown is called only when user clicks on input element.
| }); | ||
| }); | ||
|
|
||
| it('should open popup when clicked on borders of input element', () => { |
There was a problem hiding this comment.
I don't think the description matches what you are testing. It should be : 'should open popup when clicked on the root element'.
| }); | ||
| }); | ||
|
|
||
| it('should open popup when clicked on borders of input element', () => { |
mnajdova
changed the title
8 participants
closes #36364
preview: https://deploy-preview-36369--material-ui.netlify.app/joy-ui/react-autocomplete/
joy UI working video
screen-recording-2023-03-04-at-42306-pm_yYbkTH3w.mov
material ui Working video
https://www.loom.com/share/5386e85fa34049b2b481e44abfdade42