Skip to content
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

[website] Fix home page dark mode flicker #33545

Merged
merged 144 commits into from Oct 18, 2022

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Jul 17, 2022

Before: https://master--material-ui.netlify.app/
After: https://deploy-preview-33545--material-ui.netlify.app/

Changes

  • Most of the changes are refactoring existing styles to support CSS variables

    // example
    // From
    backgroundColor:
      theme.palette.mode === 'dark'
        ? alpha(theme.palette.primaryDark[900], 0.72)
        : 'rgba(255,255,255,0.72)',
        
    // to
    backgroundColor: 'rgba(255,255,255,0.72)',
    ...theme.applyDarkStyles({
      backgroundColor: alpha(theme.palette.primaryDark[900], 0.72)
    })
  • Add theme.applyDarkStyles() to docs theme: A utility for creating styles for dark mode. It will check the theme CSS variables and apply the proper styles for dark mode.

    e.g.

    theme.applyDarkStyles({
      color: '#fff'
    })
    
    // Result
    // when there are no CSS variables, fallback to object spread
    ...theme.palette.mode === 'dark' && {
      color: '#fff',
    }
    
    // For CSS variables, it uses data attribute
    ':where([data-muidocs-color-scheme="dark"]) &': {
      color: '#fff',
    }

    use :where() so that it does not create CSS specificity

    This utility is specific to our docs because there are pages that are not wrapped with CssVarsProvider. This way developers (us) don't need to write both syntaxes to apply dark mode styles.

  • Wrap the home page with CssVarsProvider and set CSS variable prefix to --muidocs so that it does not interfere with Material UI demos in the future.

  • Theme tokens do not change (copy from the existing implementation)

  • Workaround for :where() selector. This is an existing issue in emotion (as well as styled-components)


Part of #15588

@siriwatknp
Copy link
Member Author

The text color is different:

@mnajdova This one is fixed.

@siriwatknp siriwatknp requested review from mnajdova and removed request for danilo-leal October 17, 2022 12:18
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah 🎉

@siriwatknp siriwatknp merged commit 906b0d4 into mui:master Oct 18, 2022
Comment on lines -92 to +58
<Chip label="JavaScript" onDelete={() => {}} />
</Stack>
</ThemeProvider>
}),
]}
>
<Chip label="React" color="primary" onDelete={() => {}} />
<Chip label="Javascript" onDelete={() => {}} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a small regression on this one, fixed in 31eeb83.

@oliviertassinari
Copy link
Member

@danilo-leal you might be interested in having a look at the new page, and seeing if there are any tweaks you would like to make.

For example, some of the colors changed to be truer to how it was initially designed: https://www.figma.com/file/4uv2dT18rXJPZBbrbpw61q/Marketing-site?node-id=3224%3A11442.

Before ❌: https://634559a4d8e9450008ca12e0--material-ui-docs.netlify.app/
Screenshot 2022-10-18 at 12 53 42

After ✅: https://634e653c355ff00008613f12--material-ui-docs.netlify.app/
Screenshot 2022-10-18 at 12 53 46

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 18, 2022

@siriwatknp There is a regression on Firefox and Safari:

Screenshot 2022-10-18 at 22 13 29

It was reported as a PM https://twitter.com/messages/2927740052-3199492438. It's probably easy to fix, I didn't look.

@siriwatknp
Copy link
Member Author

siriwatknp commented Oct 19, 2022

@siriwatknp There is a regression on Firefox and Safari:

Here is the fix: #34822. I will do a hotfix deployment.

@danilo-leal
Copy link
Contributor

@oliviertassinari at this point, the website is a truer representation of how the website (at least the main pages) should look like rather than Figma 😬 We'll be actually updating Figma to match with what's in production, but thanks for heads-up!

@cpboyd
Copy link

cpboyd commented Oct 24, 2022

The home page now seems to default to light mode for me, regardless of the system settings.

If I manually use setMode('system'), it applies the expected theme but shouldn't this be the default setting? It seems that systemMode is only defined if the mode is set to 'system'. <CssVarsProvider theme={theme} defaultMode="system"> resolved my expected behavior. Is the home page supposed to default to light mode, regardless of someone's system settings?

@siriwatknp
Copy link
Member Author

You are right! I think the home page should default to system preference. Do you want to submit a PR?

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 30, 2022

I can't reproduce #33545 (comment) anymore. I assume the bug was fixed, somewhere?

@siriwatknp
Copy link
Member Author

I can't reproduce #33545 (comment) anymore. I assume the bug was fixed, somewhere?

It was fixed in #35216

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 1, 2022

@siriwatknp Great, thanks for the link 👍.

In pacocoursey/next-themes#20 (comment), I see that they changed the default mode config from light to system. This seems a bit backward. It could lead to surprises. Say if as a developer, you only added logic for the light mode. For example with Tailwind CSS https://tailwindcss.com/docs/dark-mode, dark mode only happens if you start to write logic for it. So 👍 to keep light as the default in MUI System.

feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
@paulincai
Copy link

At present, from my light mode latest version of Chrome on the mui.com I cannot turn on the dark mode of the website. It goes dark and back to light.

@mnajdova
Copy link
Member

At present, from my light mode latest version of Chrome on the mui.com I cannot turn on the dark mode of the website. It goes dark and back to light.

@paulincai please create an issue with a reproduction so that we can see what is the issue.

@paulincai
Copy link

@mnajdova as my message says, this is about the main website mui.com. However, since I dropped that comment, the website seems to have been fixed. Switching theme works ok now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
website Pages that are not documentation-related, marketing-focused.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants