material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[system] Support new props with variants

Open galechus opened this issue 5 years ago • 6 comments

Hi, is this possible extend PaperClassKey and PaperProps with custom values?

I would like to get something like this:

export const theme = createMuiTheme({
    overrides: {
        MuiPaper: {
            dark: {
                backgroundColor: COLORS.BLACK,
            },
            light: {
                backgroundColor: COLORS.WHITE,
            },
        },
    },
});

And using the component would look like this:

<Paper filled="dark" />

galechus avatar Jan 29 '20 13:01 galechus

@galechus We have worked on this problem in v5 with #15573. We have solved the problem for existing props as well as for JavaScript with new props:

import * as React from "react";
import { ThemeProvider, createMuiTheme } from "@material-ui/core/styles";
import Paper from "@material-ui/core/Paper";

const theme = createMuiTheme({
  components: {
    MuiPaper: {
      variants: [
        {
          props: { filled: "dark" },
          style: {
            backgroundColor: "#000",
            color: "#fff"
          }
        }
      ]
    }
  }
});

export default function Demo() {
  return (
    <ThemeProvider theme={theme}>
      <Paper filled="dark">filled="dark"</Paper>
    </ThemeProvider>
  );
}
Screenshot 2021-04-18 at 00 22 08

https://codesandbox.io/s/continuousslider-material-demo-forked-nxq1s?file=/demo.tsx:0-562

However, it doesn't work with TypeScript:

Screenshot 2021-04-18 at 00 39 54

For benchmark, I could get it working with Stiches: https://codesandbox.io/s/stitches-demo-with-stitches-reset-forked-yedfi?file=/src/App.tsx, well done @peduarte.

import * as React from "react";
import { styled } from "@stitches/react";

const Paper = styled("div", {
  variants: {
    filled: {
      dark: {
        backgroundColor: "#000",
        color: "#fff"
      }
    }
  }
});

export default function App() {
  return <Paper filled="dark">hello</Paper>;
}

Regarding the next steps, I think that:

  • We can use this issue to keep track of our progress on the support of new props, as well as collect developers' interest.
  • Maybe it would make sense to add in the styled() method the support for a variants key with the same API, as in the theme. This will make it easier for developers to switch between the two. Not sure, we can wait for requests. I have shared the API Stiches above, Chakra has something similar.

cc @mnajdova

oliviertassinari avatar Apr 17 '21 23:04 oliviertassinari

@oliviertassinari I stumbled upon this issue as well. Could something like this be possible?

Assuming I want to add a new prop to say the Avatar component: prop is "size" with values "small", "medium" and "large". I can do something like this to get rid off the first error:

import type {
  AvatarProps,
} from "@material-ui/core/Avatar"

// Create new sizes for Avatar
declare module "@material-ui/core/Avatar" {
  interface AvatarProps {
    size: "small" | "medium" | "large"
  }
}

Even though it works in the theme overrides, it still throws an missing prop Error while actually using the component. In this case, it would seem prudent to also provide a way to override Props in the same vein as Variant overrides. Something like this: AvatarPropsOverrides. So in the Avatar example, the type definitions would be:

export interface AvatarPropsVariantOverrides {}

export interface AvatarPropsOverrides {}

export interface AvatarTypeMap<P = {}, D extends React.ElementType = 'div'> {
  props: P & {
    ...
  } & AvatarPropsOverrides;
  defaultComponent: D;
}

Then the type module augmentation would be:

import type {
  AvatarPropsOverrides,
} from "@material-ui/core/Avatar"

// Create new sizes for Avatar
declare module "@material-ui/core/Avatar" {
  interface AvatarPropsOverrides {
    size?: "small" | "medium" | "large"
  }
}

This should solve the issue IMHO. Would be happy to send a pull request if this is acceptable.

shripadk avatar Jul 19 '21 19:07 shripadk

@shripadk although this looks good, there are other aspects that we need to solve before this is possible.

One I could think of immediately is, how will the component know which props to forward to the root element? Currently, all unhandled props are passed, now we will need to have a more complex solution here, for example forward only valid HTML props.

mnajdova avatar Jul 20 '21 10:07 mnajdova

@mnajdova I did not realise that custom props are being passed down to the native DOM element. I thought the default behaviour was not to pass down any prop unless it is whitelisted in the implementation of the component. So I assumed that custom props were utilized only for generating styles for the component. Thanks for providing clarity on that. Yes I agree with you that it does require a more complex solution.

shripadk avatar Jul 20 '21 10:07 shripadk

I would like to refer you to a related issue and my comment there: https://github.com/mui/material-ui/issues/34831#issuecomment-2470589633

It is unacceptable that you have to rebuild half of a component to extend its props just because there is no interface in the theme to prevent the added props from being forwarded to the DOM.

ChristianGrete avatar Nov 12 '24 14:11 ChristianGrete

More than 3 years to prevent forwarded props and yet no solution zzz

GuilhermeMaier avatar Oct 14 '25 14:10 GuilhermeMaier