Skip to content

[TextField] Fix typescript error when using inputAdornedStart, inputAdornedEnd classes with default variant and standard variant #36863

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.

Sign up for GitHub

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

Closed
sai6855 wants to merge 4 commits into from
Closed

[TextField] Fix typescript error when using inputAdornedStart, inputAdornedEnd classes with default variant and standard variant #36863

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
49049bf
fix
sai6855 Apr 12, 2023
495b22a
doccs:api
sai6855 Apr 12, 2023
7a99bc2
trigger
sai6855 Apr 12, 2023
ad107fa
tests
sai6855 Apr 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/pages/material-ui/api/input.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
"input",
"inputSizeSmall",
"inputMultiline",
"inputAdornedStart",
"inputAdornedEnd",
"inputTypeSearch"
],
"globalClasses": { "focused": "Mui-focused", "disabled": "Mui-disabled", "error": "Mui-error" },
Expand Down
10 changes: 10 additions & 0 deletions docs/translations/api-docs/input/input.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@
"nodeName": "the input element",
"conditions": "<code>multiline={true}</code>"
},
"inputAdornedStart": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the input element",
"conditions": "<code>startAdornment</code> is provided"
},
"inputAdornedEnd": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the input element",
"conditions": "<code>endAdornment</code> is provided"
},
"inputTypeSearch": {
"description": "Styles applied to {{nodeName}} if {{conditions}}.",
"nodeName": "the input element",
Expand Down
4 changes: 4 additions & 0 deletions packages/mui-material/src/Input/inputClasses.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export interface InputClasses {
inputSizeSmall: string;
/** Styles applied to the input element if `multiline={true}`. */
inputMultiline: string;
/** Styles applied to the input element if `startAdornment` is provided. */
inputAdornedStart: string;
/** Styles applied to the input element if `endAdornment` is provided. */
inputAdornedEnd: string;
Comment on lines +33 to +35
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli Apr 17, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sai6855 sai6855 Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@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 types in d.ts file doesn't know that if variant is not provided it should apply OutlinedTextFieldProps to TextFieldProps.

code reference: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/TextField/TextField.d.ts#L229

but types d.ts file doesn't know that if variant is not provided it should apply OutlinedTextFieldProps to TextFieldProps.

Although this will be fixed in this PR #36737

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli Apr 17, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sai6855 sai6855 Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@ZeeshanTamboli okay, i've moved changes to #36737 , can you review that PR. I will close this PR, once #36737 is merged

/** Styles applied to the input element if `type="search"`. */
inputTypeSearch: string;
}
Expand Down
6 changes: 6 additions & 0 deletions packages/mui-material/src/TextField/TextField.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import { expectType } from '@mui/types';
const filledInputTypeSearch = (
<TextField variant="filled" InputProps={{ classes: { inputTypeSearch: 'search-input' } }} />
);
const StandardInputAdornedStart = (
<TextField variant="standard" InputProps={{ classes: { inputAdornedStart: 'search-input' } }} />
);
const DefaultInputAdornedStart = (
<TextField InputProps={{ classes: { inputAdornedStart: 'search-input' } }} />
);
const standardOutlinedClassname = (
<TextField
variant="standard"
Expand Down
6 changes: 0 additions & 6 deletions packages/mui-material/test/typescript/hoc-interop.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,17 @@ const filledProps = {

// baseline behavior
<TextField variant="filled" {...filledProps} />;
// @ts-expect-error
Copy link
Contributor Author

@sai6855 sai6855 Apr 12, 2023

Choose a reason for hiding this comment

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

removed this because didn't understand the reasoning behind expecting typescript error when classes are getting applied.

<TextField {...filledProps} />; // desired to throw

// styled
{
const StyledTextField = styled(TextField)``;
<StyledTextField variant="filled" {...filledProps} />; // desired to pass
// @ts-expect-error
<StyledTextField {...filledProps} />; // undesired, should throw
}

// @emotion/styled
{
const StyledTextField = emotionStyled(TextField)``;
<StyledTextField variant="filled" {...filledProps} />;
// @ts-expect-error
<StyledTextField {...filledProps} />; // desired to throw
}

// https://github.com/mui/material-ui/issues/14586
Expand Down