feat: sync color mode between browser tabs #6723
Conversation
|
✔️ [V2] 🔨 Explore the source changes: 0de81ae 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6216545d2a3b480008694ebf 😎 Browse the preview: https://deploy-preview-6723--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6723--docusaurus-2.netlify.app/ |
|
Size Change: +629 B (0%) Total Size: 782 kB
ℹ️ View Unchanged
|
|
this is so cool |
Josh-Cena
added
the
status: awaiting review
| console.error(err); | ||
| } | ||
| }; | ||
| window.addEventListener('storage', onChange); |
There was a problem hiding this comment.
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)
| @@ -31,6 +31,10 @@ const ToggleComponent = memo( | |||
| const [focused, setFocused] = useState(false); | |||
| const inputRef = useRef<HTMLInputElement>(null); | |||
|
|
|||
| useEffect(() => { | |||
There was a problem hiding this comment.
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
| setTheme(coerceToTheme(storedTheme)); | ||
| } | ||
| } catch (err) { | ||
| console.error(err); |
There was a problem hiding this comment.
a good candidate for reportError later, see #6747
Josh-Cena
removed
the
status: awaiting review
Motivation
This is quite handy, especially when you have several tabs open from the same docs site.
Also fixes #6733.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Related PRs