search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.92k stars 32.27k forks source link

[RFC] Support custom variants in the theme #21749

Closed mnajdova closed 3 years ago

mnajdova commented 4 years ago

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:

const theme = outerTheme => createMuiTheme({
  variants: {
    MuiTypography: [
      {
        props: { variant: 'headline1' }, // combination of props for which the styles will be applied
        styles: {
          padding: '5px 15px',
          border: `5px dashed ${ ${outerTheme.palette.primary.main}}`,
        },
      },
      {
        props: { variant: 'headline1', color: 'secondary' },
        styles: {
          padding: '5px 15px',
          border: `5px dashed ${outerTheme.palette.secondary.main}`,
        },
      },
    ],
  },
});

declare module '@material-ui/core/Typography/Typography' {
  interface TypographyPropsVariantOverrides {
    headline1: true;
    h1: false; // variant="h1" is no longer available
  }
}

<Typography variant="headline1" color="secondary" />

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:

import React from "react";
import { makeStyles } from "@material-ui/core/styles";
import { deepmerge } from "@material-ui/utils";
import MuiButton, {
  ButtonProps as MuiButtonProps
} from "@material-ui/core/Button";

type ButtonProps = Omit<MuiButtonProps, "variant"> & {
  variant: "text" | "outlined" | "contained" | "dashed";
};

const useStyles = makeStyles(theme => ({
  root: ({ variant }: ButtonProps) => ({
    ...(variant === "dashed" && {
      border: "2px dashed grey"
    })
  })
}));

const Button: React.FC<ButtonProps> = props => {
  const variantClassses = useStyles(props);
  const { classes: propsClasses, variant: propsVariant, ...rest } = props;

  const classes = deepmerge(variantClassses, propsClasses);
  const variant = propsVariant !== "dashed" ? propsVariant : undefined;

  return <MuiButton classes={classes} variant={variant} {...rest} />;
};

export default function App() {
  return (
    <Button variant="dashed" color="secondary">
      Custom variant
    </Button>
  );
}

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 https://github.com/mui-org/material-ui/pull/21648 is implementing this feature for the Button component. This is how it can be used:

import React from 'react';
import {
  createMuiTheme,
  makeStyles,
  ThemeProvider,
} from '@material-ui/core/styles';
import Button from '@material-ui/core/Button';

const theme = outerTheme => createMuiTheme({
  variants: {
    MuiButton: [
      {
        props: { variant: 'dashed' },
        styles: {
          padding: '5px 15px',
          border: `5px dashed ${ ${outerTheme.palette.primary.main}}`,
        },
      },
      {
        props: { variant: 'dashed', color: 'secondary' },
        styles: {
          padding: '5px 15px',
          border: `5px dashed ${outerTheme.palette.secondary.main}`,
        },
      },
    ],
  },
});

export default function App() {
  return (
    <ThemeProvider theme={theme}>
      <Button variant="dashed" color="secondary">
        Custom variant
      </Button>
    <ThemeProvider>
  );
}

The typescript users, can use module augmentation for defining their new variants types:

declare module '@material-ui/core/Button/Button' {
  interface ButtonPropsVariantOverrides {
    dashed: true;
  }
}

Drawbacks

Always with new API we have to consider also the drawbacks of adding it. Here are some points:

Alternatives

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 new classKeys 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 and label slots in the Button.

const theme = outerTheme => createMuiTheme({
  variants: {
    MuiButton: [
      {
        props: { variant: 'dashed' },
        styles: {
          root: {
            padding: '5px 15px',
            border: `5px dashed ${ ${outerTheme.palette.primary.main}}`,
          },
          label: {
            color: outerTheme.palette.primary.main;
          }
        },
      },
      {
        props: { variant: 'dashed', color: 'secondary' },
        styles: {
          root: {
            padding: '5px 15px',
            border: `5px dashed ${ ${outerTheme.palette.secondary.main}}`,
          },
          label: {
            color: outerTheme.palette.secondary.main;
          },
        },
      },
    ],
  },
});

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.

oliviertassinari commented 4 years ago

