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] Fix overriding TextField value and onChange prop #6642

Merged
merged 4 commits into from
Apr 18, 2017
Merged

[AutoComplete] Fix overriding TextField value and onChange prop #6642

merged 4 commits into from
Apr 18, 2017

Conversation

avocadowastaken
Copy link
Contributor

Fix for another regression caused by #6497.

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

@oliviertassinari
Copy link
Member

I'm not sure to understand the use case for that change. Don't we want user to override those values?

@avocadowastaken avocadowastaken changed the title [AutoComplete] Fix overriding TextField value prop. [AutoComplete] Fix overriding TextField value and onChange prop. Apr 18, 2017
@avocadowastaken
Copy link
Contributor Author

avocadowastaken commented Apr 18, 2017

I'm not sure to understand the use case for that change.

E.g - Spreading props from redux-form fields.

Don't we want user to override those values?

If so - its a breaking change.
I already fixed it on my side but still it looks like a footgun cause devs can accidentally break internal AutoComplete functionality.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 18, 2017

Thanks for providing more detail. Alright, I guess we could make an exception for those two values as they are quite idiomatic in the React world and reduce the breaking change potential.

{...other}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment for the why? Thanks

Copy link
Contributor Author

@avocadowastaken avocadowastaken Apr 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

Is there anything else?

@oliviertassinari oliviertassinari changed the title [AutoComplete] Fix overriding TextField value and onChange prop. [AutoComplete] Fix overriding TextField value and onChange prop Apr 18, 2017
@oliviertassinari
Copy link
Member

@umidbekkarimov Thanks.

@antoinerousseau
Copy link
Contributor

antoinerousseau commented Apr 25, 2017

handleChange could call props.onChange if set

edit: sorry no, that would be a breaking change too

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants