Material-ui: Unchanged colours on theme switching (dark/light)

Created on 14 Dec 2019  路  12Comments  路  Source: mui-org/material-ui


The palette colours (primary and secondary colours) remain unchanged over dynamically switching from dark/light themes. This is behaviour can clearly be observed with <Typography> objects, this forces developers to programmatically check if the theme is in dark or light mode and to accordingly adapt the colours adding a lot of overhead.

  • [X] The issue is present in the latest release.
  • [X] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

Expected Behavior 馃

I defined my primary colour to red (import red from '@material-ui/core/colors/red';). On a dark mode in the colour code is #f44336 and on a light mode the colour code is #f44336. See the following screenshots:

Dark mode:
image

Light mode:
image


The theme should automatically switch colours according to the selected mode, in dark mode a lighter variant (e.g. red[300]) of colours should be picked whereas in light mode darker variant of the palette colour should be picked (e.g. red[700]) to increase the contrast and improve the overall design.

Steps to Reproduce 馃暪

Steps:

  1. Set a colour palette, e.g. primary colour to red and secondary colour to yellow.
  2. Display a text using the <Typography> component with the color:'primary' propriety set.
  3. Open Google Chrome/Firefox developer tool and compare the hex colour code of the generated text on dark and light mode to notice that it is unchanged.

Context 馃敠


I am trying to maximise the contrast of my text elements and improve the overall aesthetic of my website.

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.7.2 |
| React | v16.12.0 |
| Browser | Google Chrome v78.0.3904.108 |
| TypeScript | No |
| etc. | |

support

Most helpful comment

@aress31 I faced the same issue after is stopped using the jss provider. In short, rather than passing the theme you generated once for ThemeProvider, you have to generate it each time you change it so it affects all styles. In code:

const themeA =createMuiTheme(...);  --> const geThemeA =()=>createMuiTheme(...);
const themeB =createMuiTheme(...); --> const getThemeB =()=>createMuiTheme(...);
...
switchTheme=(...)...setState{theme:a?themeA:themeB}; --> switchTheme(...)...setState{theme:a?geThemeA():geThemeB()};
...
<ThemeProvider theme={theme}>

This works for us. The createMuiTheme needs to be instantiated every time a change is made and this also shouldn't be an issue performance wise. Can this be documented or are there any plans on fixing this?

All 12 comments

馃憢 Thanks for using Material-UI!

We use GitHub issues exclusively as a bug and feature requests tracker, however,
this issue appears to be a support request.

For support, please check out https://material-ui.com/getting-started/support/. Thanks!

If you have a question on StackOverflow, you are welcome to link to it here, it might help others.
If your issue is subsequently confirmed as a bug, and the report follows the issue template, it can be reopened.

It's somewhat related to #18776.

Yes, it is indeed kind of related, I believe that an effort should be made to ensure that the colour property of all components get updated on theme switching. In the case that I documented in this ticket, the bug affects <Typography>, #18776 relates to <Link> but this problem could also very well affect ` and other components.

The fix is quite simple, just switch the colour property of non-surface objects to (primary/secondary).light on a dark theme and (primary/secondary).dark on a light theme this is what I have been programmatically doing on the previous website I designed with Material UI and having this issue fixed on the library level would be awesome!

@aress31 Without a reproduction, I'm not sure to understand the problem you are facing. As far as I know, it's a documentation/teaching issue.

Alright, let me try to rephrase the bug/issue.

The color propriety of ALL components should automatically get updated when proving a new theme to MuiThemeProvider, i.e. on theme switching (dark/light). In such a way that if I set a Foobar with the following palette:

palette: {
      type: isDarkMode ? 'dark' : 'light',
      primary: red,
      secondary: grey,
      // Used by `getContrastText()` to maximize the contrast between the background and
      // the text.
      contrastThreshold: 4,
      // Used to shift a color's luminance by approximately
      // two indexes within its tonal palette.
      // E.g., shift from Red 500 to Red 300 or Red 700.
      tonalOffset: 0.4,
      background: {
        default: isDarkMode ? '#121212' : '#FFFFFF',
      },
    },

The colour of the <Typography> component (see the above) should be set to palette.primary.light when isDarkMode === true and to palette.primary.dark when isDarkMode === false which is currently not the case.

It wasn't designed to behave this way, you need to change the colors.

Ok @oliviertassinari understood, but maybe this should be considered as it helps boosting up the contrast between foreground and background, or at least maybe the option to have the colour palette behave this way should be implemented?

@aress31 I believe it's the responsibility of the developers when providing a custom palette to make sure the contrast is good enough.

@oliviertassinari I totally agree but the problem here is that only the main colour is being used in <Typography> regardless of the theme type (light and dark).

@oliviertassinari, I think that my issue was not entirely understood.

Material Design states that on a dark theme, unsaturated colours should be used in order to improve the contrast with the background and limit eye strains as opposed to a light theme where saturated colours are used.

Right now MuiTheme takes a main, a light and a dark colour variant, however, elements such as <Typography> only stick to the main colour variant regardless of the theme value. This seems to be a bug to me, as if following the Material Design doc, on a dark theme the light colour variant should automatically be applied to all display elements.

I hope that I made my ticket clearer. 馃榿

@aress31 I faced the same issue after is stopped using the jss provider. In short, rather than passing the theme you generated once for ThemeProvider, you have to generate it each time you change it so it affects all styles. In code:

const themeA =createMuiTheme(...);  --> const geThemeA =()=>createMuiTheme(...);
const themeB =createMuiTheme(...); --> const getThemeB =()=>createMuiTheme(...);
...
switchTheme=(...)...setState{theme:a?themeA:themeB}; --> switchTheme(...)...setState{theme:a?geThemeA():geThemeB()};
...
<ThemeProvider theme={theme}>

@aress31 I faced the same issue after is stopped using the jss provider. In short, rather than passing the theme you generated once for ThemeProvider, you have to generate it each time you change it so it affects all styles. In code:

const themeA =createMuiTheme(...);  --> const geThemeA =()=>createMuiTheme(...);
const themeB =createMuiTheme(...); --> const getThemeB =()=>createMuiTheme(...);
...
switchTheme=(...)...setState{theme:a?themeA:themeB}; --> switchTheme(...)...setState{theme:a?geThemeA():geThemeB()};
...
<ThemeProvider theme={theme}>

This works for us. The createMuiTheme needs to be instantiated every time a change is made and this also shouldn't be an issue performance wise. Can this be documented or are there any plans on fixing this?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

revskill10 picture revskill10  路  3Comments

pola88 picture pola88  路  3Comments

ryanflorence picture ryanflorence  路  3Comments

reflog picture reflog  路  3Comments

activatedgeek picture activatedgeek  路  3Comments