-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
fix(v2): switch a toggle when system theme changed #2325
Conversation
Deploy preview for docusaurus-2 ready! Built with commit 154c350 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I'm wondering why I'm not able to reproduce the original bug. The toggle switches well for me when I change my system theme.
@@ -61,6 +61,14 @@ const useTheme = () => { | |||
} | |||
}, [setTheme]); | |||
|
|||
useEffect(() => { | |||
window |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to check for disableDarkMode
here, otherwise this runs on sites which still disable dark mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Motivation
Resolve #2295.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Go to preview and in the settings of your OS change the system theme to the opposite (dark -> light and vice versa). Make sure the theme and the toggle state in navbar change appropriately on the preview website.
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)