[TextField] Add sizeMedium & variants support #26468
Conversation
siriwatknp
added
the
component: text field
Let's hold off, at least till beta, this one is obvious as somebody experienced some pain for the use-case. |
oliviertassinari
changed the title
| @@ -17,6 +17,8 @@ export interface InputBaseClasses { | |||
| error: string; | |||
| /** Styles applied to the input element if `size="small"`. */ | |||
| sizeSmall: string; | |||
| /** Styles applied to the input element if `size="medium"`. */ | |||
| sizeMedium: string; | |||
There was a problem hiding this comment.
sizeMedium and inputSizeMedium are never applied on the DOM. https://github.com/mui-org/material-ui/blob/8cc7dd0568adde2b8c23804fabdadbd7cfff21d6/packages/material-ui/src/InputBase/InputBase.js#L28
In #25969 I was showcasing the problem with adding too many classes on the DOM nodes. I would personally advocate removing all the default class names / styleOverrides, to match Bootstrap: https://getbootstrap.com/docs/5.0/components/buttons/#sizes, forcing to customize the default class name first as a healthy constraint.
There was a problem hiding this comment.
Good catch, in my opinion, we should add those classes. Again I think we are coming back to this too many times :) On the PR that you linked, margin: none does not provide any styles, and it probably should never provide, but with the sizing props is different, especially if developers change the default props. If the default size for the component is changed to be small, there is no way of how the component can be customized in case size="medium"
There was a problem hiding this comment.
especially if developers change the default props.
I would advocate that they shouldn't, that we shouldn't make it easier for them. I personally fail to see the use cases for size: small as a default. Instead, they could/should customize all the values to shift the size, making size: normal looks smaller.
There was a problem hiding this comment.
Ok, I probably have hard time making a call of when this is ok and when not. Should we make it consistent for all props in that case? What I am trying to understand is, is this one-time thing, or should we follow this thinking everywhere. For example, if in a component, the primary is the default color, should we provide a class for it? I think it would be best if we have a rule that applies everwhere, thank having this discussion on each change we do :) (it's my fault 100%, but I am trying to make us more productive :D)
There was a problem hiding this comment.
For the color, I would come back to what I was saying around following Bootstrap. Being primary by default seems subjective, it makes sense to have a class name/style override.
I think that we can come up with a RFC during the beta phase for this. To resolve this once for all.
There was a problem hiding this comment.
Agree, and following Bootstrap as a base seems like a great start.
There was a problem hiding this comment.
Agree, we should remove all "medium" size if it means "default". will revert the change.
There was a problem hiding this comment.
related issue about default. #13028
closes #26459
variantsto InputBase components.d.tssizeMediumto inputBaseClassesQuestion
I see that there are some components that missed classes like
InputLabelhas onlysizeSmallFormLabelhas onlycolorSecondaryShould we fix these in v5 before or in beta?