Unresolved questions

@mnajdova Which started at https://github.com/mui-org/material-ui/pull/21648#discussion_r450362827. 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:

https://github.com/mui-org/material-ui/blob/f432112f0bcfb1fd2002002db181e83edc99d906/packages/material-ui/src/Slider/Slider.js#L286

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.

https://github.com/mui-org/material-ui/blob/f432112f0bcfb1fd2002002db181e83edc99d906/packages/material-ui/src/Chip/Chip.js#L127-L135

I would vote for waiting. The alternative will be to target, with a CSS global selector, the class name.

sakulstra commented 4 years ago

At the moment component variations are usually implemented as some sort of wrappers, which makes them self-sufficient components you can easily package & share to other projects. They are usually either wrapped as new component <DashedButton /> which utilizes <Button /> under the hood, or as reexport with a new variant <Button variant="fancy">Fancy</Button/> like in https://github.com/mui-org/material-ui/issues/15573#issuecomment-489054784

How/Would sharing such a themed component work with this new api? As I get it you cannot simply package/import this component, but have to inject the custom theme variation into the theme.

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).

toniocodo commented 4 years ago

👍 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. It would also make maintenance much easier as only this theme object would have to be maintained on MUI upgrades.

mnajdova commented 4 years ago

At the moment component variations are usually implemented as some sort of wrappers, which makes them self-sufficient components you can easily package & share to other projects. They are usually either wrapped as new component which utilizes

@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.

taschetto commented 4 years ago

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.

oliviertassinari commented 4 years ago

its efforts on things that will add more value

@taschetto Thanks for the feedback. Do you have specific items in mind?

taschetto commented 4 years ago

its efforts on things that will add more value

@taschetto Thanks for the feedback. Do you have specific items in mind?

Promoting lab components to core would be great.

FernandoGOT commented 4 years ago

I think that this feature is fantastic, I just suggest something like a property base, so we can define from which variant it is inheriting its base and just apply another “layer” of css. Ex:

  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 primary button, plus the override/styles that I provided for the dashed variant

whoisryosuke commented 4 years ago

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!

mnajdova commented 4 years ago

@whoisryosuke these are really good questions, thanks for looking into this :) Here are some thoughts/answers.

How does the matcher property work exactly? All the examples I see use variant as the prop I'm targeting, but what if I want to create a component with a size prop? (e.g.

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: { size: 'large' }, it would map to the sizeLarge classKey, if you specify both a variant and size, soemthing like { variant: 'dashed', size: 'large' }, it would map to the classKey: dashedSizeLarge. I am interested on what do you think is limited? Clients can basically define any props combination here, as long as the component supports it.

Is the name matcher misleading? Is props maybe a better name for it?

