Consume more tokens from arches theme, add dark mode toggle #91
Conversation
jacobtylerwalls
changed the title
|
looks like some webpack failures in the CI? |
|
That's expected, since this depends on themes/default.ts in archesproject/arches#11319. I could update this branch to depend on it, but that seems like avoidable churn? |
jacobtylerwalls
force-pushed
the
jtw/consume-tokens
branch
from
4afd684
to
7d99575
Compare
jacobtylerwalls
force-pushed
the
jtw/consume-tokens
branch
from
7d99575
to
81daf62
Compare
This reverts commit 2979f72.
Let's do a follow up, could be good to onboard another team member onto theming stuff? |
Yowza! I guess... do you have any manual overrides of the dev/8.0.x arches theme? I know on your system you did some deep dives into css reset for vue, so I'm wondering if you had any artifacts from that lying around, like css layers or something. |
|
boo failing builds |
oh huh. after you pulled main into the branch things are working now 🎉 |
There was a problem hiding this comment.
isDarkModeEnabled works! (🎉 ). However, regardless of if its value, the theme when the page is loaded is always light. I was able to solve this locally via
onMounted(() => {
if (isDarkModeEnabled.value) {
toggleDarkMode();
}
});
However, this may not be the right solution. There's further complexity to add that may nullify that pattern. Namely, I can set the toggle to dark, see the change, refresh the page, and it's automatically set back to light. My guess is that either cookies or localstorage need to be leveraged to persist this through page refreshes.
If we wanted to get really fancy we could add a column to the User model to support preferred theme, but that's probably overkill at this point. And we'd still need something on the browser to persist for anonymous user so /shrug that's not an entirely baked idea.
If this component is changed to read from cookies/localstorage on mount, it'd be more readable to change the toggle method to accept an arg so it can be explicitly set the mode. This will also support adding other themes later. However I'm not married to this pattern, YAGNI and whatnot /shrug. Regardless, reading from cookies/localstorage nullifies the simple code laid out above.
But this reveals another titchy thing: we're handling all of theming in this component. If I have a cookie ( or bit of localstorage ) saved that says I prefer dark mode, I'd expect the application to handle this regardless of the presence or absence of the header / DarkModeToggle. Like in reports and whatnot. It could be argued that the ability to switch themes should be handled in this component of course, but the ability to query a preferred theme and apply it should be handled at a deeper level.
Perhaps even in core since it's touching the <html> element, but I don't feel strongly about that yet.
I'll update the github action. Maybe we need the updateproject command to yell about this again when the github templates change? |
|
The actions are dense; better to just call out the tiny change in the upgrade instructions. |
IMO it should be automated, part of the |
|
👋 ! Just circling back on this re: where the conversation landed earlier. I believe it ended up being that some of this logic gets moved to core. Essentially the logic to derive the user scheme preference and set it on the The rest -- supporting multiple scheme preferences and adding a column to the |
Fix header skip closes #87
Add dark mode toggle closes #91