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
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
}}
/>
@PabloDinella There are two issues in your example:
Could not find the referenced rule checked in Component.
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
- You are referencing unknown style rules. JSS warns with this message:
Could not find the referenced rule checked in Component.
- The
checked
anddisabled
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.
Most helpful comment
@PabloDinella There are two issues in your example:
Could not find the referenced rule checked in Component.
checked
anddisabled
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:
and
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