[TextField] Fix typescript error when using inputAdornedStart, inputAdornedEnd classes with default variant and standard variant #36863
Conversation
Netlify deploy previewhttps://deploy-preview-36863--material-ui.netlify.app/ Bundle size report |
|
|
||
| // baseline behavior | ||
| <TextField variant="filled" {...filledProps} />; | ||
| // @ts-expect-error |
There was a problem hiding this comment.
removed this because didn't understand the reasoning behind expecting typescript error when classes are getting applied.
| inputAdornedStart: string; | ||
| /** Styles applied to the input element if `endAdornment` is provided. */ | ||
| inputAdornedEnd: string; |
There was a problem hiding this comment.
If I remove both of these classes, it throws an error in outlined variant textfield, when the variant="outlined" is not explicitly provided to the Text Field component.
Also, these classes are for the standard text field. Not sure why it throws type error for an implicit outlined text field if I remove them. If you can find out the reason it would be great.
There was a problem hiding this comment.
@ZeeshanTamboli when variant="outlined" is not applied, in component implementaion code, default component is set to OutlinedInput here. but types in d.ts file doesn't know that if variant is not provided it should apply OutlinedTextFieldProps to TextFieldProps.
but
typesind.tsfile doesn't know that if variant is not provided it should applyOutlinedTextFieldPropstoTextFieldProps.
code reference: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/TextField/TextField.d.ts#L229
but
typesd.tsfile doesn't know that if variant is not provided it should applyOutlinedTextFieldPropstoTextFieldProps.
Although this will be fixed in this PR #36737
There was a problem hiding this comment.
If the type is going to be fixed in #36737 where TypeScript will be able to pick OutlinedTextFieldProps for implicit outlined variant, then I think you can include these classes in that PR. There isn't a need to create a separate PR because it is related to the bug you are trying to solve. Here in this PR, in my opinion, it is confusing for implicit outlined text field (i.e variant is not applied). The classes being added here is only for the standard variant, not for outlined text fields.
This would help in future if somebody wants to check the related PR. Basically, it should not confuse a future reader. We also don't know when that PR will be merged and released after this.
There was a problem hiding this comment.
@ZeeshanTamboli okay, i've moved changes to #36737 , can you review that PR. I will close this PR, once #36737 is merged
|
Closing this PR as fixes are done in #36737 |
4 participants
closes #36862
before: https://codesandbox.io/s/infallible-haibt-ytenql?file=/styles.css:0-38
after: https://codesandbox.io/s/goofy-leavitt-428poo?file=/demo.tsx