Closed xaviergonz closed 6 years ago
I agree!
Couldn't there be something like this instead?
<Button colorOverrides={{
The reason that isn't currently possible is the very same reason the
color
prop is an enum to begin with: it isn't possible to modify the generated CSS classes via props.
Each of the color
s you can specify exist as a predefined class (customizable via the palette). Changing the color prop changes which of these classes is applied. If we could inject prop values into the generated styles (as needed for your proposed solution),there wouldn't be any restriction on the colors that you could use, so your solution wouldn't be needed. Catch 22.
I believe JSS has some developments in this regard, so never say never.
When I said "I agree" I was referring to the opportunity we have to make override simpler. I wasn't backing any specific implementation. There are some areas I want to look into:
I may have missed something, but why not wrap your custom red button component in a MuiThemeProvider
, and provide it a new theme with red being the primary color. You can then just use <Button color="primary" />
@ianschmitz I thought about that too, but I'm weary of performance.
@oliviertassinari couldn't it do something like what https://typestyle.github.io/#/core/-style- does? they basically generate a css class based on some style and with a class name which is the hash of the style. If the class is effectively the same (same attributes) then the same class is reused since the hash is the same. That basically allows you to keep using classes while using styles.
I'd be interested to hear more of the potential performance issues. It's a recommended pattern based on the docs (i use this extensively on work project).
Here is a simplified example:
const customTheme = outerTheme => ({
...outerTheme,
palette: {
...outerTheme.palette,
primary: red,
},
});
const Button = () => (
<MuiThemeProvider theme={customTheme}>
<Button color="primary">Custom Primary</Button>
</MuiThemeProvider>
);
I have a similar issue. The default color/backgroundcolor of the .MuiFormLabel-focused-186
and .MuiInput-inkbar-191:after
I want to change these colors for every textfield to #ffa000.
What is the best solution for this?
Thank you
@Eurythmax take a look at https://material-ui.com/customization/themes/#customizing-all-instances-of-a-component-type
Thanks @ianschmitz i have made my own Textfield now.
import React from 'react';
import { MuiThemeProvider, createMuiTheme } from 'material-ui/styles';
import TextField from 'material-ui/TextField';
const theme = createMuiTheme({
overrides: {
MuiInput: {
focused: {
color: '#ffa000',
},
inkbar: {
'&:after': {
backgroundColor: '#ffa000',
},
},
},
MuiFormLabel: {
focused: {
color: '#ffa000',
},
},
},
});
function TextFieldOverride(prop) {
console.log(prop.fullWidth); // eslint-disable-line
const fullWidth = (prop.fullWidth === 'true');
const autoFocus = (prop.autoFocus === 'true');
return (
<MuiThemeProvider theme={theme}>
<TextField
autoFocus={autoFocus}
margin={prop.margin}
id={prop.id}
label={prop.label}
type={prop.type}
fullWidth={fullWidth}
></TextField>
</MuiThemeProvider>
);
}
export default TextFieldOverride;
I'd be interested to hear more of the potential performance issues. It's a recommended pattern based on the docs (i use this extensively on work project).
Here is a simplified example:
const customTheme = outerTheme => ({
...outerTheme,
palette: {
...outerTheme.palette,
primary: red,
},
});
const Button = () => (
<MuiThemeProvider theme={customTheme}>
<Button color="primary">Custom Primary</Button>
</MuiThemeProvider>
);
@ianschmitz The performance implications of this approach is deeply linked to the JSS work needed behind the scene. The main point: We cache the injected CSS with the following tuple (styles
, theme
).
theme
: If you provide a new theme at each render, a new CSS object will be injected. So, instead of using the theme callback property of the MuiThemeProvider
, it's better to provide a static theme object. So, both for UI consistency and performance, it's better to render a bounded & limited number of theme object.styles
: The larger the styles object is, the more work is needed. Try having as few component in the children of MuiThemeProvider
It should be fast enough for most of the use cases, but to be cautious. I will document it.
How come this is closed? There is still no way to change the color and this does not work:
const customTheme = outerTheme => ({
...outerTheme,
palette: {
...outerTheme.palette,
primary: red,
},
});
Error: index.js:2178 Warning: Material-UI: missing color argument in fade(undefined, 0.12).
@joesanta555 I have added 2 examples in the docs: https://github.com/mui-org/material-ui/blob/faf1ead52970e2b1d4adafed8dc69aaa625c5763/docs/src/pages/demos/buttons/CustomizedButtons.js
Thank you. I read the closed issues before and could not find the 2 examples.
Finally a nice way of doing it π π
Thank you again for this
This is why I think the JSS/CSS-Modules solution is way overboard. I understand why you did it, but there are a lot of cases where it adds way more overhead than it is worth. Honestly, I would love to be able to disable it altogether.
This problem is losing me and my clients money as we speak...
My ultimate dream would be to use css-modules with some theming solution like react-toolbox
did. For modern browsers we could use CSS variables which would allow even nested theme support. Material-ui is my favourite ui framework by far, but nothing can beat css modules re performance and productivity. However, I am glad u picked JSS from css-in-js solutions, but missing css-modules anyway :) I know I can still use css-modules, but global Mui component theming must be done with JSS, plus I need to define some vars like colors etc in my css files, instead of importing those from default theme object
@robbyemmert why dont u like css-modules? :)
Honestly, I would love to be able to disable it altogether.
@robbyemmert From an architectural point of view, it should be possible to disable it and extract global CSS. But it would require a bit of effort. What's your issue? Have you seen https://material-ui.com/guides/interoperability/ or https://material-ui.com/customization/themes/#customizing-all-instances-of-a-component-type?
@klis87 We are looking into CSS variables support in #12827. We gonna try to follow the community with the most popular styling solutions.
@oliviertassinari that's fantastic news, can't wait for this!
I agree that changing button colors is too complex. I want to avoid nesting themes due to performance issues so I've made this custom button component, but I'm not sure if this would have even worse performance issues. I'd love some feedback on it.
It works fine and allows me to set any color for any button variants (i.e. either default
, inherit
, primaryLight
, primary
, primaryDark
, secondaryLight
, secondary
, secondaryDark
or any html color, such as #ff00ff
for either outlined, text or contained buttons). I can also override the font color of the button if needed, and the hover no longer depends on what theme.palette.[primary/secondary].dark
is - it's calculated using fade
or darken
instead.
Here's the code:
// ./Button/useColor.ts
import { useTheme } from "@material-ui/core/styles";
const useColor: UseColor = variant => {
const theme = useTheme();
switch (variant) {
case "primaryLight": {
return theme.palette.primary.light;
}
case "primary": {
return theme.palette.primary.main;
}
case "primaryDark": {
return theme.palette.primary.dark;
}
case "secondaryLight": {
return theme.palette.secondary.light;
}
case "secondary": {
return theme.palette.secondary.main;
}
case "secondaryDark": {
return theme.palette.secondary.dark;
}
case "default":
case "inherit": {
return undefined;
}
default: {
return variant;
}
}
};
export default useColor;
interface UseColor {
(variant?: string): string | undefined;
}
// ./Button/index.tsx
import React, { FC } from "react";
import { fade, darken } from "@material-ui/core/styles/colorManipulator";
import MuiButton, {
ButtonProps as MuiButtonProps
} from "@material-ui/core/Button";
import { makeStyles } from "@material-ui/core/styles";
import useColor from "./useColor";
const useStyles = makeStyles(theme => ({
textPrimary: (props: StylesProps) => {
if (props.bgColor) {
const color = props.fontColor || props.bgColor;
return {
color,
"&:hover": {
backgroundColor: fade(
props.bgColor,
theme.palette.action.hoverOpacity
)
}
};
}
return {};
},
containedPrimary: (props: StylesProps) => {
if (props.bgColor) {
const color =
props.fontColor || theme.palette.getContrastText(props.bgColor);
return {
color,
backgroundColor: props.bgColor,
"&:hover": {
backgroundColor: darken(props.bgColor, 0.25),
// Reset on touch devices, it doesn't add specificity
"@media (hover: none)": {
backgroundColor: "transparent"
}
}
};
}
return {};
},
outlinedPrimary: (props: StylesProps) => {
if (props.bgColor) {
const color = props.fontColor || props.bgColor;
return {
color,
border: `1px solid ${fade(color, 0.5)}`,
"&:hover": {
backgroundColor: fade(
props.bgColor,
theme.palette.action.hoverOpacity
),
border: `1px solid ${color}`
}
};
}
return {};
}
}));
const Button: FC<ButtonProps> = ({
children,
color = "default",
fontColor,
...props
}) => {
const { variant } = props;
const bgColor = useColor(color);
const classes = useStyles({ bgColor, fontColor });
let btnClasses: Record<string, string> | undefined;
let btnColor: MuiButtonProps["color"];
if (color !== "default" && color !== "inherit") {
btnColor = "primary";
switch (variant) {
case "contained": {
btnClasses = { containedPrimary: classes.containedPrimary };
break;
}
case "outlined": {
btnClasses = { outlinedPrimary: classes.outlinedPrimary };
break;
}
case "text":
case undefined: {
btnClasses = { textPrimary: classes.textPrimary };
break;
}
default:
break;
}
} else {
btnColor = color;
}
return (
<MuiButton {...props} classes={btnClasses} color={btnColor}>
{children}
</MuiButton>
);
};
export default Button;
export interface ButtonProps extends Omit<MuiButtonProps, "color"> {
readonly color?: string;
readonly fontColor?: string;
}
interface StylesProps {
readonly bgColor: string | undefined;
readonly fontColor: string | undefined;
}
I want to avoid nesting themes due to performance issues so I've made this custom button component, but I'm not sure if this would have even worse performance issues. I'd love some feedback on it.
Do you have an example that we could profile? I'm not aware of noticeable performance implications when nesting themes.
This is more a rant than a real issue really.
I used to have 4 colors for raised buttons: default - gray, primary - blue, secondary - green, contrast - red
Due to the recent change to the palette that removed the contrast color from I embarked into the task of making an additional custom red button and this is what I ended up with:
My first rant is about having to opt-in and out of classes depending on if the button is disabled or not. If I don't do this then, since the generated classes have more priority than the default ones then the disabled styles are never applied. Wouldn't it make more sense to make a deep merge of the current theme classes (including overrides) with the custom ones and then create class names based on that merge so that disabled overrides are not lost?
The second rant is about how overly complex this all seems. Couldn't there be something like this instead?
each of those properties would default to undefined, meaning no customization applied and therefore the API is backwards compatible
There's not so much of a problem with IconButton for example, since you can just use something like
still though it is funny how you have to be careful with not setting the color when disabled, so there could be something like icon, disabledIcon and ripple inside colorOverrides
And same thing about checkbox, etc.
Just my two cents π