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

Reset to OS theme / colorMode should possible #8074

Open
6 of 7 tasks
juliusmarminge opened this issue Sep 9, 2022 · 19 comments · May be fixed by #8474 or #10474
Open
6 of 7 tasks

Reset to OS theme / colorMode should possible #8074

juliusmarminge opened this issue Sep 9, 2022 · 19 comments · May be fixed by #8474 or #10474
Labels
apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee bug An error in the Docusaurus core causing instability or issues with its execution domain: theme Related to the default theme components

Comments

@juliusmarminge
Copy link

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

I don't think this is a bug as much as intended behavior, but I think that intention can be confusing so I'll post an issue anyways.

Docusaurus is currently handling themes both using local storage, and syncing the user preferences. This often leads to the theme going out of sync:

function getStoredTheme() {
var theme = null;
try {
theme = localStorage.getItem('${ThemeStorageKey}');
} catch (err) {}
return theme;
}
var storedTheme = getStoredTheme();
if (storedTheme !== null) {
setDataThemeAttribute(storedTheme);

Reproducible demo

https://preferred-theme-test.vercel.app

Steps to reproduce

Lets say at day I toggle the theme on the website to use light mode. That would store a KV-pair { "theme": "light" } in localstorage. I leave the page and when I come back at night my device is now using darkmode, so I'd probably want the page to load in dark mode. However, since the KV-pair is in localstorage, that takes precedence so it loads light mode instead.

Expected behavior

I'd want the page to load in dark mode.

Actual behavior

The page loads the KV pair from localstorage and uses light mode.

Your environment

Self-service

  • I'd be willing to fix this bug myself.
@juliusmarminge juliusmarminge added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Sep 9, 2022
@juliusmarminge
Copy link
Author

I can submit a PR for this if you agree with my standpoint, but since I think this is intended I won't do anything until I hear back from you

@Josh-Cena
Copy link
Collaborator

The UX is quite hard to design. How should the localStorage value be deemed stale? Or should we not persist that value at all?

@juliusmarminge
Copy link
Author

The UX is quite hard to design. How should the localStorage value be deemed stale? Or should we not persist that value at all?

I usually don't persist it at all tbf but there is definitely a compromise to be made.

+ If you don't use localstorage, the theme is always in sync with the user's device,

- However say the user is on dark mode but want the page in light, the theme would reset to dark on every pageload.

I'd take the payoff and assume that users who want light pages probably has light mode enabled on their device

@juliusmarminge
Copy link
Author

juliusmarminge commented Sep 9, 2022

@Josh-Cena
Copy link
Collaborator

I think refreshing definitely should not change the color mode—that would feel really shaky. The only question is whether we can invalidate the localStorage somehow in the following case:

  1. Light mode is selected—either as default or as user choice
  2. User's device switched from light mode to dark mode the next time the site is opened

I think to do that we have to persist the current system theme in local storage as well?

(Also, I use system-wide dark mode, but the dark mode on some sites is just horrible that I use light mode on those instead.)

@juliusmarminge
Copy link
Author

I think refreshing definitely should not change the color mode—that would feel really shaky. The only question is whether we can invalidate the localStorage somehow in the following case:

  1. Light mode is selected—either as default or as user choice
  2. User's device switched from light mode to dark mode the next time the site is opened

I think to do that we have to persist the current system theme in local storage as well?

(Also, I use system-wide dark mode, but the dark mode on some sites is just horrible that I use light mode on those instead.)

That sounds like a good idea. We could store a blob

{"theme": "light", "userPreferred": "dark"}

and check if the mediaMatch === userPreferred in some way

@Josh-Cena Josh-Cena changed the title theme getting out of sync Color mode should be re-synced if system mode changes Sep 9, 2022
@Josh-Cena Josh-Cena added domain: theme Related to the default theme components and removed status: needs triage This issue has not been triaged by maintainers labels Sep 9, 2022
@juliusmarminge
Copy link
Author

I could play around and file a PR if you like?

@Josh-Cena
Copy link
Collaborator

Ah, sure, feel free to!

@slorber
Copy link
Collaborator

slorber commented Sep 30, 2022

Hey,

To me, once a user selects light mode, we persist it and the site should forever use that color. We definitively want to keep using the explicit choice of the user. If user wants to reset to OS setting, this choice must be explicit.

If the user wants to revert to the system theme, then there must be an explicit option to do so, like on many sites.

CF https://developer.mozilla.org/

CleanShot 2022-09-30 at 16 06 11@2x

The problem is that we only have 2 values in the current switch, and the user has no ability to reset to OS switch. I'm more likely willing to add a 3rd icon in our state machine to make it at least possible to revert to os theme (ie erase the localstorage value).

It's not a very conventional UX to have and I'm not sure we can find a good icon representing OS theme though 😅 maybe a mixture of both icons at the same time? Or maybe we'd want to use a dropdown like many other websites 🤷‍♂️ some people probably won't like it.


Not 100% related to this exact problem but In general I like the ideas expressed here:

What we want is more likely to give the ability to reset to "inherit"

CleanShot 2022-09-30 at 16 12 40@2x

@flying-sheep
Copy link

flying-sheep commented Nov 22, 2022

Some people won’t like any chosen UI, but this is a tri-state piece of data. Any UI that allows switching between all three states is preferable to the current one which makes one of them inaccessible. Working is better than broken, and only once things work, UX starts to become a topic.

I built this little thing a while ago btw: https://flying-sheep.github.io/react-color-scheme-switch/

@slorber slorber changed the title Color mode should be re-synced if system mode changes Reset to OS theme / colorMode should possible Dec 8, 2022
@slorber
Copy link
Collaborator

slorber commented Dec 8, 2022

I tried to solve this but this need a bit more refactoring than I thought.

We probably need 2 states now:

  • colorMode: the current, effective color mode, never null (we already have this one)
  • colorModeChoice: the choice of the user, including null (or "os")?

If we only have the effective colorMode in state, then it becomes impossible to know we are in a state of "os color mode", and thus transition to the appropriate next value.


Also, I think the transition state machine should take into account the effective color mode to decide on which value to use next.

IE:

  • if colorModeChoice=OS & colorMode=light => click should transition to dark
  • if colorModeChoice=OS & colorMode=light => click should transition to light

What I want to avoid here: users on the first visit are likely all having the OS setting. When they first click on the toggle, we probably want to give them feedback. Transitioning from light (OS setting) to light (user choice) may be a weird initial feedback 🤷‍♂️

The other option is to use a dropdown instead of a toggle button.

@flying-sheep
Copy link

flying-sheep commented Dec 8, 2022

That’s exactly the UX the Python community is converging on. I didn’t have time to expand on it when I wrote my last comment, but here’s basically me saying the exact same things as you just did, rust-lang/this-week-in-rust#2274 (comment), with some example implementations.

@nasso
Copy link

nasso commented Dec 21, 2022

how about keeping the same icon button, but with a third state? clicking it cycles between "light"/"dark"/"auto". when set to "auto", just store { "theme": "auto" } in localStorage.

we just need a new icon, could be just an "A" or some i18n-friendly icon like a magic wand. the icon could also just be some sort of badge on top of the 🌙/☀️ we already have, to show both the current theme and the fact that it was automatically figured out.

that would be more than enough and is better than what we have currently imho

@slorber
Copy link
Collaborator

slorber commented Dec 21, 2022

Yes we agree we need 3 states and an icon to represent the new 3rd state.

That's not really worth pursing this discussion, but rather working on a concrete implementation based on my comments here:
#8074 (comment)

@nasso
Copy link

nasso commented Dec 22, 2022

I'd like to work on that!

@nasso nasso linked a pull request Dec 22, 2022 that will close this issue
3 tasks
@ilg-ul
Copy link
Contributor

ilg-ul commented Mar 10, 2023

Any progress on adding an OS theme mode?

@Zwyx
Copy link
Contributor

Zwyx commented May 11, 2023

Hey all,

@slorber, I'd like to suggest the solution adopted by the website https://ui.shadcn.com/ (it's a UI library project being built at https://github.com/shadcn/ui):

Please play with the theme switcher, and you'll notice how the icons work:

  • the button's icon always represents the theme currently displayed: either light or dark, it has no third state;
  • however, in the menu which opens when the button is clicked, there are the three states, with a "laptop" icon for the system theme.

I find this method very intuitive, practical, easy to understand. It has three states, but it only shows the two icons that everybody understands: Sun and Moon. It doesn't require to come up with an icon for the 3rd-state, which seems to be an impossible task 😄 Even the one chosen by Mozilla is questionable. And the "laptop" icon here is great in the menu, but wouldn't be great in the button, especially because it would be the first one seen by the user when navigating to the site for the first time.

I use this method now on my new projects. I have just changed the name of the third option from "System" to "Same as device" to be more friendly to non-tech people.

(Also, this website uses next-theme under the hood — we could take inspiration from how it handles states.)

@Zwyx
Copy link
Contributor

Zwyx commented Sep 14, 2023

Just wanted to revive this. I've made a demo project showing what I suggested above, and explained the implementation in this article.

image

@slorber I'd be keen to know your opinion on this way of doing it 😉

@slorber slorber added the apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee label Sep 25, 2023
@mahozad
Copy link

mahozad commented Nov 24, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apprentice Issues that are good candidates to be handled by a Docusaurus apprentice / trainee bug An error in the Docusaurus core causing instability or issues with its execution domain: theme Related to the default theme components
Projects
None yet
8 participants