-
Notifications
You must be signed in to change notification settings - Fork 0
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
Consume more tokens from arches theme, add dark mode toggle #91
base: main
Are you sure you want to change the base?
Changes from 11 commits
7e3bbaf
f811b9b
cfc70dc
9149cdf
ec29833
ad89307
81daf62
2979f72
b5aaa35
7ba76f7
8b764d5
abdb414
822246c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<script setup lang="ts"> | ||
import { ref } from "vue"; | ||
import { useGettext } from "vue3-gettext"; | ||
|
||
import ToggleButton from "primevue/togglebutton"; | ||
|
||
import { DEFAULT_THEME } from "@/arches/themes/default.ts"; | ||
|
||
const { $gettext } = useGettext(); | ||
|
||
const isDarkModeEnabled = ref( | ||
window.matchMedia && | ||
window.matchMedia("(prefers-color-scheme: dark)").matches, | ||
); | ||
|
||
function toggleDarkMode() { | ||
chrabyrd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const element = document.querySelector("html"); | ||
// Remove the leading dot from the selector to get class name. | ||
element!.classList.toggle( | ||
DEFAULT_THEME.theme.options.darkModeSelector.substring(1), | ||
); | ||
isDarkModeEnabled.value = !isDarkModeEnabled.value; | ||
} | ||
</script> | ||
|
||
<template> | ||
<ToggleButton | ||
jacobtylerwalls marked this conversation as resolved.
Show resolved
Hide resolved
|
||
:model-value="isDarkModeEnabled" | ||
:on-label="$gettext('Dark')" | ||
:off-label="$gettext('Light')" | ||
off-icon="pi pi-sun" | ||
on-icon="pi pi-moon" | ||
:pt="{ | ||
root: { ariaLabel: $gettext('Toggle dark mode') }, | ||
label: { style: { display: 'none' } }, | ||
}" | ||
@click="toggleDarkMode" | ||
/> | ||
</template> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ import { useGettext } from "vue3-gettext"; | |
|
||
import Button from "primevue/button"; | ||
|
||
import { SECONDARY } from "@/arches_lingo/constants.ts"; | ||
|
||
const { $gettext } = useGettext(); | ||
</script> | ||
|
||
|
@@ -13,11 +15,15 @@ const { $gettext } = useGettext(); | |
as="a" | ||
:label="$gettext('Register')" | ||
:href="arches.urls.signup" | ||
:severity="SECONDARY" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wanted to deemphasize these buttons. |
||
:outlined="true" | ||
/> | ||
<Button | ||
chrabyrd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
as="a" | ||
:label="$gettext('Multi-factor login')" | ||
:href="arches.urls.auth + '?next=/'" | ||
:severity="SECONDARY" | ||
:outlined="true" | ||
/> | ||
</div> | ||
</template> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,13 +52,11 @@ const greeting = computed(() => { | |
</script> | ||
|
||
<template> | ||
<div style="display: flex; align-items: center"> | ||
<div style="display: flex; align-items: center; gap: 1rem"> | ||
<span v-if="user">{{ greeting }}</span> | ||
<Button | ||
style="margin-left: 1rem" | ||
:label="$gettext('Sign out')" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without using the label prop, it wasn't using the tokens for button label font weight (not noticeable until I started overriding that weight via the theme) |
||
@click="issueLogout" | ||
> | ||
{{ $gettext("Sign out") }} | ||
</Button> | ||
></Button> | ||
</div> | ||
</template> |
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.
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 viaHowever, 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 tolight
. 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.