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] closeIcon prop controls the clearIcon #23591

Closed
2 tasks done
techdev5521 opened this issue Nov 18, 2020 · 10 comments · Fixed by #23617
Closed
2 tasks done

[Autocomplete] closeIcon prop controls the clearIcon #23591

techdev5521 opened this issue Nov 18, 2020 · 10 comments · Fixed by #23617
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

@techdev5521
Copy link

  • 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 closeIcon prop is misnamed and corresponds to the icon for clearing the input.

Expected Behavior 🤔

The closeIcon prop to corresponds to the icon for closing the input.

Steps to Reproduce 🕹

https://codesandbox.io/s/mui-autocomplete-mislabeled-closeicon-46kt3?file=/src/App.js:693-5392. (This isn't the official template because it was complaining about an unrelated dependency not being met.

Steps:

  1. Import Autocomplete from @material-ui/lab
  2. Use the closeIcon prop

Context 🔦

As it relates to MUI I just want the closeIcon prop to correspond to the appropriate end adornment. In practice, I am feeding the Autocomplete with an asynchronous fetch and want to fire the call only when the menu is closed.

Screen Shot 2020-11-18 at 12 05 30 AM

Your Environment 🌎

Tech Version
Material-UI v4.11.0
React v16.3.1
Browser Google Chrome 86
TypeScript
etc.
@techdev5521 techdev5521 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 18, 2020
@eps1lon eps1lon added bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 18, 2020
@eps1lon
Copy link
Member

eps1lon commented Nov 18, 2020

Thanks for the report.

Confused me as well. I guess clearIcon would be more appropriate.

@eps1lon eps1lon added the good first issue Great for first contributions. Enable to learn the contribution process. label Nov 18, 2020
@oliviertassinari oliviertassinari changed the title Autocompelte's closeIcon prop actually controls the clearIcon. [Autocompelte] closeIcon prop controls the clearIcon Nov 18, 2020
@oliviertassinari oliviertassinari changed the title [Autocompelte] closeIcon prop controls the clearIcon [Autocomplete] closeIcon prop controls the clearIcon Nov 18, 2020
@akhilmhdh
Copy link
Contributor

@eps1lon Hey there, can I fix this

@techdev5521
Copy link
Author

Tangentially related, would it be possible to also expose the close icon via props?

@akhilmhdh
Copy link
Contributor

@techdev5521 Do you mean close in sense closing the list. If that's the case I think its fired on

Can be: `"toggleInput"`, `"escape"`, `"select-option"`, `"remove-option"`, `"blur"
and exposed as onClose and I think there is no icon for that.

@techdev5521
Copy link
Author

@akhilmhdh I mean exposing the close icon for being overridden in the same way the clear icon is exposed now.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 19, 2020

I mean exposing the close icon for being overridden in the same way the clear icon is exposed now.

@techdev5521 What do you mean by "close icon"? Are you referring to the drop-down open/close icon?
Capture d’écran 2020-11-19 à 16 37 58

@techdev5521
Copy link
Author

@oliviertassinari Sorry for being vague, yes that is exactly what I mean :)

(Also, thanks @akhilmhdh for opening that PR! Looking at what you did makes it easy for me to address similar issues in the future!)

@akhilmhdh
Copy link
Contributor

@techdev5521 hey there sorry 😅on my side. Didn't saw the message.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 19, 2020

@techdev5521 Thanks for the clarification. Maybe, as a follow-up, we should replace these two props with a components={{ ClearIcon, DropdownIcon }} one, allowing to inject components deep in the rendering tree: #21453.

@techdev5521
Copy link
Author

@oliviertassinari I've given that a skim and the components prop would be a very flexible way to achieve the desired outcome. When I've more time I'll give that thread a deeper read.

In the meantime, I just had the idea that I could manipulate the params passed in the renderInput prop to achieve the desired outcome for the time being. Before I do that, do you see a better short term fix?

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