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

[system] Support for prefers-color-scheme with no flashes #15588

Closed
eluchsinger opened this issue May 4, 2019 · 29 comments
Closed

[system] Support for prefers-color-scheme with no flashes #15588

eluchsinger opened this issue May 4, 2019 · 29 comments
Labels
new feature New feature or request package: system Specific to @mui/system

Comments

@eluchsinger
Copy link
Contributor

eluchsinger commented May 4, 2019

Is there a way to support prefers-color-scheme in the future? I guess, it would be possible for every project to make a custom implementation to support it, but maybe it could be a good idea to add support for it by default.

The CSS media feature lets you use a media query to adapt your styling to the light or dark theme requested by the system. If the user's browser is using dark theme, the application could adapt to it and use dark theme as well, instead of showing any eye-hurting white.

It can also be seen on the documentation:

  • macOS 11.6, w/ System Preferences > General > Appearance > Dark
  • Chrome 93.0.4577.82
  • Visit https://mui.com
  • Page initially loads in light theme, but switches to dark
  • Further navigation stays in dark
  • Issue can be repeated by doing a force page reload

Expected that the page wouldn't switch appearance mode/flash. Figured it was best to describe this FYI issue here?

Benchamark

Related

@oliviertassinari oliviertassinari added the new feature New feature or request label May 4, 2019
@eps1lon
Copy link
Member

eps1lon commented Jun 28, 2019

We currently use this media query in our docs to switch to a dark theme on the client. Since we prerender them we always deliver the version with the light theme that gets hydrated with the preferred color scheme causing a flash of outdated styles if a dark theme is preferred.

As far as I know the only solution would be to use the CSS media query in our core components which would use the colors in the dark theme. This would add a lot of styles to the core for a query that isn't supported by most of our supported platforms or even some operating systems.

I think it's better to write a custom theme that adds the query to all the necessary classes concerning color (which is probably a lot). I guess this is one of those instances where CSS variables would be the best solution (if they can read media queries).

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 6, 2019

Notice the introduction of new section in the documentation: https://material-ui.com/customization/palette/#user-preference

import React from 'react';
import useMediaQuery from '@material-ui/core/useMediaQuery';
import { ThemeProvider } from '@material-ui/core/styles';

function App() {
  const prefersDarkMode = useMediaQuery('(prefers-color-scheme: dark)');

  const theme = React.useMemo(
    () =>
      createMuiTheme({
        palette: {
          type: prefersDarkMode ? 'dark' : 'light',
        },
      }),
    [prefersDarkMode],
  );

  return (
    <ThemeProvider theme={theme}>
      <Routes />
    </ThemeProvider>
  );
}

This topic is close to the discussion we have in #16367.

@elyobo
Copy link

elyobo commented Dec 20, 2019

We have the odd situation where useMediaQuery always fails during the first render, causing an incorrect flash of unstyled content in some cases. The following ugly workaround works, because it should not be the case that both media queries fail.

   const prefersDarkMode = useMediaQuery('(prefers-color-scheme: dark)')
   const theme = useTheme(prefersDarkMode)
 
   // Refuse to render until media queries align
   const prefersLightMode = useMediaQuery('(prefers-color-scheme: light)')
   if (prefersDarkMode !== !prefersLightMode) return null

The first render both fail, so it refuses to render anything, then it soon corrects and renders a logically consistent result (only one of prefers light or prefers dark) and we get no incorrectly styled flash.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 20, 2019

@elyobo Your workaround is equivalent to the usage of the NoSsr component, right?

@elyobo
Copy link

elyobo commented Dec 20, 2019

