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] unexpectly changes field value on mouse hover #20602

Open
2 tasks done
goffioul opened this issue Apr 16, 2020 · 23 comments
Open
2 tasks done

[Autocomplete] unexpectly changes field value on mouse hover #20602

goffioul opened this issue Apr 16, 2020 · 23 comments
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@goffioul
Copy link
Contributor

goffioul commented Apr 16, 2020

An Autocomplete component with freeSolo and autoSelect changes the field value by simply hovering the mouse over the dropdown menu and then bluring the field.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The problem is illustrated in this sandbox: https://codesandbox.io/s/trusting-dewdney-cvj7ss?file=/src/App.js

In the first field, click the popup arrow icon to open the dropdown menu, then hover the mouse down such that menu items are successively highlighted. Continue to move mouse down outside of the menu area and click anywhere to blur the field.

The field value (and associated state) will be changed to the last menu item that was hovered (and highlighted).

Expected Behavior 🤔

As the user hasn't made any selection in the dropdown menu, the field value shouldn't change.

Context 🔦

The use case is to create a free text entry with a set of suggested values for quick entry.

Your Environment 🌎

Tech Version
Material-UI v4.9.10
Material-UI lab v4.0.0-alpha.49
React 16.12.0
@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Apr 16, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 16, 2020

@goffioul This is one of the reasons why this props combination isn't used by default by the Autocomplete. autoSelect is meant to be used with disableCloseOnSelect.

However, I think that there is room for improvement here. What do you think of this patch?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 3cb1ca0c5..4de19514a 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -790,6 +790,10 @@ export default function useAutocomplete(props) {
     setHighlightedIndex(index, 'mouse');
   };

+  const handleListboxMouseLeave = () => {
+    changeHighlightedIndex('reset', 'next');
+  };
+
   const handleOptionTouchStart = () => {
     isTouch.current = true;
   };
@@ -955,6 +959,7 @@ export default function useAutocomplete(props) {
         // Prevent blur
         event.preventDefault();
       },
+      onMouseLeave: handleListboxMouseLeave,
     }),
     getOptionProps: ({ index, option }) => {
       const selected = (multiple ? value : [value]).some(

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. labels Apr 16, 2020
@marcosvega91
Copy link
Contributor

marcosvega91 commented Apr 17, 2020

Hi, i can make a PR for this 👯

editeted
Hi @oliviertassinari, are you sure that is a bug?
It seams that the issue happen also without freesolo attribute. I think is a normal behaviour of autoselect or not?

If true, the selected option becomes the value of the input
when the Autocomplete loses focus unless the user chooses
a different option or changes the character string in the input.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 17, 2020

@marcosvega91 Thanks for the interest in this issue :). The proposed change impacts all the combination of props. It's meant to be a broader fix. The autoSelect prop makes the problem more noticeable.

You will find a similar behavior on Kendo-UI jQuery, Downshift, a native select, or event Chrome's autocomplete box, Chrome's URL bar, and Google search autocomplete bar.

@oliviertassinari oliviertassinari changed the title Autocomplete unexpectly changes field value on mouse hover [Autocomplete] unexpectly changes field value on mouse hover Apr 17, 2020
@goffioul
Copy link
Contributor Author

The proposed patch seems to fix the problem in my use case. Thanks.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 18, 2020

Giving up on the issue, for now. It doesn't work #20615

@oliviertassinari oliviertassinari added discussion and removed good first issue Great for first contributions. Enable to learn the contribution process. bug 🐛 Something doesn't work labels Apr 18, 2020
@goffioul
Copy link
Contributor Author

@oliviertassinari Not sure I understand. Does that mean it's gonna stay like that?

@oliviertassinari
Copy link
Member

@goffioul If you have a concern with this behavior, favor autoSelect={false}. We will wait to get more feedback in this direction before considering a change.

@goffioul
Copy link
Contributor Author

@oliviertassinari That is not really an option. Try again in my sandbox, I've disabled autoSelect on the first field. Double-click in the first field to select all text, then enter hello, then use Tab to move to the next field (or just click anywhere to blur the field): onChange is not called and the state is not updated. The only work around is to use the combination value/onInputChange.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 18, 2020

Use the two states (value/onChange and inputValue/onInputChange) in this case.

@goffioul
Copy link
Contributor Author

There must be something more subtle than just mirroring the same behavior in the two states. When I do that (check my sandbox), there's an exception generated in useAutocomplete as soon as you clear the field:

image

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 18, 2020

value and inputValue are two different states, you can't control them with the same value.

@goffioul
Copy link
Contributor Author

Does that mean the app needs 2 state value? If yes, how does the application know which one to use? Could you provide an example of how to use the 2 states of Autocomplete?

@oliviertassinari
Copy link
Member

@goffioul
Copy link
Contributor Author

Thanks for the demo. With 2 state values, how does the app know the value to use? E.g. select all text in the field, type foobar and blur the field. The states value and inputValue are different. Obviously, the user's intent is to have foobar as the wanted value. So it seems the state value has no other purpose than to make the field have the right value on mount.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 18, 2020

@goffioul What's your use case? "value" is the selected value by the user, "inputValue" is what is currently rendered in the textbox. Maybe we should better describe this too in the documentation.

Enter

@goffioul
Copy link
Contributor Author

I want a field that can accept free text and also provide a list of predefined value for quick selection. The field also needs to have a non-empty value on mount. And whatever is currently in the field is the wanted value, whether entered with a keyboard or selected from the dropdown list.

This is about editing an object that contains a text field, but the user needs to be able to select a value from a predefined set of entries, instead of having to enter the text value every time. Usually, the text field value is part of the predefined set, but one must retain the ability to enter arbitrary text.

This is illustrated with this video:
https://drive.google.com/open?id=13ynBvSYKxevwFwwFUXrZ4KpDttm3O8VN

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 18, 2020

Thanks for the video, it's perfect, all you need is in the documentation. I'm working on a patch to translate the learnings of this discussion into a better docs.

@goffioul
Copy link
Contributor Author

Please note that in the video, it only works because I'm using a combination value/onInputChange. I can't use inputValue/onInputChange, because the value on mount is lost (see #19423). I can't use value/onChange, because onChange is not called on blur, or value is reset (see #20603, not sure what will be the final behavior, tbh). This blur issue could potentially solved by using autoSelect, but then you run into #20602, which is not gonna solved.

So I'm left with your suggestion to force the app to use 2 state values, one of which is only used to set the value on mount.

@plamenkoyovchev
Copy link

plamenkoyovchev commented Jun 18, 2021

@goffioul This is one of the reasons why this props combination isn't used by default by the Autocomplete. However, I think that there is room for improvement here. What do you think of this patch?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 3cb1ca0c5..4de19514a 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -790,6 +790,10 @@ export default function useAutocomplete(props) {
     setHighlightedIndex(index, 'mouse');
   };

+  const handleListboxMouseLeave = () => {
+    changeHighlightedIndex('reset', 'next');
+  };
+
   const handleOptionTouchStart = () => {
     isTouch.current = true;
   };
@@ -955,6 +959,7 @@ export default function useAutocomplete(props) {
         // Prevent blur
         event.preventDefault();
       },
+      onMouseLeave: handleListboxMouseLeave,
     }),
     getOptionProps: ({ index, option }) => {
       const selected = (multiple ? value : [value]).some(

In which version of material-ui-lab this patch is present?

Currently I'm using "@material-ui/lab": "^4.0.0-alpha.58" and this patch is missing there.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 18, 2021

@plamenkoyovchev #20602 (comment)

@oliviertassinari
Copy link
Member

We likely want to make this happen, with #23916.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed discussion labels Jun 27, 2021
@pfuentes1402
Copy link

I have solved this problem by removing the autoSelect property, or by setting autoSelect={false}.

@Snipx
Copy link

Snipx commented Apr 27, 2022

Totally agree this one would be great to fix, we are struggling with it as well

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! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants