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

[InputBase] Provide input adornments with FormControlContext #14364

Merged
merged 3 commits into from
Feb 5, 2019

Conversation

mtidei
Copy link
Contributor

@mtidei mtidei commented Jan 31, 2019

Fixes #14360

@joshwooding
Copy link
Member

Yeah... I was fairly certain this would break the variant inheriting 😅

@joshwooding
Copy link
Member

@oliviertassinari Do we still need to set the value to null in InputBase if we do it in the InputAdornment?
https://github.com/mui-org/material-ui/blob/d939f2beadb05d33b8edc6d5cb4fa1b10c907ef8/packages/material-ui/src/InputBase/InputBase.js#L407

@eps1lon
Copy link
Member

eps1lon commented Feb 1, 2019

@oliviertassinari Do we still need to set the value to null in InputBase if we do it in the InputAdornment?

Comment got lost during #13590:
https://github.com/mui-org/material-ui/pull/13590/files#diff-1f94c95b4e8094138b96211b29be700eL180

Code got added in #10229:
https://github.com/mui-org/material-ui/pull/10229/files#diff-deaeb5c7e253bfa4f497fa8ace0b3607R237

Original change didn't include a test. To be safe we should add a test that would fail prior to #10229 and then see if this test would pass now without setting the value to null.

@joshwooding
Copy link
Member

Original change didn't include a test. To be safe we should add a test that would fail prior to #10229 and then see if this test would pass now without setting the value to null.

I agree, we’ve asked @mtidei for help reproducing the error so we can add a test
#14360 (comment)

@oliviertassinari oliviertassinari merged commit 1781aa7 into mui:next Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants