-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[RFC] Support custom variants in the theme #21749
Comments
@mnajdova Which started at #21648 (comment). I would be leaning toward waiting for the request of this. As we are still early in the v5 alpha phase. I think that if it's important, it will come up. I also doubt that developers will need this layer of complexity. When is this useful? It seems that when the component has a complex structure, it will allow to save 1 level of specificity for the developers that want to customize the theme built on top of Material-UI. Here is one case: Now, considering that in the codebase we aren't consistent on this point. Sometimes we increase specificity to avoid having to create too many class names. I would vote for waiting. The alternative will be to target, with a CSS global selector, the class name. |
At the moment How/Would sharing such a themed component work with this new api? I think this would be a quite nice approach to ship a e.g. bootstrap themed material-ui (which is a usecase i'm not personally interested in, but read about). |
👍 We are currently creating a company MUI theme and I think that approach would be very useful to us. It would avoid to have to wrap components and expose them in separate package. Styling an app would then just be matter of using the theme on MUI lib and would come with all specific variations of components. |
@sakulstra this API is not preventing you to still utilize the wrapper components if that's your preference. On the other hand, if you want to share you variants across different project, you can just share you custom theme and everything would work out of the box by using the core MUI components. |
Months ago I would say "YES I WANT IT" but now thinking about it... Usually I when I create custom components I tend to add more than just styles. Thus I think I will not stop using that approach even with this new API. I believe we can do a good job with the current API and the core team should focus its efforts on things that will add more value instead of another way to do something that we can do today already. |
@taschetto Thanks for the feedback. Do you have specific items in mind? |
Promoting lab components to core would be great. |
I think that this feature is fantastic, I just suggest something like a matcher: { variant: 'dashed' },
base: {variant: 'primary'},
styles: {
root: {
padding: '5px 15px',
border: `5px dashed ${ ${outerTheme.palette.primary.main}}`,
},
label: {
color: outerTheme.palette.primary.main;
}
} The above example is a button with everything from the |
I'm really surprised to see how far this has come, cheers to all the devs who've contributed to make this happen 🤘🔥 I have a few questions 🤔
Apologies if any of this has already been covered. I tried parsing through this thread and the PR and didn't immediately see any of this covered. And again, props to everyone who's knocked this out (s/o to @mnajdova). This might not seem like the most "important" feature, but this is some pretty cutting edge stuff in the design system space, so it's nice to see such a large UI library looking to adapt to these concepts. Very forward thinking! I wish everyone all the luck! |
@whoisryosuke these are really good questions, thanks for looking into this :) Here are some thoughts/answers.
The matcher is simply a subset of the components props, which is based on the props specified generating the classkey selector. So if you provide in your matcher: Is the name
We were looking into this API, the main reason of why I decided to go with simple java object for this, is easier support for combination of props. If you want to define the styles for example for
Not sure I understand this correctly. Are you suggesting that multiple
I am reluctant of adding this new API in the first iteration, mainly because it may create run-time problems (there may be cyclic definition of the styles dependencies, and the processing would be much more complicated, which may in the end affect the perf). It is perfectly normal to define a function for common styles that can be parameterized, or clients can just spread common styles, I really don't see big benefit of adding it as part of the API of the variants. However this is not final, I will experiment with this, and may create a follow up PR of this together with the other feedback we will receive. Does this makes sense? Update:
I agree, generally the variants can be defined as functions or common styles object that can be re-used on more components. This is a good point, I will add a more advanced example in the customization docs to maybe illustrate something like this 👍
Currently this API is following the |
@mnajdova Thank you for taking the time to respond and answer all my questions 👍 I can see why stuff like extending variants would be left for maybe another version, definitely can open a can of worms in terms of poor performance. But I'd definitely keep it in mind, as ultimately this would eliminate a lot of code duplication down the line (which is what I meant by variants "sharing" styles). I have some clarity for the unanswered questions:
No, more that it'd help to organize variants if they were grouped together by their variant "key". Like the example you have above, you're listing two In design systems, I like to group my variants, so I can quickly go through and see "size" variants or "color" variants. With this way, I'd have to CTRL/CMD+F and search for the variant key, and collapse each variant object individually -- and also hope they're next to each other (and not mixed around). Current API: const theme = outerTheme => createMuiTheme({
variants: {
MuiTypography: [
{
matcher: { variant: 'headline1' }, // combination of props for which the styles will be applied
styles: {
padding: '5px 15px',
border: `5px dashed ${ ${outerTheme.palette.primary.main}}`,
},
},
{
matcher: { variant: 'headline1', color: 'secondary' },
styles: {
padding: '5px 15px',
border: `5px dashed ${outerTheme.palette.secondary.main}`,
},
},
],
},
}); Suggested structure: const theme = outerTheme => createMuiTheme({
variants: {
MuiTypography: [
{
variant: 'headline1', // combination of props for which the styles will be applied
options: [
{
// default option?
styles: {
padding: '5px 15px',
border: `5px dashed ${ ${outerTheme.palette.primary.main}}`,
},
}
{
color: 'secondary',
styles: {
padding: '5px 15px',
border: `5px dashed ${ ${outerTheme.palette.secondary.main}}`,
},
}
]
},
],
},
}); This way variants are organized better, and grouped by their key. Seems odd that I can create variant options "out of order", it would make a theme modular -- but more confusing to parse. The reason I suggest using I'm not a fan of variant API that are limited to a single prop, it makes the design system too simplistic and causes creating extra components for "separating responsibility" (like |
I have renamed the const theme = createMuiTheme({
variants: {
MuiTypography: [
{
props: { size: 'large', color: 'green' },
styles: {
fontSize: 40,
backgroundColor: 'green'
},
},
],
},
}); We plan to possibly extend this in the future, so people can add new properties for their design systems. The first key in the theme I had in mind for this was Having this said, I am not sure if adding additional grouping by variant would be the best choice, but let me spend some more time on that before making a final decision. |
I think that we should aim for a name the communicate the intent to adds extra possible styles for states on the components. It seems that
I would be cautious with the complexity that comes with it from a user of the library perspective. In order to know which style applies when (for ease of maintainability), you have to traverse a tree. Is it simpler than a list? |
Moreover if there is no variant in the props matcher we will need to have something like null root which would even more complicate things... |
Thanks everyone who participated in this RFC and helped with polishing the API. We started adding this across the components that supports the variants props as a start (you can see the list of the components in the RFC description). For everyone who would want to contribute, please follow the #22006 as a template. |
Hi. Is it possible to get support for 'Card' component too ? At the moment I don't see this component in the list of WIP |
Hi, thanks for the cool work! I also need to add more variants to MUI because our designers want to have more variants. How is it working now? We're currently on |
@Jack-Works Do you have a specific example of a variant your designers want to support? |
For example, 4 sizes than 3 sizes (this library) and more color in the theme than primary or secondary |
Hi! I have tried about this in alpha 17. // App.tsx
/// <reference path="./declaration.d.ts" />
import { Typography } from '@material-ui/core';
export default <Typography variant="cool" />;
// declaration.d.ts
declare module '@material-ui/core/Typography/Typography' {
interface TypographyPropsVariantOverrides {
cool: true;
h1: false; // variant="h1" is no longer available
}
} I found it didn't merged the declaration. And I found only variant are overwritable, it would be nice if size and color also does. |
@Jack-Works Does it mean that |
I think the idea is great but it doesn't work unfortunately. My version of |
It works great here: https://codesandbox.io/s/globalthemevariants-material-demo-forked-kqqly?file=/demo.tsx. |
Does it work if you move the |
I would expect it to work from a different file, we use this approach with the lab to allow customizations with the theme. |
Strange, it doesn't work on my machine. I will try again later |
I have a minimal reprod to show it didn't work.
{
"dependencies": {
"@material-ui/core": "^5.0.0-alpha.17",
"@types/react": "^17.0.0",
"@types/react-dom": "^17.0.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
"typescript": "^4.1.2"
}
} # I'm not sure if pnpm is core of this problem
pnpm install
tsc --init Then add
declare module "@material-ui/core/Typography/Typography" {
interface TypographyPropsVariantOverrides {
cool: true;
h1: false; // variant="h1" is no longer available
}
}
/// <reference path="./decl.d.ts" />
import { Typography } from "@material-ui/core";
import React from "react";
export default <Typography variant="cool" />; Type of And |
@Jack-Works here is a working example - https://codesandbox.io/s/autumn-glade-zvl5n?file=/src/declarations.d.ts
declare module "@material-ui/core/Typography/Typography" {
interface TypographyPropsVariantOverrides {
cool: true;
h1: false; // variant="h1" is no longer available
}
}
+ // disable automatic export
+ export {}; Let me know if this will resolve your issue. |
Yes, that works thanks! But I'm curious why, based on what I learn writing |
@mnajdova Should we document it in https://next.material-ui.com/guides/typescript/#customization-of-theme? |
Yes, will do it tomorrow |
hi! I'd like to add my custom colors to the button component (e.g. "error" and "warning") but it seems not possible with the current typing on "color". How can I do that? |
@Jack-Works you might want to follow this issue #13875 |
Hey @oliviertassinari @mnajdova, is there any plan for the components where their support for custom variants is marked as postponed? For instance, i currently have a custom "borderless" TableCell variant where MUI's PropTypes are yelling at me, as its PropTypes do not allow variants which are not included by default. 😄 |
@EliasJorgensen we are discussing this currently with the development of the new design system cc @siriwatknp In some components the |
would be nice to be able to create variants for specific slots. now the variants are always applied to the root slot. |
Summary
This RFC is proposing a solution for adding custom variants for the core components inside the theme. We already have an option for adding custom overrides inside the theme, with this RFC we want to extend it to support custom variants as well.
Basic example
The API could look something like this:
Motivation
From the developer's survey, the 3rd most popular use case for Material-UI is to build a custom design system on top of it. This proposal is meant to make it easier. Currently developers can add new props combination by creating wrapper components:
Adding and removing variants from Material-UI components creates a challenge. You have to document these variants as well as making sure they will be used correctly. Solving the issue at the documentation level will likely require making progress on #21111.
While this option is already available, we have heard pushbacks from the community around it.
The issues with the wrapper path are:
In the long run, it could be ideal if we can implement the Material Design light and dark themes with this approach alone.
Detailed design
PR #21648 is implementing this feature for the
Button
component. This is how it can be used:The typescript users, can use module augmentation for defining their new variants types:
Drawbacks
Always with new API we have to consider also the drawbacks of adding it. Here are some points:
classKey
definitionAlternatives
This can be implemented with wrapper components. Another idea that we tried was, relaxing the typings of the
overrides
key, and allowing users to specify the newclassKeys
directly there - this will mean that clients need to know how the props are converted to classes keys inside each component.Adoption strategy
As this is a new API, the adoption can be straight forward for the users.
Unresolved questions
One thing that we need to decide on whether to support slots styles inside the defining, for example, defining the styles of the
root
andlabel
slots in theButton
.This may require changes in the components implementation and adding some new
classKeys
that will support this API.Progress
Here is a list of all components that would benefit from this API. This list will help us track the progress of where the API is implemented.
Avatar [Avatar] Custom variant #22139
Badge [Badge] Custom variant #22140
Button [Button] Custom variant #21648
ButtonGroup [ButtonGroup] Custom variant #22160
Chip [Chip] Custom variant #22161
CircularProgress [Postponed]
Divider [Divider] Custom variant #22182
Drawer [Postponed]
Fab [Fab] Custom variant #22189
FormControl [Postponed]
FormHelperText [Postponed]
InputAdornment [Postponed]
InputLabel [Postponed]
LinearProgress [Postponed]
Link [Link] Custom variant #22204
Menu [Postponed]
MenuList [Postponed]
MobileStepper [Postponed]
NativeSelect [Postponed]
Paper [Paper] Custom variant #22216
Select [Postponed]
SwipeableDrawer [Postponed]
TableCell [Postponed]
Tabs [Postponed]
TextField [Postponed]
Toolbar [Toolbar] Custom varaint #22217
Typography [Typography] Add custom variants support #22006
Alert [Alert] Custom varaint #22218
Pagination [Pagination] Custom variant #22219
PaginationItem [PaginationItem] Custom variant #22220
Skeleton [Skeleton] Custom variant #22243
TimelineDot [TimelineDot] Custom variant #22244
The text was updated successfully, but these errors were encountered: