-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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 always-false boolean condition #38665
Conversation
@@ -59,7 +59,7 @@ | |||
|
|||
window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', () => { | |||
const storedTheme = getStoredTheme() | |||
if (storedTheme !== 'light' || storedTheme !== 'dark') { | |||
if (storedTheme !== 'light' && storedTheme !== 'dark') { |
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.
!a || !b
=> !(a && b)
i.e. original was equivalent to !(storedTheme === 'light' && storedTheme === 'dark')
which would only be true for Erwin Schrodinger.
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.
Thanks for opening this PR @battlesnake
IDK why it has finally not been included in #38626 since it was the original change, confirmed by my first tests in #38626 (comment). Maybe something forgotten while mixing both solutions :)
Thanks, I'm actually quite surprised that such an issue was actually found live already by someone! I just found it by analysis while translating the example to an AngularJS directive. |
Description
In the DOCS only:
I found an always-false condition while reading an example. It's for a branch that's difficult to test and will rarely get triggered (even on a global scale), so understandably hasn't been noticed yet.
Motivation & Context
In the DOCS only:
Example for color-modes / theme switcher will likely fail to handle changes in browser setting correctly, due to always-false conditional expression.
Type of changes
Checklist
npm run lint
)Live previews
Related issues