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.

Sign up for GitHub

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

feat: sync color mode between browser tabs #6723

Merged
merged 2 commits into from
Feb 23, 2022
Merged

feat: sync color mode between browser tabs #6723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9574c1
feat: sync color mode between browser tabs
lex111 Feb 19, 2022
0de81ae
Update packages/docusaurus-theme-common/src/utils/colorModeUtils.tsx
slorber Feb 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/docusaurus-theme-classic/src/theme/Toggle/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import React, {useState, useRef, memo} from 'react';
import React, {useState, useRef, useEffect, memo} from 'react';
import type {Props} from '@theme/Toggle';
import {useThemeConfig, type ColorModeConfig} from '@docusaurus/theme-common';
import useIsBrowser from '@docusaurus/useIsBrowser';
Expand All @@ -31,6 +31,10 @@ const ToggleComponent = memo(
const [focused, setFocused] = useState(false);
const inputRef = useRef<HTMLInputElement>(null);

useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I have mitigated feeling because we now have the toggle state in 2 places: localstorage + react state

but it's probably fine because localstorage can't be considered reliable in all cases (and we want the toggle to keep working on such cases)

that should be fine for now 👍 but I have some refactoring ideas for later

setChecked(defaultChecked);
}, [defaultChecked]);

return (
<div
className={clsx(styles.toggle, className, {
Expand Down
31 changes: 20 additions & 11 deletions packages/docusaurus-theme-common/src/utils/colorModeUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ type ColorModeContextValue = {
readonly setDarkTheme: () => void;
};

const ThemeStorage = createStorageSlot('theme');
const ThemeStorageKey = 'theme';
const ThemeStorage = createStorageSlot(ThemeStorageKey);

const themes = {
light: 'light',
Expand All @@ -45,7 +46,7 @@ const getInitialTheme = (defaultMode: Themes | undefined): Themes => {
};

const storeTheme = (newTheme: Themes) => {
createStorageSlot('theme').set(coerceToTheme(newTheme));
ThemeStorage.set(coerceToTheme(newTheme));
};

function useColorModeContextValue(): ColorModeContextValue {
Expand All @@ -69,17 +70,25 @@ function useColorModeContextValue(): ColorModeContextValue {

useEffect(() => {
if (disableSwitch) {
return;
return undefined;
}

try {
const storedTheme = ThemeStorage.get();
if (storedTheme !== null) {
setTheme(coerceToTheme(storedTheme));
const onChange = (e: StorageEvent) => {
if (e.key !== ThemeStorageKey) {
return;
}
} catch (err) {
console.error(err);
}
try {
const storedTheme = ThemeStorage.get();
if (storedTheme !== null) {
setTheme(coerceToTheme(storedTheme));
}
} catch (err) {
console.error(err);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a good candidate for reportError later, see #6747

}
};
window.addEventListener('storage', onChange);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's going to be a problem but it's worth double-checking if this storage event listener could throw in some cases? (like iframe + blocking cookies, see #4501)

return () => {
window.removeEventListener('storage', onChange);
};
}, [disableSwitch, setTheme]);

// PCS is coerced to light mode when printing, which causes the color mode to
Expand Down