feat(theme): allow reverting to auto theme mode #8474

nasso · 2022-12-22T05:26:00Z

Pre-flight checklist

I have read the Contributing Guidelines on pull requests.
If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
If this is a new API or substantial change: the PR has an accompanying issue (closes Reset to OS theme / colorMode should possible #8074) and the maintainers have approved on my working plan.

Motivation

When the user changes the theme manually by clicking on the toggle, the user's preference gets persisted in localStorage, and there's no way to go back to the system/OS/browser theme (which I just call "auto"; browser is free to report anything it wants, not necessarily what it or the OS is using). I added a third state to the existing toggle, that allows the user to set the theme to "automatic" mode, which will honour prefers-color-scheme. The preference of the user is still persisted to localStorage, except now we sometimes store theme=auto. I created a rudimentary icon in Figma, inspired by MDN's icon but in the style of the rest of the icons in Docusaurus' classic theme (i.e. stroke width, margins).

As described in #8074 (comment), a colorModeChoice was introduced to support the third value: auto. colorMode is now computed from colorModeChoice, the source of truth. I also followed the same logic for the state machine cycling logic.

On the DOM side of things (what actually changes the theme), the data-theme attribute still contains either dark or light (I thought about allowing it to be auto and using CSS media queries but that's too much work right now, so probably out of scope). To support the third state in the theme toggle button, I added a data-theme-choice attribute, that may also be auto. The logic otherwise stays the same.

Finally, the current configuration options include respectPrefersColorScheme, which is used to determine whether light or dark should be used when the user has not manually changed the theme. I deprecated respectPrefersColorScheme in favour of a third possible value for defaultMode: auto. This makes the logic much easier to follow: light will always be light, dark will always be dark, and auto will always use whatever is the preferred colour scheme.

Test Plan

I think the only way to test it would be with E2E tests? I did not add any because I couldn't find any test for the previous behaviour (i.e. clicking the toggle changes the data-theme attribute).

prefers-color-scheme: dark

prefers-color-scheme: light

Test links

Deploy preview: https://deploy-preview-8474--docusaurus-2.netlify.app/

Related issues/PRs

Fixes #8074.

nasso · 2022-12-22T06:10:41Z

I'm getting errors like:

[ERROR] Docusaurus server-side rendering could not render static page with path /.
[INFO] It looks like you are using code that should run on the client-side only.
To get around it, try using `<BrowserOnly>` (https://docusaurus.io/docs/docusaurus-core/#browseronly) or `ExecutionEnvironment` (https://docusaurus.io/docs/docusaurus-core/#executionenvironment).
It might also require to wrap your client code in `useEffect` hook and/or import a third-party library dynamically (if any).

...

ReferenceError: window is not defined
ReferenceError: window is not defined
ReferenceError: window is not defined
ReferenceError: window is not defined
...
ReferenceError: window is not defined
ReferenceError: window is not defined
[ERROR] Unable to build website for locale en.
[ERROR] Error: Failed to compile with errors.
    at /workspaces/docusaurus/packages/docusaurus/lib/webpack/utils.js:180:24
    at /workspaces/docusaurus/node_modules/webpack/lib/MultiCompiler.js:554:14
    at processQueueWorker (/workspaces/docusaurus/node_modules/webpack/lib/MultiCompiler.js:491:6)
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11)

I don't really understand why nor how I can debug this. Any hint?

slorber

Thanks

It doesn't build so it requires at least a few changes 🤪

Will do a better review of the rest when I have time

slorber · 2023-01-05T17:46:09Z

packages/docusaurus-theme-common/src/contexts/colorMode.tsx

+        return ColorModes.dark;
+      case ColorModeChoices.auto:
+      default:
+        return window.matchMedia('(prefers-color-scheme: dark)').matches


it's not possible to access window when rendering React components (and useMemo is executed when rendering).

When doing SSR/SSG, window does not exist and it's impossible to query for prefers color scheme because we are not inside a browser.

The error maybe doesn't explain this well, but with experience we figure out the problem quite fast:

6:29:39 AM: [ERROR] Docusaurus server-side rendering could not render static page with path /docs/2.1.0/creating-pages/. 6:29:39 AM: [INFO] It looks like you are using code that should run on the client-side only. 6:29:39 AM: To get around it, try using `<BrowserOnly>` (https://docusaurus.io/docs/docusaurus-core/#browseronly) or `ExecutionEnvironment` (https://docusaurus.io/docs/docusaurus-core/#executionenvironment). 6:29:39 AM: It might also require to wrap your client code in `useEffect` hook and/or import a third-party library dynamically (if any). 6:30:15 AM: ReferenceError: window is not defined

oooh that makes a lot of sense yeah

so what's the plan here, since we cannot know in advance what theme to render with from the server-side? do i just assume light when window isn't available or is there something smarter to do?

@nasso We use defaultTheme, which always statically exists.

the thing is, defaultMode now includes auto, but here we need to figure out the effective theme (light or dark)

It's not possible to figure out the effective theme during the SSR render phase and unfortunately will never be possible. There's no theme when running in Node.js. That's why we use inlined JS and the data-theme attribute to set the effective theme as soon as we can to avoid a "theme flash". Doing this in render/useMemo is both too late and not possible. Doing this in useEffect is possible but also too late to avoid the flash.

I think there's already something we do in existing code that is even not compatible with React 18. The server render must match exactly the first client render (hydration). And we can't get the effective theme on the server, so both the server/first-client render should use the exact same value (the default theme, not the effective one).

I mean: this existing code is likely wrong because it can lead to different values for the server render and the first client render:

const getInitialColorMode = ( defaultMode: ColorModeChoice | undefined, ): ColorModeChoice => coerceToColorMode( ExecutionEnvironment.canUseDOM ? document.documentElement.getAttribute('data-theme-choice') ?? document.documentElement.getAttribute('data-theme') : defaultMode, );

It should simply be instead: useState(defaultMode)

The effective theme state can only be set later, after hydration, at best in a useLayoutEffect

i think one way we could get rid of the FOUC altogether would be to handle auto as a possible value for the data-theme attribute, and use @media queries in CSS to set the theme instead of dynamically querying the preference using JS. CSS media queries would only apply on [data-theme=auto]. this would probably require modifying a lot of CSS though.

That's a possibility, but I think it will duplicate CSS, and also be annoying for users that already have custom dark/light mode elements.

We/they would now have to use more complex selectors everywhere:

@media (prefers-color-scheme: dark) [data-theme=auto] .subselector, [data-theme=dark] .subselector { ... }

@media (prefers-color-scheme: light) [data-theme=auto] .subselector, [data-theme=light] .subselector { ... }

So I don't think it would be a great move, our current theming CSS DX is better:

[data-theme=dark] .subselector { ... }

[data-theme=light] .subselector { ... }

that is definitely much better.

And we can't get the effective theme on the server, so both the server/first-client render should use the exact same value (the default theme, not the effective one).

does that mean there would be no way to set the default theme to the system preference? because i don't see what the server would render when the default is "auto"

netlify · 2023-01-05T21:20:19Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`51176f1`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/63ed84fd34c1830008306f7f
😎 Deploy Preview	https://deploy-preview-8474--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2023-01-05T21:20:56Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟠 87	🟢 97	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 82	🟢 100	🟢 100	🟢 100	🟢 90	Report

nasso · 2023-01-05T21:22:35Z

woops didnt know github would remove the other review requests 🤔

nasso requested review from slorber, lex111 and Josh-Cena as code owners December 22, 2022 05:26

facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 22, 2022

slorber requested changes Jan 5, 2023

View reviewed changes

slorber marked this pull request as draft January 5, 2023 17:47

nasso added 2 commits January 5, 2023 21:10

feat(theme): allow reverting to auto theme mode

0ec235b

docs: update configuration options

2daab21

nasso force-pushed the main branch from 0f4d7af to 2daab21 Compare January 5, 2023 21:15

nasso marked this pull request as ready for review January 5, 2023 21:21

nasso requested review from slorber and removed request for lex111 and Josh-Cena January 5, 2023 21:21

nasso and others added 2 commits January 28, 2023 20:28

chore: update translations

3a5208a

Merge branch 'facebook:main' into main

51176f1

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(theme): allow reverting to auto theme mode #8474

feat(theme): allow reverting to auto theme mode #8474

nasso commented Dec 22, 2022 •

edited

nasso commented Dec 22, 2022

slorber left a comment

slorber Jan 5, 2023

nasso Jan 5, 2023

Josh-Cena Jan 5, 2023 •

edited

nasso Jan 5, 2023

slorber Jan 6, 2023 •

edited

nasso Jan 6, 2023

slorber Jan 6, 2023

nasso Jan 6, 2023

netlify bot commented Jan 5, 2023 •

edited

github-actions bot commented Jan 5, 2023 •

edited

nasso commented Jan 5, 2023

feat(theme): allow reverting to auto theme mode #8474

Are you sure you want to change the base?

feat(theme): allow reverting to auto theme mode #8474

Conversation

nasso commented Dec 22, 2022 • edited

Pre-flight checklist

Motivation

Test Plan

Test links

Related issues/PRs

nasso commented Dec 22, 2022

slorber left a comment

Choose a reason for hiding this comment

slorber Jan 5, 2023

Choose a reason for hiding this comment

nasso Jan 5, 2023

Choose a reason for hiding this comment

Josh-Cena Jan 5, 2023 • edited

Choose a reason for hiding this comment

nasso Jan 5, 2023

Choose a reason for hiding this comment

slorber Jan 6, 2023 • edited

Choose a reason for hiding this comment

nasso Jan 6, 2023

Choose a reason for hiding this comment

slorber Jan 6, 2023

Choose a reason for hiding this comment

nasso Jan 6, 2023

Choose a reason for hiding this comment

netlify bot commented Jan 5, 2023 • edited

✅ [V2]

github-actions bot commented Jan 5, 2023 • edited

⚡️ Lighthouse report for the deploy preview of this PR

nasso commented Jan 5, 2023

nasso commented Dec 22, 2022 •

edited

Josh-Cena Jan 5, 2023 •

edited

slorber Jan 6, 2023 •

edited

netlify bot commented Jan 5, 2023 •

edited

github-actions bot commented Jan 5, 2023 •

edited