-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[system] Add applyStyles()
to theme
#40667
Conversation
Netlify deploy previewhttps://deploy-preview-40667--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
* }) | ||
*``` | ||
*/ | ||
export default function applyStyles<K extends string>(key: K, styles: CSSObject) { |
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.
Can this be more generic? ie, can this be also used to apply styles for say, dir="rtl/ltr"
?
Or do we need a separate function for that?
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 might be wrong but I don't think it's the responsibility of applyStyles()
because applyStyles
is for customization that's related to colors.
I have never seen a case where rtl/ltr are differ in terms of colors. Moreover, the rtl/ltr should be done at a higher level. In emotion, it's done by a stylis plugin that changes CSS properties, e.g. from marginLeft
to marginRight
.
// If CssVarsProvider is used as a provider, | ||
// returns ':where([data-mui-color-scheme="light|dark"]) &' | ||
const selector = (this as Theme) | ||
.getColorSchemeSelector('dark') |
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.
Can you verify if this is defined in material package ? I stumbled upon this when solving another issue.
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.
getColorSchemeSelector
is defined when CssVarsProvider
is used.
I think it should be attached to the theme by extendTheme
. I will create another issue to tackle this separately.
@@ -45,7 +45,6 @@ export interface Theme extends BaseTheme { | |||
components?: Components<BaseTheme>; | |||
unstable_sx: (props: SxProps<Theme>) => CSSObject; | |||
unstable_sxConfig: SxConfig; | |||
applyDarkStyles: (css: CSSObject) => CSSObject; |
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.
applyStyles
is already declared in the SystemTheme
.
Shouldn't this have been a breaking change since it's renaming a function and be part of a major version? |
This is our mistake. The new API is introduced in Anyway, since we haven't documented it yet, I hope that it won't affect anyone. Sorry for the mistake. |
Motivation
Note: it replaces the existing
theme.applyDarkStyles
from #40324Examples
Docs
I won't add this utility to the docs for now to battle test with zero-runtime a bit more.