Material-ui: [RFC] Replace variant with kind in the components where the variant is used for altering the DOM structure

[mnajdova]

Summary 💡

This RFC is proposing solution to the problem that we faced in https://github.com/mui-org/material-ui/issues/21749 - supporting custom variants for the component. The problem is the following: we want to offer developers the possibility to add their own custom variant values so that they can style their components differently that the supported options we have by default. However in some of the components, the variant property is used not only for styling, but also for changing the internal DOM structure. The problem is that if developers define their own custom variant, like dashed the component will not render anything, or at least will behave unexpectedly because it depends on the variant prop to be some of the defined values. This is the list of the components where we faced this issue:

• CircularProgress
• Drawer
• FormControl
• FormHelperText
• InputAdornment
• InputLabel
• LinearProgress
• Menu
• MenuList
• MobileStepper
• NativeSelect
• Select
• SwipeableDrawer
• TableCell
• Tabs
• TextField

Solution

My proposal for solving this issue is the following. We would replace the variant prop in this component with a different property - kind. This property will behave exactly as the current variant prop. After this change is done, we can safely then use the variant property for purely styling purposes and expose it to the developers for defining their custom variants.

[oliviertassinari]

If I understand correctly, developers will do:

<TextField kind="outlined" />
<Button variant="outlined" />

This feels inconsistent. Have we considered alternative solutions? What if we had these components covered later on, by supporting any prop in the theme.variants matches?

[mnajdova]

The other alternative that I had in mind was, adding different prop for styling purposes or allowing clients to specify their own (but that I believe would be later down the road), I believe that is what you are suggesting?

My only worry with this was, how would clients know that they cannot use the variant prop overrides for these particular components?

[eps1lon]

It feels like we're leaking implementation details. Developers shouldn't need to know about the internal DOM structure or what is applied by CSS, JS etc to decide whether something is a kind or a variant.

This will lock the current implementation to the technology used at the time. If we later discover that we don't need to change the internal structure or we do want to change the internal structure then we can't do this because of the variable naming.

I understand that we do already leak some of these details but we do it in a controlled fashion in a per-component basis. Not as a general concept that we need to teach users of this library.

This RFC is proposing solution to the problem that we faced in #21749

Could you include a brief summary of the problem in the opening description of this issue? It's not clear to me whether this is an implementation problem or interface problem or teaching problem.

[oliviertassinari]

The other alternative that I had in mind was, adding different prop for styling purposes or allowing clients to specify their own (but that I believe would be later down the road), I believe that is what you are suggesting?

@mnajdova Yes, this is what I was suggesting, we could hold on these components (CircularProgress, Drawer, etc.) until we resolve this other issue.

My only worry with this was, how would clients know that they cannot use the variant prop overrides for these particular components?

The first layer of an answer will be the prop-type and types that will complain. The second layer of an answer would be the generated documentation that doesn't include "string" as part of the available options.

[eps1lon]

The first layer of an answer will be the prop-type and types that will complain.

Types can only tell you what does or does not work. They can't tell you what you should use which is the problem here.

[mnajdova]

The problem is the following: we want to offer developers the possibility to add their own custom variant values so that they can style their components differently that the supported options we have by default. However in some of the components, the variant property is used not only for styling, but also for changing the internal DOM structure.

@eps1lon I am missing here the description of why this is a problem, thanks of bringing it up. The problem is that if developers define their own custom variant, like dashed the component will not render anything, or at least will behave unexpectedly because it depends on the variant prop to be some of the defined values. Is it more clear now? Will update the PR description.

[mbrookes]

@eps1lon As a specific example, the TextField component is a wrapper around the Input, FilledInput and OutlinedInput components. The variant prop determines the component to be used, as well as some other logic.

[eps1lon]

@mbrookes I understand the technical problem for the implementation.

However, none of this should affect the API because then you lock the implementation in place. What if we want to split up the Button into each variant? We couldn't do this without a breaking change where we switch from variant to kind.

Edit: Which wouldn't be a problem with wrapper components because the user would be in charge of handling variants.

[mbrookes]

What if we want to split up the Button into each variant?

Do you have any idea how long it took me to merge them? 😅

We couldn't do this without a breaking change where we switch from variant to kind.

Understood, and I'm not arguing in favour of "kind", i jut misinterpreted the last paragraph of https://github.com/mui-org/material-ui/issues/22259#issuecomment-675459374 to mean that clarification of the nature of issue was needed, so I was simply adding a specific example.

@mnajdova Thinking out loud, how about:

1. Rendering the default variant when the variant prop value isn't recognised?
2. Allowing the underlying variant to be specified as a property of the custom variant in the theme?
   This would allow developers to apply, for example, a customised variant of the outlined text field.

[mnajdova]

1. Rendering the default variant when the variant prop value isn't recognised?

We can definitely do this.

1. Allowing the underlying variant to be specified as a property of the custom variant in the theme?
   This would allow developers to apply, for example, a customised variant of the outlined text field.

This sounds interesting, let me see how the structure would look like. Another similar idea I had was prefixing the variants with the original variant, something like 'filled-dashed', so we can test for the DOM structure if the variant starts with something rather than is equal to. Anyway, this is a good idea, let me experiment with it a bit to see how it may look like.

[eps1lon]

Do you have any idea how long it took me to merge them? sweat_smile

Ideally we'd be able to extract specific variants with a compiler for bundle size reduction. So that people who only use variant X only get the code for variant X. That's a topic for https://github.com/mui-org/material-ui/issues/16280 though.