feat: improve theme auto-selection #296
Conversation
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
js/src/Themes.js
Outdated
| } else if (document.body.dataset.vscodeThemeKind === "vscode-dark") { | ||
| // Special case for VS Code | ||
| vuetify.framework.theme.dark = true; | ||
| this.set("dark", true); | ||
| this.save_changes(); | ||
| } else if (document.body.dataset.vscodeThemeKind === "vscode-light") { | ||
| vuetify.framework.theme.dark = false; | ||
| this.set("dark", false); | ||
| this.save_changes(); |
There was a problem hiding this comment.
So I think if dark==null signals 'auto', we should only set "dark_effective", and not "dark" right?
Also, should we listen to changes in the property?
There was a problem hiding this comment.
(Assuming we can listen to attribute changes)
There was a problem hiding this comment.
Ah, you're correct about it having to be dark_effective.
There was a problem hiding this comment.
When you say
should we listen to changes in the property?
You mean dark_effective?
iisakkirotko
force-pushed
the
02-08-feat_improve_theme_auto-selection
branch
from
c964eef
to
3610418
Compare
There was a problem hiding this comment.
I think the code can be simpler with my suggestion and we still need to listen to changes in vuetify.framework.theme.dark.
In the template we did sth like:
watch { 'vuetify.framework.theme.dark': (value) { ..} }
I'm not sure how we can do it here, maybe @mariobuikhuizen knows?
iisakkirotko
force-pushed
the
02-08-feat_improve_theme_auto-selection
branch
from
3610418
to
f803ab9
Compare
| vuetifyThemeChange() { | ||
| if ( | ||
| this.get("dark") !== null && | ||
| this.get("dark") !== vuetify.framework.theme.dark | ||
| ) { | ||
| this.set("dark", vuetify.framework.theme.dark); | ||
| this.set("dark_effective", vuetify.framework.theme.dark); | ||
| this.save_changes(); | ||
| } |
There was a problem hiding this comment.
| vuetifyThemeChange() { | |
| if ( | |
| this.get("dark") !== null && | |
| this.get("dark") !== vuetify.framework.theme.dark | |
| ) { | |
| this.set("dark", vuetify.framework.theme.dark); | |
| this.set("dark_effective", vuetify.framework.theme.dark); | |
| this.save_changes(); | |
| } | |
| vuetifyThemeChange() { | |
| if ( | |
| this.get("dark") !== null && | |
| this.get("dark") !== vuetify.framework.theme.dark | |
| ) { | |
| this.set("dark", vuetify.framework.theme.dark); | |
| } | |
| this.set("dark_effective", vuetify.framework.theme.dark); | |
| this.save_changes(); |
I think we always want to set dark_effective right?
|
I think we can also spend a sentence on this feature in https://ipyvuetify.readthedocs.io/en/latest/usage.html#themes |
|
Original dark_jlab feature was added in #196 but was not yet released, so we don't have a breaking change. |
No description provided.