Why a matcher property vs more explicit property names? (e.g. { prop: "variant", match: "secondary", styles: {} -- similar to @oliviertassinari's suggestion here. This is an API I've seen other libraries use (like Styled System or xStyled), and it feels more semantic than "matcher". I'm targeting a prop -- it should be named that. Makes the structure of the variant more flat and easier to read/discern. I'm assuming this is done for perf reasons (I haven't peeked at the source yet) - but in terms of usability and UX/DX, I think a flat and explicit API is better.

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 { variant: 'dashed', size: 'large' }, we would need to convert the props matcher to array, something like [{prop: 'variant', value: 'dashed'}, {prop: 'size', value: 'large'}]. To me this seems like mimicing javascript object, so I really don't see the point of just specifying the object itself.. I am open for better API suggestions anyway, so please let me know if I am missing something here.

Why not group similar variants? - Depending on the number variants, the component styles might get harder to discern. This is one of the reasons why I recommended the flat structure above. This can also be resolved by grouping variants by the prop they target (see xStyled variants for an example). That way I can go through my theme and just collapse whatever variants I'm not working with and it eliminates the possibility they could get jumbled up in the theme.

Not sure I understand this correctly. Are you suggesting that multiple matchers combination can result in the same style?

How are extensions handled? @FernandoGOT makes a great point. Even in the RFC example, we can see a large amount of code duplication because the variants are nearly the same. I could see factoring out the styles into their own variables (like a dashedButton = { / styles here / } that gets used inside each variant). But this gets a little confusing, requiring me to create variables for things - name them properly - differentiate what props are similar - etc. An extends prop for the variant that picks up another variant and applies the current variant's styles as overrides would solve this. Styled System achieves this in their old variant API (not sure about the new component-level one).

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: You can always specify first a matcher that is more generic, like { variant: 'dashed' } and specify the styles for all dashed variants, and then specify the ones which are more specific, like { variant: 'dashed', color: 'secondary' } for example. It's not completely answering the question, but I thought is worth mentioning

How are variants shared across components? - Many times I'll create a variant that is modular enough to be used by other components (like a stateful variant that handles success/error/etc and changes bg color + text -- or a general scaling variant that changes font size). I'm assuming it's similar to manually extending a variant, you just define a variant as a separate variable (or function really because of outerTheme usage) and use that inside the theme variants as needed. An example of that would be interesting to see - if someone is willing to go down this road to customization, they're probably approaching that edge case as well.

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 👍

How does this work with the "Styled Components" API? - I've seen it mentioned a few times that this RFC is for handling custom variants on a theme level, and that there's already a solution for creating new components with variants -- but I haven't seen an example of extending a MUI component using the styled API and applying variants. With more complex design systems, I tend to work with the styled API because it allows for greater flexibility with things like CSS selectors. I could see myself using this theme-based custom variant API, but I feel like it'd start to clash with the styled component, or limit my variants to more simple properties. Libraries like xStyled offer a variant utility that allows developers to quickly create these variants inside the Styled syntax.

Currently this API is following the overrides API of defining the styles. It is a different topic that we are interesting in looking over of how this API can be used together with styled components.

whoisryosuke commented 4 years ago

@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:

Why not group similar variants? - Depending on the number variants, the component styles might get harder to discern. This is one of the reasons why I recommended the flat structure above. This can also be resolved by grouping variants by the prop they target (see xStyled variants for an example). That way I can go through my theme and just collapse whatever variants I'm not working with and it eliminates the possibility they could get jumbled up in the theme.

Not sure I understand this correctly. Are you suggesting that multiple matchers combination can result in the same style?

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 headline1 variants. These are separate objects, instead of being grouped by the fact they share the same variant key (despite using a different secondary matcher prop, color: secondary for one of the variant options).

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 prop instead of matcher is to allow for developers to define their own custom variant prop (like <Button size="large" color="green">). This allows developers to combine variants, something not possible with the current API if I can only use the variant prop. I'd have to create complex variants that combine variants (like dashed.large or something to represent "dashed" and size variances).

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 <LargeButton> or <DashedButton>), when components can/should be built with more flexibility (I don't create a <HoverButton>...I expect a button to contain this state/variant).

mnajdova commented 4 years ago

I have renamed the matcher to props, hopefully this removes the confusion of what it is. As the name suggest, it can be used with any combination of props, not just the variant prop (that prop is the the one that usually clients want to add new values to, so that's why the examples are connected to it). You can definitely define:

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 additions, but variants sounded much better. I see now that it can create confusion that it can be used only with the variant prop, which may be problematic..

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.

oliviertassinari commented 4 years ago

The first key in the theme I had in mind for this was additions, but variants sounded much better. I see now that it can create confusion that it can be used only with the variant prop, which may be problematic..

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 variant is a name most people will associate it with this purpose. I'm not sure we have a better candidate name for it, but we are definitely biased by prior-arts.

I am not sure if adding additional grouping by variant would be the best choice

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?

mnajdova commented 4 years ago

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...

mnajdova commented 4 years ago

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.

andrey1andrey commented 4 years ago

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

Jack-Works commented 3 years ago

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 "@material-ui/core": "5.0.0-alpha.5" (blocked by notistack that broke by a breaking change in alpha 6) but if the newer alpha allows us to customize in an easier way I'd like to upgrade. Also, is there a recommended pattern of extending those in v5?

oliviertassinari commented 3 years ago

@Jack-Works Do you have a specific example of a variant your designers want to support?

Jack-Works commented 3 years ago

For example, 4 sizes than 3 sizes (this library) and more color in the theme than primary or secondary

Jack-Works commented 3 years ago

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.

oliviertassinari commented 3 years ago

@Jack-Works Does it mean that <Typography variant> is working as expected for you and we could extend the approach to more props?

Jack-Works commented 3 years ago

I think the idea is great but it doesn't work unfortunately. My version of TypographyPropsVariantOverrides didn't merge with the declaration of the mui types. Does it work on your machine? I'm using pnpm don't know if it is affecting TypeScript type resolution.

oliviertassinari commented 3 years ago

It works great here: https://codesandbox.io/s/globalthemevariants-material-demo-forked-kqqly?file=/demo.tsx.

Jack-Works commented 3 years ago

Does it work if you move the declare module to a single file (so-called "ambient module" (a file that doesn't have a top-level import/export therefore it's inner declaration is global) in TS)? In a big project, it's not ok to define it everywhere when the component is used.

oliviertassinari commented 3 years ago

I would expect it to work from a different file, we use this approach with the lab to allow customizations with the theme.

Jack-Works commented 3 years ago

Strange, it doesn't work on my machine. I will try again later

Jack-Works commented 3 years ago

I have a minimal reprod to show it didn't work.

package.json

{
  "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 "jsx": "react" in the compilerOptions of tsconfig.json

decl.d.ts

declare module "@material-ui/core/Typography/Typography" {
    interface TypographyPropsVariantOverrides {
        cool: true;
        h1: false; // variant="h1" is no longer available
    }
}

test.ts

/// <reference path="./decl.d.ts" />
import { Typography } from "@material-ui/core";
import React from "react";
export default <Typography variant="cool" />;

Type of variant is (JSX attribute) variant?: "inherit" | "h1" | "h2" | "h3" | "h4" | "h5" | "h6" | "subtitle1" | "subtitle2" | "body1" | "body2" | "caption" | "button" | "overline" | undefined

And cool is not in it. So it's a type error.

mnajdova commented 3 years ago

@Jack-Works here is a working example - https://codesandbox.io/s/autumn-glade-zvl5n?file=/src/declarations.d.ts The thing you were missing in the decl.d.ts is disabling automatic exports like this:

decl.d.ts


declare module "@material-ui/core/Typography/Typography" {
interface TypographyPropsVariantOverrides {
cool: true;
h1: false; // variant="h1" is no longer available
}
}

Let me know if this will resolve your issue.

Jack-Works commented 3 years ago

Yes, that works thanks! But I'm curious why, based on what I learn writing export {} make this file a non-ambient module so it's inner declaration are local. Why it can be merged into the global scope?

oliviertassinari commented 3 years ago

@mnajdova Should we document it in https://next.material-ui.com/guides/typescript/#customization-of-theme?

mnajdova commented 3 years ago

@mnajdova Should we document it in https://next.material-ui.com/guides/typescript/#customization-of-theme?

Yes, will do it tomorrow

Jack-Works commented 3 years ago

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?

oliviertassinari commented 3 years ago

@Jack-Works you might want to follow this issue #13875

oliviertassinari commented 3 years ago

@mnajdova Is there anything left to be done on this RFC? I'm asking because it seems that we are done (related to #15573).

mnajdova commented 3 years ago

@mnajdova Is there anything left to be done on this RFC? I'm asking because it seems that we are done (related to #15573).

Correct, we are done. We can close it.

EliasJorgensen commented 2 years ago

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. :smile:

mnajdova commented 2 years ago

@EliasJorgensen we are discussing this currently with the development of the new design system cc @siriwatknp In some components the variant is used for more than styling purposes, so we cannot reuse it. I would propose in the meantime using some data-* attribute, for example, something like - https://codesandbox.io/s/jolly-waterfall-6piyj?file=/src/App.tsx or use a wrapper component.

joao-mambelli commented 2 years ago

would be nice to be able to create variants for specific slots. now the variants are always applied to the root slot.