-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
feat(ui): updates to dark mode logic #3740
base: master
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 9362246394Details
💛 - Coveralls |
@@ -666,6 +669,8 @@ export default { | |||
hideTopbar: false, | |||
title: '', | |||
messages: [], | |||
systemDark: false, | |||
mq_system_dark: null, |
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.
why don't use a watch for this instead?
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.
Also it's not clear to me what mq_system_dark
is used for, you never re-assign it, I would instead use a let in beforeMount instead
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.
First time using Vue. Will re look at the logic here. Thanks for the notes.
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.
It's ok no worries I appreciate a lot your help 🙏🏼
beforeUnmount() { | ||
if (this.mq_system_dark !== null) { | ||
this.mq_system_dark.removeEventListener( | ||
'change', | ||
this.updateSystemDark, | ||
) | ||
} | ||
}, |
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.
this is a bit useless as the App will only be unmounted when the page is destroyed
I don't have time right now to work on this. |
@zackbcom No worries, keep it open for now will see if I can find some time to look at it :) |
Pull Request Test Coverage Report for Build 11250226051Details
💛 - Coveralls |
This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label |
Changes
null
as system defined.TODO
Testing Artifacts