mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.83k stars 32.26k forks source link

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

Closed eluchsinger closed 2 years ago

eluchsinger commented 5 years ago

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:

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

Benchamark

Related

eps1lon commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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

elyobo commented 4 years ago

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 commented 4 years ago

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

elyobo commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

elyobo commented 4 years ago

Ok if I make a PR to extend the docs for some of those options?

On Sat, 21 Dec 2019, 08:41 Olivier Tassinari, notifications@github.com wrote:

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

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mui-org/material-ui/issues/15588?email_source=notifications&email_token=AADDR7QBMJ5J7FAXPF4YQYLQZU3Y5A5CNFSM4HKZBV6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHOH3GI#issuecomment-568098201, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADDR7RHJPOR3RGHGSP4OELQZU3Y5ANCNFSM4HKZBV6A .

oliviertassinari commented 4 years ago

Sure, it would help

HiranmayaGundu commented 4 years ago

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 commented 4 years ago

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

HiranmayaGundu commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

krissh-the-dev commented 4 years ago

We can do something like:


// checking if the media query is true
const colorScheme = window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light'; 
const theme = createMuiTheme({ palette: {  type: colorScheme  } });

to achieve dark theme based on browser/system settings.

fosteman commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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

oliviertassinari commented 2 years ago

@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 commented 2 years ago

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

oliviertassinari commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

The new issue in question: #34880.