-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
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
[core] Update typings for theme's components #26125
Conversation
// JSS property bag where values are based on props | ||
| CreateCSSProperties<Partial<ComponentsPropsList[Name]>> | ||
// JSS property bag based on props | ||
| PropsFunc< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a side note (of the scope of the PR). There might be something to consider regarding PropsFunc
. It's currently not implemented but the type accepts it: https://codesandbox.io/s/globalthemeoverride-material-demo-forked-x099w?file=/demo.tsx.
import * as React from "react";
import { ThemeProvider, createMuiTheme } from "@material-ui/core/styles";
import Button from "@material-ui/core/Button";
const theme = createMuiTheme({
components: {
MuiButton: {
styleOverrides: {
root: () => ({
fontSize: "1rem"
})
},
variants: [
{
props: { variant: "text" },
style: () => ({
textTransform: "none"
})
}
]
}
}
});
export default function GlobalThemeOverride() {
return (
<ThemeProvider theme={theme}>
<Button>font-size: 1rem</Button>
</ThemeProvider>
);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware we have an open issue about it, I would propose we open one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look into this next week 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be a function of the theme
? Not sure if props makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it depends on what we want to support for the API?
I would personally use the props & styleProps
from the styleOverrides
key. As for the variants
key, unclear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok let's start the discussion on an dedicated PR 👍
Updates the
theme.components.styleOverrides
andtheme.components.variants
to use the same typigns for the styles as theexperimentalStyled()
.