-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
Proposal (breaking change): remove inputProps from Input, TextField, etc. #9326
Comments
From #9313 (comment) For custom inputs, it would turn: Case 1 - open ended (non-type checked)
|
As an aside, with this change, |
I'm going to move forward with this because the typescript for |
So here is what I've found: a) Why does
|
So I think this is cleaner/clearer, but it does come at the expense of this usage: Old <TextField type="number" inputProps={{ min: "0", max: "10", step: "1" }} /> New <TextField type="number" InputProps={ {inputProps: { min: "0", max: "10", step: "1" }} } /> Still, the pattern seems much more obvious here, and we are less likely to wonder why we have both |
@pelotom It's only here for ease of use. I do think we have this "hack" at many other locations. I'm fine removing it. It's reducing the API surface and making the library more TypeScript compatible 👍 .
@rosskevin Does this make the API more consistent? I mean, the |
For those that may find this in the future, the import { InputProps as MuiInputProps } from 'material-ui/Input'
<TextField
InputProps={{
component: (inputProps: MuiInputProps) => (
<MaskedInput {...inputProps} guide={false} mask={mask} placeholderChar="\u2000" />
),
}}
type="tel"
value={value}
/> |
@oliviertassinari I've submitted the PR, I think it's a good change. With regards to |
Declaration of Input-dependence? 😄 |
This is a really bizarre change! In version 22 and before, to apply a I think that was already confusing because it was not clear why some of native properties were directly available on TextField (like However, in the new release beta.23, the props should be set like this: Based on what I read the reasoning behind this change is to eliminate confusion. IMHO this is several times more confusing, and in reality users should keep digging documentation and copy/paste this every time they need it. Of course I'm a single user and have one vote! Others may think differently. I appreciate your great work on this amazing library regardless. |
@semiadam You make a good point. The first use case for providing properties on the TextField element is to alter the native input. The |
Reasoning: TextField is a convenience wrapper for the most common cases (80%). It cannot be all things to all people, otherwise the API would grow out of control. Given it had some advanced props propagating, but not all, it was confusing and limiting. Restricting this convenience wrapper to just 80% and allowing advanced use only through the uppercase component props reduces confusion and reduces the surface area. I agree that you would think inputProps belongs on TextField, but consistency-wise, it does not, as evidenced by the pattern of use of every other component contained and controlled by TextField. I hope that explanation helps. I spent a good bit of time untangling the naming and patterns here to arrive at the current state. |
@rosskevin Any lead for improving the documentation? Maybe we can add a not about it in the header of https://material-ui.com/api/text-field/#textfield
// ... example An example: https://material-ui.com/api/form-control/ |
Not a bad idea. We should probably highlight the structure of it and point to the underlying components. I think a literal display of source structure might be useful, all under a header like "Advanced Configuration" |
I'm on the documentation part. |
The
Input
component has bothand
The first thing to note is that the type for
inputComponent
is wrong, it should bewhere
...
is whatever properties could actually be passed down by the parent component to the child component. That brings us toinputProps
. What are they here for? If you look at most other components, for exampleButton
, there is no prop like this, onlyand the way you would inject whatever custom props your overriding component needs would be
So why isn't
Input
the same way, and do we really need theinputProps
prop? Besides consistency with the rest of the code base, a good reason not to haveinputProps
is that it's impossible to make type safe. We basically have to saySo I'd like to advocate removing
inputProps
and instead having a closed set of specified props thatInput
can pass toinputComponent
.See also #9313.
The text was updated successfully, but these errors were encountered: