Skip to content
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

[withStyles] Add the ability to get the component's props from within the style object #7633

Closed
GabeDuarteM opened this issue Aug 2, 2017 · 39 comments
Assignees
Labels
new feature New feature or request priority: important This change can make a difference
Milestone

Comments

@GabeDuarteM
Copy link
Contributor

GabeDuarteM commented Aug 2, 2017

Description

When using createStyleSheet, there is no way (at least that i know of) to access the component's props. I think this is important, as sometimes it is wanted to pass sizes, url images, and other stylings as props to the component.

Today the only solution to this is to separate these stuff into inline-styles, but as far as i know, material-ui v1 uses jss and react-jss, and those two already gives you the possibility of access the props via functions that receive the props and then return the desired style, like that:

const styles = {
  button: {
    background: props => props.color
  },
  root: {
    backgroundImage: props => `url(${props.backgroundImage})`
  }
}

// Reference: https://github.com/cssinjs/react-jss#example

What do you think of implementing something like that on material-ui too?

@GabeDuarteM GabeDuarteM changed the title [Feature request] Add the ability to get the component's props from within the style object [Feature request: v1] Add the ability to get the component's props from within the style object Aug 2, 2017
@oliviertassinari
Copy link
Member

react-jss implement that feature. You can always use it over our styling solution. The real motivation for implementing that feature here should be around simpler/better internal implementation of the components.

@oliviertassinari oliviertassinari changed the title [Feature request: v1] Add the ability to get the component's props from within the style object [core] Add the ability to get the component's props from within the style object Aug 2, 2017
@oliviertassinari oliviertassinari added new feature New feature or request and removed core labels Aug 22, 2017
@lewisdiamond
Copy link

react-jss injectStyles() does this too, but it seems to be that it would be better to add props to the StyleRuleCallback.

const styles = (props, theme) => ({})
That way you are not limited to only values depending on props.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 12, 2017

@Shamplol feedback makes me think that we could be taking advantage of this new feature to allow users providing a custom color to our components, instead of the predefined one. The CSS size impact is unclear but it would make the component color override even simpler. This is something to investigate. Then, once css variables browser support is high enough (2 years from now?), we can rely on it.

@sakulstra
Copy link
Contributor

sakulstra commented Nov 1, 2017

@oliviertassinari wouldn't the css size actually decrease in some cases?
As i get it, we currently define all classes for ...Primary and ...Accent - wouldn't this change mean, that we would only have to maintain classes for ...Colorized? Or are you concerned about the generated css?

Either way, I think this would hugely improve dx as we have to basically reimplement complex classes like https://github.com/callemall/material-ui/blob/v1-beta/src/Button/Button.js#L11 when we want to use non palette colors.

@oliviertassinari
Copy link
Member

wouldn't the css size actually decrease in some cases?

@sakulstra Hard to tell. It will depend on the implementation. Maybe :).

@pelotom
Copy link
Member

pelotom commented Nov 9, 2017

From a TypeScript typing perspective, it would be nice if both props and theme could be accessed this way within a styles specification:

const styles = {
  button: {
    background: ({ theme }) => theme.palette.primary[200]
  },
  root: {
    backgroundImage: ({ props }) => `url(${props.backgroundImage})`
  }
};

The reason is that TypeScript frequently fails to infer the right type for withStyles when it is passed a function object, so you have to provide extra type annotations to make it work, e.g.

withStyles<'button' | 'root'>(theme => ...)

If props are passed in too, this will become

withStyles<'button' | 'root', Props>((theme, props) => ...)

@lucasmafra
Copy link

What's the current status of this? It would be really nice to have that feature

@oliviertassinari
Copy link
Member

@lucasmafra I have added this feature to the post v1 release milestone. The sooner we can release v1, the better.

@carlgunderson
Copy link

This functionality is key to being able to write expressive style rules that combine props, media queries, and interactive states. You can't replace that functionality with inline styles. Unfortunately, withStyles is unusable in any of my projects until this gets added.

@designjudo
Copy link

You can always setup a custom theme, and use this convention: We really like how styled components gives you access to the props.theme out of the box by nesting ThemeProvider inside MuiThemeProvider when both called theme={theme}. It extends the default theme that mui exposes

//inline with HOC Method
 h1 style= {{ 'color: this.props.theme.palette.primary[500]' }}

//styled components ( no method necessary )
const Heading = styled.h1`
  color: ${p => p.theme.palette.primary['500']};
`

@damien-monni
Copy link

damien-monni commented Mar 23, 2018

I just needed to use function values, so I used react-jss' injectSheet() function. I am used to use Mui's withStyles() function, so is there any drawback when using injectSheet instead of withStyles?

@pelotom
Copy link
Member

pelotom commented Aug 5, 2018

@oliviertassinari this probably has significant implications for the typing of withStyles, let me know if I can help with that

@oliviertassinari
Copy link
Member

@pelotom Thanks, I will let you know. I hope I can start looking at the issue this month.

@gillchristian
Copy link

gillchristian commented Aug 17, 2018

Is there work in progress regarding this? It's a key feature IMO, maybe I could help with it.

EDIT: I started to work on it. Managed to pass props to the styles function that withStyles accepts, only problem is styles are not updated when props change. Will create a PR when that is solved.

@japrogramer
Copy link

Hi I just came across a use case where i needed this to customize the colors of the avatar component and there is no other way to control the style for all the variants of the component other than this.

const styles = theme => ({
  chip:{
  },
  separator: {
    marginRight: theme.spacing.unit,
  },
  fright: {
    float: 'right',
  },
  fleft: {
    float: 'left',
  },
  outlinedPrimary:{
    color: props =>  stringToColor( props.username),
    border: props => `1px solid ${fade(stringToColor(props.username), 0.5)}`,
    '$clickable&:hover, $clickable&:focus, $deletable&:focus': props => ({
      backgroundColor: fade(stringToColor(props.username), theme.palette.action.hoverOpacity),
      border: `1px solid ${stringToColor(props.username)}`,
      }),
  },
  outlined: {
    backgroundColor: 'transparent',
    border: props => `1px solid ${
      theme.palette.type === 'light' ? stringToColor(props.username) : fade(stringToColor(props.username))
    }`,
    '$clickable&:hover, $clickable&:focus, $deletable&:focus': props => ({
      backgroundColor: fade(stringToColor(props.username), theme.palette.action.hoverOpacity),
      }),
    },
});

@JacquesBonet
Copy link

JacquesBonet commented Aug 31, 2018

You can check my solution Japrogramer: https://github.com/JacquesBonet/jss-material-ui

@japrogramer
Copy link

thanks, I will take a look at it.

@2p4b
Copy link

2p4b commented Sep 4, 2018

I needed this feature earlier today so i wrote a HOC. withStyles does some caching on its own so i can't really tell how much this affect that but i will look at the caching implementation of withStyles in my spare time, for now anyone looking for a quick way to get props and theme to play nice there you go

Warning

This component will do a full remount if props change something to do with the index number of the stylesheet class changing or somthing in the withStyles HOC

import React from 'react';
import { withStyles } from '@material-ui/core/styles';

const { createElement, forwardRef } = React

const withPropsStyles = ( style ) => {

    const withPropsStyles = ( component ) => {

        return forwardRef( (props, ref) => {

            const proxy = (theme) => style(props, theme)

            const hoc = withStyles(proxy)(component)

            return props.children ? 
                createElement(hoc, { ...props, ref}, props.children) :
                createElement(hoc, {...props, ref}) 
        }) 
    }

    return withPropsStyles
}

export default withPropsStyles

Example use

const styles = (props, theme) => ({
    root:{
        backgroundColor: props.light ? theme.palette.primary.light : theme.palette.primary.main 
    },
})

const SomeComponent = ({classes}) => <div className={classes.root}/>

export default withPropsStyles(styles)(SomeComponent)

conclusion

Simple and works(but rem the full remount cost too)

@koshea
Copy link
Contributor

koshea commented Sep 4, 2018

With this change can we remove inline style usage from the library in favor of 100% JSS? My app does not work with inline styles and when I went to replace the ripple effect with JSS I realized I needed this feature. Maybe a small performance hit, but seems cleaner.

@2p4b
Copy link

2p4b commented Sep 4, 2018

@koshea ditch the inline style. I also really don't like inline styles too, it should work just fine as a drop in replacement for withStyles as a decorator or as in the example.

@cherniavskii
Copy link
Member

cherniavskii commented Sep 27, 2018

I also wanted to mention, that using inline styles doesn't allow to enable strong Content Security Policy.
It requires adding unsafe-inline flag to styles directives, which is not secure.

Dynamic styles with props support should resolve that problem

@leomeloxp
Copy link

Hi guys, sorry to jump in to the discussion "just like this". I recently started using Mui (with Typescript) and whilst I find it an extremely powerful library, it certainly has its complexities.

I noticed in some comments above that there's a bit of a discussion for whether this feature should be (props, theme) => {} or (theme, props) => {}. I'd like to reinforce what @pelotom said about making both props and theme named properties in his comment. Making it that way will probably make it easier for us to refactor the style definitions once this change lands (which I'm really looking forward to). Cheers 🙂

@oliviertassinari
Copy link
Member

Thank you everyone for the patience! This issue is being taken care of in #13503. It's a requirement for the component helpers we want to implement. We have also started experimenting with the hook API: https://twitter.com/olivtassinari/status/1058805751404261376.

@city41
Copy link
Contributor

city41 commented Jun 5, 2019

Did this make it into 4.0? It looks like the makeStyles callback does not have a props parameter.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 5, 2019

@city41 const classes = useStyles(props);

@city41
Copy link
Contributor

city41 commented Jun 5, 2019

I see. So it looks like it is

const useStyles = makeStyles(theme => {
    return {
        foo: {
            color: theme.props.danger ? '#ff0000' : '#00ff00'
        }
    };
});

function MyComponent(props) {
    const classes = useStyles(props);

    return <div className={classes.foo}>...</div>;
}

I don't see this documented in the styles API section on the website. Let me see if I can send a PR.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 5, 2019

@city41 There is a start of a documentation in https://material-ui.com/styles/basics/#adapting-based-on-props.

@city41
Copy link
Contributor

city41 commented Jun 5, 2019

cool, glad to see the docs are getting updated. for anyone else coming to this issue, here is how I combined theme and props to style a component

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

interface ButtonProps {
    destructive: boolean;
}

const useButtonStyles = makeStyles<Theme, ButtonProps>(theme => {
    return {
        root: props => ({
            backgroundColor: props.destructive ? theme.palette.error.main : theme.palette.primary.main
        })
    };
});

export const PrimaryButton: React.FunctionComponent<ButtonProps> = props => {
    const classes = useButtonStyles(props);

    return <Button {...props} className={classes.root} variant="contained" />;
};

@mfaizan1
Copy link

mfaizan1 commented Mar 5, 2020

How can i use prop in an external styles json file ?

for example this is external file

const typographyStyle = {
  title2: {
    fontFamily:"Montserrat",
    fontStyle:"normal",
    fontWeight:"800",
    fontSize:"72px",
    lineHeight:"88px",
    letterSpacing:"-0.02em",
    // color:"#304C82"
    color : props => {
      console.log(props,'c1');
      return props.color
    }

  }
};

export default typographyStyle;

i import this file and spread the object

import typographyStyle from "../assets/jss/material-kit-pro-react/views/componentsSections/typographyStyle";


const styles = theme =>  ({
    ...typographyStyle,
    homeSearch:{
        width: '100%',
        '& div':{
            '& input':{
                "color":"#304C82",
                height: 65,
                fontFamily: 'Open Sans',
                fontStyle: 'normal',
                fontWeight: 800,
                fontSize: '48px',
                lineHeight: '65px',
                letterSpacing: '-0.03em',
                '&::placeholder':{
                    fontFamily: 'Open Sans',
                    fontStyle: 'normal',
                    fontWeight: 800,
                    fontSize: '48px',
                    lineHeight: '65px',
                    letterSpacing: '-0.03em',
                    color: '#EAEAEA'
                }
            }
        }
    },

  });

now on color function i get props = {} .

can someone help me in this regard ?

UPDATE:

seems like i am doing something wrong cause i am getting empty object even in my main styles.js file

    homeSearch: props => {
        console.log(props);
        return {
        width: '100%',
        border:  `1px solid ${props.color}`
        ,
        '& div':{
            '& input':{
                "color":"#304C82",
                height: 65,
                fontFamily: 'Open Sans',
                fontStyle: 'normal',
                fontWeight: 800,
                fontSize: '48px',
                lineHeight: '65px',
                letterSpacing: '-0.03em',
                '&::placeholder':{
                    fontFamily: 'Open Sans',
                    fontStyle: 'normal',
                    fontWeight: 800,
                    fontSize: '48px',
                    lineHeight: '65px',
                    letterSpacing: '-0.03em',
                    color: '#EAEAEA'
                }
            }
        }
    }
},

@mui mui locked as resolved and limited conversation to collaborators Mar 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new feature New feature or request priority: important This change can make a difference
Projects
None yet
Development

No branches or pull requests