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

[theme] Should we keep the light and dark palette color values? #21371

Closed
2 tasks done
dipunj opened this issue Jun 8, 2020 · 6 comments
Closed
2 tasks done

[theme] Should we keep the light and dark palette color values? #21371

dipunj opened this issue Jun 8, 2020 · 6 comments
Labels
breaking change design: material This is about Material Design, please involve a visual or UX designer in the process discussion out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future)

Comments

@dipunj
Copy link

dipunj commented Jun 8, 2020

I have been stuck on this issue for quite a long, to ask for help here. I could not find clearly what is the purpose of dark and light shades in theme.pallete.primary

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

Current Behavior 😯

When I switch to dark mode, typography component with color textPrimary changes from black to white, but typography component with color primary doesn't change from primary.light to primary.dark

Expected Behavior 🤔

I am expecting it too use the dark shade of primary when the type of theme is dark, and light shade of primary when theme is light.

Steps to Reproduce 🕹

Here is a sandbox:
https://codesandbox.io/s/cocky-dust-exk0c?file=/src/Demo.js

Context 🔦

I am using material-ui to build a page which has a toggle dark theme button, which changes the type entry in my theme object from 'light' to 'dark', and vice versa. This is my first project using material-ui, and I am not very well initiated about it. Did try my best to find it on SO.

Your Environment 🌎

Tech Version
Material-UI v4.10.1
React 16.13.1
Browser mozilla firefox developer edition
@dipunj dipunj added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 8, 2020
@joshwooding joshwooding added discussion and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 8, 2020
@oliviertassinari
Copy link
Member

The current design decision is to approach the dark and light mode as two different, isolated, themes. There are no valued that are used inside it depending on the mode. This excludes what you are proposing. I think that it's much simpler this way.

Now, I have seen a problem that might what us to think twice about it. Do we want to support the dark theme CSS media query for the first paint (when doing server side rendering)? As we are speaking, the current infrastructure lead to a flash of light theme. It's not ideal. Maybe it would be about allowing the usage of CSS variables as theme values cc @mnajdova

@dipunj
Copy link
Author

dipunj commented Jun 9, 2020

@oliviertassinari thank you for the clarification. Actually I am using next.js for ssr. And yes the flash is a problem. The only hacky workaround I found was to render a display none div untill I obtained the user preference for dark/light theme from local storage. A media query for the first paint would be awesome.

Can we please have the dark and light design decision verbosely stated in the docs as well. First time users, would find it helpful to understand what purpose the dark and light shades serve. Since, intuitively it seems that light and dark are meant to be used by when type is light and type is dark respectively, which is not the case.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 9, 2020

the flash is a problem

@dipunj We track this issue in #15588.

design decision verbosely stated in the docs as well

Oh sure, this is not the first time we have this feedback. I think that we can reopen, to see if we want to keep these variants in v5. I can no longer find them in the specification: https://material.io/design/color/the-color-system.html and I have always felt them strange, at least, from the name we give them.

@oliviertassinari oliviertassinari added this to the v5 milestone Jun 9, 2020
@oliviertassinari oliviertassinari changed the title Colors not changing from light to dark shade when theme type changes from light to dark [theme] Should we keep the light and dark palette color values? Jun 9, 2020
@joshwooding
Copy link
Member

I feel like moving closer to the spec is always a good thing so removal of these is fine by me. I also want to have a look at the new density model in the spec but that’s another topic :)

@joshwooding joshwooding mentioned this issue Jun 26, 2020
2 tasks
@oliviertassinari oliviertassinari added the design: material This is about Material Design, please involve a visual or UX designer in the process label May 9, 2021
@oliviertassinari oliviertassinari modified the milestones: v5-beta, v5.1 Jun 15, 2021
@oliviertassinari oliviertassinari removed this from the v5.1 milestone Nov 10, 2021
@siriwatknp
Copy link
Member

I don't think we will make any changes to the theme structure in v6 and v7, so I'm closing this. It's hard to say for v8 but I think it will be totally different from this issue.

@siriwatknp siriwatknp added the out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) label Dec 12, 2024
Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@dipunj How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change design: material This is about Material Design, please involve a visual or UX designer in the process discussion out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future)
Projects
None yet
Development

No branches or pull requests

4 participants