-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
Update Dark.js #17670
base: dev
Are you sure you want to change the base?
Update Dark.js #17670
Conversation
this helps to know if the device is in dark mode or not
UI Tests Results 1 files 98 suites 4s ⏱️ For more details on these failures, see this check. Results for commit 2deb986. |
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.
Hi,
Thank you for contributing!
Please take a look at my comments. Would help with it, but I have no bandwidth for it at the moment.
You can test with "pnpm dev:ssr" and "pnpm dev" in /ui.
When you're ready, a "pnpm build" without errors would ensure it passes the shallow validation for JSON also.
Plugin.mode = val | ||
Plugin.isEnabled = matchMedia.matches |
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.
isPreferred
would be better otherwise it is confusing
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.
ok i will update this
@@ -42,15 +45,18 @@ const Plugin = createReactivePlugin({ | |||
|
|||
if (__QUASAR_SSR_SERVER__) { | |||
this.isActive = dark === true | |||
this.isEnabled = matchMedia.matches |
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.
There is no matchMedia here, nor can it be. This code runs on the server. It should be defaulted to a value (probably false
) and updated only on client takeover, otherwise there will be hydration errors.
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.
noted
@@ -1,6 +1,7 @@ | |||
import { createReactivePlugin } from '../../utils/private.create/create.js' | |||
|
|||
const Plugin = createReactivePlugin({ | |||
isEnabled: false, |
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.
Should be added to Dark.json too.
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 did that on but i will update it to isPreferred
this helps to know if the device is in dark mode or not
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
The PR fulfills these requirements:
dev
branch (orv[X]
branch)fix: #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: