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 enableColorScheme option to getInitColorSchemeScript #33261
Conversation
@@ -75,6 +84,9 @@ export default function getInitColorSchemeScript(options?: GetInitColorSchemeScr | |||
if (colorScheme) { | |||
${colorSchemeNode}.setAttribute('${attribute}', colorScheme); | |||
} | |||
if (${enableColorScheme} && !!cssColorScheme) { |
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.
if (${enableColorScheme} && !!cssColorScheme) { | |
if (enableColorScheme && !!cssColorScheme) { |
?
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.
if (${enableColorScheme} && !!cssColorScheme) {
is correct because the whole function is in a literal template string.
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.
Ah it is a variable outside of the __html
string, makes sense 👍
@siriwatknp This change breaks the dark mode on http://mui.com/, it was released in v5.8.6. Compare:
In practice, it's a regression from #29454. I think that the issue is here: Shouldn't we get a single light/dark mode for @mui/system, and have Material UI/Joy UI directly rely on it? We might also want to remove maybe something like this: diff --git a/packages/mui-material/src/CssBaseline/CssBaseline.js b/packages/mui-material/src/CssBaseline/CssBaseline.js
index 2b32bd5789..39579fab5a 100644
--- a/packages/mui-material/src/CssBaseline/CssBaseline.js
+++ b/packages/mui-material/src/CssBaseline/CssBaseline.js
@@ -11,7 +11,11 @@ export const html = (theme, enableColorScheme) => ({
boxSizing: 'border-box',
// Fix font resize problem in iOS
WebkitTextSizeAdjust: '100%',
- ...(enableColorScheme && { colorScheme: theme.palette.mode }),
+ ...(enableColorScheme && {
+ [theme.getColorSchemeSelector('dark')]: {
+ colorScheme: 'dark',
+ },
+ }),
});
export const body = (theme) => ({ |
@siriwatknp Could you prioritize fixing this regression? It seems important, thanks |
The
enableColorScheme
option did not pass togetInitColorSchemeScript
. By adding this option, the script attaches the color-scheme before the application is rendered on the screen.Next step: I will add a script to minimize via terser.