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] Missing support for limitTags={0} #20815

Closed
tykdn opened this issue Apr 28, 2020 · 4 comments · Fixed by #20850
Closed

[Autocomplete] Missing support for limitTags={0} #20815

tykdn opened this issue Apr 28, 2020 · 4 comments · Fixed by #20850
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

@tykdn
Copy link
Contributor

tykdn commented Apr 28, 2020

In the api document,'the maximum number of tags that will be visible when not focused. Set -1 to disable the limit'. But I set 0 also disable the limit.Dose the limitTags api support value of 0.

Thanks!

@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! support: question Community support but can be turned into an improvement labels Apr 28, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 28, 2020

I don't think that it matters, I'm closing, prefer -1.

@tykdn
Copy link
Contributor Author

tykdn commented Apr 29, 2020

@oliviertassinari Sorry,I did not express clearly.I means, does Autocomplete support maximum number of tag of 0.
In other word, when I set 0, it should display none tag.
Thanks!

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. and removed support: question Community support but can be turned into an improvement labels Apr 29, 2020
@oliviertassinari
Copy link
Member

@tykdn Thanks for the clarification. What do you think of this fix? Do you want to submit a pull request? :)

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index f6c58d99a..cd9a300cb 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -350,7 +350,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {

   if (limitTags > -1 && Array.isArray(startAdornment)) {
     const more = startAdornment.length - limitTags;
-    if (limitTags && !focused && more > 0) {
+    if (!focused && more > 0) {
       startAdornment = startAdornment.splice(0, limitTags);
       startAdornment.push(
         <span className={classes.tag} key={startAdornment.length}>

@oliviertassinari oliviertassinari changed the title zero limitTags performance is inconsistent with documentation of API. [Autocomplete] zero limitTags performance is inconsistent with documentation of API. Apr 29, 2020
@oliviertassinari oliviertassinari changed the title [Autocomplete] zero limitTags performance is inconsistent with documentation of API. [Autocomplete] Missing support for limitTags={0} Apr 29, 2020
@tykdn
Copy link
Contributor Author

tykdn commented Apr 29, 2020

@oliviertassinari Yes, the fix looks good,I am glad to submit a pull request:).

tykdn added a commit to tykdn/material-ui that referenced this issue Apr 29, 2020
tykdn added a commit to tykdn/material-ui that referenced this issue Apr 29, 2020
tykdn added a commit to tykdn/material-ui that referenced this issue May 2, 2020
tykdn added a commit to tykdn/material-ui that referenced this issue May 2, 2020
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.

2 participants