Material-ui: Add global class names: disabled, focused, selected and checked for state modifiers?

Created on 4 Jan 2019  ·  9Comments  ·  Source: mui-org/material-ui

Pretty sure I'm missing something, but I can't figure out why this isn't working:

I want to override the colorPrimary styling for <Radio />, but the color I want is not being applied. Am I doing something wrong or this is a bug?

I copied default styles from Radio implementation.

Sandbox: https://codesandbox.io/s/pp61mnwyjj

discussion important

Most helpful comment

@PabloDinella There are two issues in your example:

  1. You are referencing unknown style rules. JSS warns with this message:
    Could not find the referenced rule checked in Component.
  2. The checked and disabled class names aren't applied to the element.

@joshwooding I'm wondering. What do you think of adding global class names: disabled, focused, selected and checked for state modifiers? The correct solutions will then be:

const styles = theme => ({
  radio: {
    "&$checked": {
      color: "green"
    },
    "&$disabled": {
      color: theme.palette.action.disabled
    }
  },
  checked: {},
  disabled: {}
});

<Radio
  classes={{
    checked: classes.checked,
    colorSecondary: classes.radio,
    disabled: classes.disabled
  }}
/>

and

const styles = theme => ({
  radio: {
    "&.checked": {
      color: "green"
    },
    "&.disabled": {
      color: theme.palette.action.disabled
    }
  },
});

<Radio className={classes.radio} />

The implication overhead is minimal, it can improve the DX. I fail to see where it can be error-prone. The only downside I can think of is about adding another way of doing stuff 🤷‍♂️. I have a coworker often using global class names because he tired of defining empty style rules.

I like that! cc @mui-org/core-contributors

All 9 comments

const styles = theme => ({
  radio: {
    "&$checked": {
      color: "green"
    },
    "&$disabled": {
      color: theme.palette.action.disabled
    }
  },
  checked: {},
  disabled: {}
});

<Radio
  classes={{
    checked: classes.checked,
    colorSecondary: classes.radio,
    disabled: classes.disabled
  }}
/>

https://codesandbox.io/s/vm0n7qyxx7

@PabloDinella There are two issues in your example:

  1. You are referencing unknown style rules. JSS warns with this message:
    Could not find the referenced rule checked in Component.
  2. The checked and disabled class names aren't applied to the element.

@joshwooding I'm wondering. What do you think of adding global class names: disabled, focused, selected and checked for state modifiers? The correct solutions will then be:

const styles = theme => ({
  radio: {
    "&$checked": {
      color: "green"
    },
    "&$disabled": {
      color: theme.palette.action.disabled
    }
  },
  checked: {},
  disabled: {}
});

<Radio
  classes={{
    checked: classes.checked,
    colorSecondary: classes.radio,
    disabled: classes.disabled
  }}
/>

and

const styles = theme => ({
  radio: {
    "&.checked": {
      color: "green"
    },
    "&.disabled": {
      color: theme.palette.action.disabled
    }
  },
});

<Radio className={classes.radio} />

The implication overhead is minimal, it can improve the DX. I fail to see where it can be error-prone. The only downside I can think of is about adding another way of doing stuff 🤷‍♂️. I have a coworker often using global class names because he tired of defining empty style rules.

I like that! cc @mui-org/core-contributors

@oliviertassinari I like that solution a lot it massive simplifies the state override story and I can see it making a big difference to the frequency of questions asked about it.

@oliviertassinari

  1. You are referencing unknown style rules. JSS warns with this message:
    Could not find the referenced rule checked in Component.
  2. The checked and disabled class names aren't applied to the element.

Alright, I didn't know about this $ref syntax. Now I understood that "&$checked" translates to something like .parent.checked and that's why I need to pass checked/disabled overrides, everything makes sense now. I just can't find any docs for this syntax, the closest I found is this. Are there better docs on this?

One more thing, shouldn't it be described in material-ui docs, how to customize Radio's colorPrimary and colorSecondary in more detail? I find this API section not much instructive and this demo has an example, but an explanation would be useful (in my case for example).

Are there better docs on this?

@PabloDinella We have tried to document the $ruleName syntax in https://material-ui.com/customization/overrides/#use-rulename-to-reference-a-local-rule-within-the-same-style-sheet.

One more thing, shouldn't it be described in material-ui docs, how to customize Radio's colorPrimary and colorSecondary in more detail?

What do you think of this methodology: https://material-ui.com/customization/overrides/#using-the-dev-tools?

@PabloDinella We have tried to document the $ruleName syntax in https://material-ui.com/customization/overrides/#use-rulename-to-reference-a-local-rule-within-the-same-style-sheet.

Perfect.

What do you think of this methodology: https://material-ui.com/customization/overrides/#using-the-dev-tools?

It's a good tip, but I can't see how it helps in this case. What I thought was something like in the demo page for selection controls:

Radio Buttons with custom colors

Radio primary and secondary colors can be overridden by passing a root class with nested classes for checked and disabled states.

Example for changing color while checked:

const styles = theme => ({
  radio: {
    "&$checked": {
      color: "green"
    },
  },
  checked: {},
});

<Radio
  classes={{
    checked: classes.checked,
    colorSecondary: classes.radio,
  }}
/>

The "&$checked" syntax is described here.

I mean, when people wonder how to override the default color, they will refer to the component demo or api ref.

Does it make sense?

I do think having more example of overriding a component's style would help

We would go full circle from global css, to local css-in-js and back to global classnames again. If there's already one way (that doesn't pollute the global namespace) I'd rather stick with it and improve docs if that is the issue instead of adding (basically) another syntax for doing the same thing.

adding (basically) another syntax for doing the same thing.

@eps1lon Yeah ok. Let's close. People can already use the global class name option to make their life easier. They can also directly use the withStyles(styles)(MuiComponent)API, it can be more intuitive.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sys13 picture sys13  ·  3Comments

iamzhouyi picture iamzhouyi  ·  3Comments

finaiized picture finaiized  ·  3Comments

reflog picture reflog  ·  3Comments

TimoRuetten picture TimoRuetten  ·  3Comments