I have no idea, haven't looked at any of the SSR stuff because we're not doing SSR, the issue is happening for us client side. First render both are false, then very soon after it renders in a sane way (can't prefer both light and dark).

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 20, 2019

Super interesting, thanks for sharing, I think that we should run the update on the layout effect. +1 for a PR.

@elyobo
Copy link

elyobo commented Dec 20, 2019

While looking at the code to understand what you were suggesting, I noticed this

  const {                                                                       
    defaultMatches = false,                                                     
    matchMedia = supportMatchMedia ? window.matchMedia : null,                  
    noSsr = false,                                                              
    ssrMatchMedia = null,                                                       
  } = {                                                                         
    ...props,                                                                   
    ...options,                                                                 
  };                                                                            

  const [match, setMatch] = React.useState(() => {                              
    if (noSsr && supportMatchMedia) {                                           
      return matchMedia(query).matches;                                         
    }                                                                           
    if (ssrMatchMedia) {                                                        
      return ssrMatchMedia(query).matches;                                      
    }                                                                           
                                                                                
    // Once the component is mounted, we rely on the                            
    // event listeners to return the correct matches value.                     
    return defaultMatches;                                                      
  });                                                                           

Our problem can be solved by passing in noSsr: true to useMediaQuery (doesn't seem to be documented), because we're not using SSR, and without passing through that flag the default is defaultMatches (which itself defaults to false). This matches what we're seeing (everything is false at first).

Perhaps the PR needed is a documentation one?

If you were thinking of using useLayoutEffect instead of useEffect it doesn't look like that would solve our problem, which is just that it's using the default value initially rather than actually running the check.

@oliviertassinari
Copy link
Member

causing an incorrect flash of unstyled content in some cases

@elyobo I had this problem in mind, I would expect useLayoutEffect to solve the problem. If you have a reproduction, it would help to investigate.

@elyobo
Copy link

elyobo commented Dec 20, 2019

@oliviertassinari our problem is that unless you pass noSsr it uses the defaultMatches value as the initial value of the state. useLayoutEffect might help, but it's not necessary for us if we can just get the match to work correctly on the first run instead, and you'd still be causing an unnecessary render I think (it would still generate the theme twice, just that the second one would happen earlier than currently, right?).

@oliviertassinari
Copy link
Member

@elyobo Ok, so we might not be able to do anything about it, noSsr seems to be the way.

@elyobo
Copy link

elyobo commented Dec 20, 2019 via email

@oliviertassinari
Copy link
Member

Sure, it would help

@HiranmayaGundu
Copy link

Hey! I just faced this issue and I had come across this blog post. The approach suggested here seems to be to use CSS Custom Properties along with script to detect whether the user has a color preference on initial load.

Theme-UI seems to do this and also provides a useColorMode hook that allows a user to toggle all the color modes which remembers preference using local storage. Material-UI could take a similar approach?

I would not mind working on this if you guys think it is a valid approach. It would be my first open source issue though so any help would be appreciated!😅

@bugzpodder
Copy link
Contributor

@HiranmayaGundu have you tried using theme-ui with material-ui? I've been trying to find examples but haven't found one.

@HiranmayaGundu
Copy link

HiranmayaGundu commented May 12, 2020

@bugzpodder No I have not tried that out, and I'm not entirely sure that is possible? Material-UI uses it's own theme specification. You could use createMuiTheme() to make a custom theme

@jonahsnider
Copy link

jonahsnider commented May 22, 2020

I'm using Next.js and I've wrapped my <App> component with the <ThemeProvider>, which should be preserving the theme between pages, but I'm still noticing a flash of light -> dark between pages.

Am I or Next.js screwing something up, or is there still not a concrete solution to this flash of unthemed changes?

Another note: Using the CSS media query would be a big pain to write, but would fix this issue, as well as using dynamic themes with AMP. This article is a good read on how a Gatsby site had a dark theme implemented using CSS variables in order to prevent any FoUC.

@oliviertassinari
Copy link
Member

@pizzafox The flash comes from the reliance on JavaScript to detect the media query. To solve the flash, there are a couple of options:

  1. Duplicate the generated CSS one for light and another one for dark, prefixed with the media query
  2. Hide the screen until the rendering of the JavaScript media query
  3. Store the preferred mode of the user into a cookie, dynamically serve this request.
  4. Move all the colors that are impacted by the dark/light mode into CSS variables, toggle the variables at the root with a media query. Used by StackOverflow.

@HiranmayaGundu
Copy link

@oliviertassinari Is it possible to allow strings like var(--color-red) in the MUI Theme? That would allow me to use CSS variables just for the palette.

@oliviertassinari
Copy link
Member

@HiranmayaGundu We have a running discussion about it in #12827.

@krissh-the-dev

This comment has been minimized.

@fosteman
Copy link

fosteman commented Jan 5, 2021

@elyobo, this is very interesting, did you find that there was an overlap of CSS style sheets on your elements ?

Screen Shot 2021-01-03 at 9 57 02 AM

Screen Shot 2021-01-03 at 9 56 50 AM

@elyobo
Copy link

elyobo commented Jan 5, 2021

I don't recall sorry @fosteman; the noSsr solution solved everything for us, as we weren't doing server side rendering, and it seems like it's probably the right approach for anyone that isn't doing server side rendering.

We set the noSsr option when we create the theme (as documented at the bottom of the section here) so that it's the default for all media queries.

@Temez1
Copy link

Temez1 commented Jan 28, 2021

Thanks for the noSsr solution.

This still seems a weird behavior and at first, I thought it didn't work at all. The query gives first false, then true but it didn't render the page with dark mode (idk why). It now works, but I assume that it should work by default noSSr true and if you are using SSR you could enable it as for SPA it's a significant overhead (renders twice, or in my case idk did it actually😅). But I guess someone smarter decided it to be the other way around (no sarcasm).

@TheBit
Copy link

TheBit commented Aug 1, 2022

What about Sec-CH-Prefers-Color-Scheme client hint header? https://web.dev/user-preference-media-features-headers/ ?

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 2, 2022

@TheBit Developers can already implement a solution relying on Sec-CH-Prefers-Color-Scheme but it forces to have a server handling each request, or to have a smart cache that can use it. The goal of this issue, I believe, is to remove this constraint: make it work when hosting on a static CDN.

@siriwatknp
Copy link
Member

I believe that we can close this issue with the experimental API CssVarsProvider.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 22, 2022

instead of showing any eye-hurting white.

@siriwatknp We could close this issue, it sounds like the root of the pain point is solved, proof: https://twitter.com/MUI_hq/status/1582390058099015685.

If we close, I think that we should create a new migration issue to have all the pages of https://mui.com/ no longer flash like the homepage, e.g. the docs pages.

@siriwatknp
Copy link
Member

instead of showing any eye-hurting white.

@siriwatknp We could close this issue, it sounds like the root of the pain point is solved, proof: https://twitter.com/MUI_hq/status/1582390058099015685.

If we close, I think that we should create a new migration issue to have all the pages of https://mui.com/ no longer flash like the homepage, e.g. the docs pages.

Yep, sounds good. We can get some help from the community to migrate the rest of the pages.

@oliviertassinari
Copy link
Member

The new issue in question: #34880.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request package: system Specific to @mui/system
Projects
None yet
Development

No branches or pull requests