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] freeSolo (tags) does not not allow enter the same value more then once #18976

Closed
adica opened this issue Dec 25, 2019 · 13 comments · Fixed by #19215
Closed

[Autocomplete] freeSolo (tags) does not not allow enter the same value more then once #18976

adica opened this issue Dec 25, 2019 · 13 comments · Fixed by #19215
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@adica
Copy link

adica commented Dec 25, 2019

In autocomplete with multiple values (tags), on the feeSolo version, if you try to insert the same value twice, the first value is deleted.

Current Behavior 😯

If I add tag and try to add it again it deletes the first one.

Expected Behavior 🤔

It should allow insert the same value many times.

Steps to Reproduce 🕹

https://material-ui.com/components/autocomplete/#multiple-values
in the freeSolo version:

  1. add the tag aa
  2. try adding the tag aa again
  3. instead of adding it, it will delete the first aa tag

tags

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Dec 25, 2019
@oliviertassinari
Copy link
Member

@adica What's your use case?

@adica
Copy link
Author

adica commented Dec 25, 2019

not sure if I need the double tag, but definitely it should not delete the previous one..

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. and removed waiting for user information labels Dec 25, 2019
@oliviertassinari
Copy link
Member

Ok, it should probably clear the input and keep the existing values.

@m4theushw
Copy link
Member

@oliviertassinari I'm in doubt if this should happen only with custom values or also when the user clicks in an option which matches its search but already added.

@oliviertassinari oliviertassinari removed bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. labels Dec 27, 2019
@gmltA
Copy link
Contributor

gmltA commented Jan 9, 2020

@oliviertassinari yep, this behaviour would be nice to have, I guess. Any updates regarding this issue?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 11, 2020

not sure if I need the double tag, but definitely it should not delete the previous one.

@adica What do you think of this patch? Do you want to submit a pull request :)?

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index de0bfd292..7eabcb117 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -430,7 +430,7 @@ export default function useAutocomplete(props) {
     setValue(newValue);
   };

-  const selectNewValue = (event, newValue) => {
+  const selectNewValue = (event, newValue, origin = 'option') => {
     if (multiple) {
       const item = newValue;
       newValue = Array.isArray(value) ? [...value] : [];
@@ -439,7 +439,7 @@ export default function useAutocomplete(props) {

       if (itemIndex === -1) {
         newValue.push(item);
-      } else {
+      } else if (origin !== 'freeSolo') {
         newValue.splice(itemIndex, 1);
       }
     }
@@ -597,7 +597,7 @@ export default function useAutocomplete(props) {
             // Allow people to add new values before they submit the form.
             event.preventDefault();
           }
-          selectNewValue(event, inputValue);
+          selectNewValue(event, inputValue, 'freeSolo');
         }
         break;
       case 'Escape':

I'm in doubt if this should happen only with custom values or also when the user clicks in an option which matches its search but already added.

@m4theushw I would rather encourage the filterSelectedOptions prop. Alternatively, we could push #19064 further and allow developers to ignore the delete events. What do you think?

this behaviour would be nice to have, I guess. Any updates regarding this issue?

@gmltA could you be more specific?

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

gmltA commented Jan 12, 2020

@oliviertassinari I meant this one for freeSolo case

Ok, it should probably clear the input and keep the existing values.

@oliviertassinari
Copy link
Member

@gmltA Cool, so we have found a consensus.

@m4theushw
Copy link
Member

@oliviertassinari pushing #19064 will give more flexibility.

@oliviertassinari
Copy link
Member

@m4theushw I agree. Still, shouldn't we implement both?

@m4theushw
Copy link
Member

@oliviertassinari Yes, I'll give some time and see if any author wants to contribute. If not I can help.

@oliviertassinari
Copy link
Member

@m4theushw Awesome. Thank you for the help on this component :).

@adica
Copy link
Author

adica commented Jan 13, 2020

#19215

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! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants