Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Consume more tokens from arches theme, add dark mode toggle #91

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Consume more tokens from arches theme, add dark mode toggle #91

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7e3bbaf
Consume more tokens from arches theme
jacobtylerwalls Sep 10, 2024
f811b9b
Add dark mode toggle
jacobtylerwalls Sep 10, 2024
cfc70dc
Add aria-label on dark mode toggle
jacobtylerwalls Sep 11, 2024
9149cdf
Consume custom toast styling from core
jacobtylerwalls Sep 12, 2024
ec29833
Consume fontFamily token
jacobtylerwalls Oct 7, 2024
ad89307
Avoid arrow function
jacobtylerwalls Oct 7, 2024
81daf62
Un-tokenize some styles
jacobtylerwalls Oct 7, 2024
2979f72
Ignore constants.ts from coverage
jacobtylerwalls Oct 30, 2024
b5aaa35
Revert "Ignore constants.ts from coverage"
jacobtylerwalls Oct 30, 2024
7ba76f7
PR feedback
jacobtylerwalls Nov 1, 2024
8b764d5
Merge branch 'main' into jtw/consume-tokens
jacobtylerwalls Nov 1, 2024
abdb414
Adjust action template
jacobtylerwalls Nov 1, 2024
822246c
Respect the initial value of isDarkModeEnabled
jacobtylerwalls Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Add login interface [#13](https://github.com/archesproject/arches-lingo/issues/13)
- Add front-end router [#11](https://github.com/archesproject/arches-lingo/issues/11)
- Add dark mode toggle
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
- Add backend for search [#67](https://github.com/archesproject/arches-lingo/issues/67)

### Fixed
Expand Down
23 changes: 22 additions & 1 deletion arches_lingo/media/js/views/root.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import createVueApplication from 'arches/arches/app/media/js/utils/create-vue-application';
import { ArchesPreset, DEFAULT_THEME } from '@/arches/themes/default.ts';

import { definePreset } from '@primevue/themes';
import { createRouter, createWebHistory } from 'vue-router';

import LingoApp from '@/arches_lingo/App.vue';
Expand All @@ -10,7 +12,26 @@ const router = createRouter({
routes,
});

createVueApplication(LingoApp).then(vueApp => {
const LingoPreset = definePreset(ArchesPreset, {
components: {
button: {
root: {
label: {
fontWeight: 600,
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
},
},
},
},
});

const LingoTheme = {
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
theme: {
...DEFAULT_THEME,
preset: LingoPreset,
},
};

createVueApplication(LingoApp, LingoTheme).then(vueApp => {
vueApp.use(router);
vueApp.mount('#lingo-mounting-point');
});
12 changes: 10 additions & 2 deletions arches_lingo/src/arches_lingo/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,20 @@ router.beforeEach(async (to, _from, next) => {
</div>
</div>
</main>
<Toast />
<Toast
:pt="{
summary: { fontSize: 'medium' },
detail: { fontSize: 'small' },
messageIcon: {
style: { marginTop: 'var(--p-toast-messageicon-margintop)' },
},
}"
/>
</template>

<style scoped>
main {
font-family: sans-serif;
font-family: system-ui, sans-serif;
Copy link
Member Author

@jacobtylerwalls jacobtylerwalls Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, there's now a difference (on my local browser) between semibold and bold. (sans-serif in my browser was lacking a weight value for 600). Which means: we can preset the font weight for button labels to 600/semibold without it being too loud (it was looking the same as bold)

Then we'll have a default for button labels that looks good both in the controlled list manager (arches chrome) and here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh I'd say remove system-ui, as that varies from system to system and pulls us away from a unified experience. https://infinnie.github.io/blog/2017/systemui.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I saw that also, but I can't remember if I saw that before or after I made this addition, 😄 . I'll back it out. You have a good point we're building a unified experience, not a blog for a law office.

height: 100vh;
width: 100vw;
overflow-x: hidden;
Expand Down
39 changes: 39 additions & 0 deletions arches_lingo/src/arches_lingo/components/header/DarkModeToggle.vue
View file
Open in desktop
Copy link
Contributor

@chrabyrd chrabyrd Nov 1, 2024 •
edited
Loading

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 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.

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>
18 changes: 12 additions & 6 deletions arches_lingo/src/arches_lingo/components/header/PageHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useGettext } from "vue3-gettext";
import Menubar from "primevue/menubar";

import { routeNames } from "@/arches_lingo/routes.ts";
import DarkModeToggle from "@/arches_lingo/components/header/DarkModeToggle.vue";
import UserInteraction from "@/arches_lingo/components/user/UserInteraction.vue";
import SearchDialog from "@/arches_lingo/components/header/SearchDialog.vue";

Expand All @@ -21,15 +22,15 @@ const items = ref([

<template>
<Menubar
class="page-header"
:model="items"
style="border-radius: 0"
>
<template #start>
<RouterLink
:to="{ name: routeNames.root }"
style="text-decoration: none; color: inherit"
>
<h2>{{ $gettext("Arches Lingo") }}</h2>
<h1>{{ $gettext("Arches Lingo") }}</h1>
</RouterLink>
<SearchDialog />
</template>
Expand All @@ -43,18 +44,23 @@ const items = ref([
:class="item.icon"
aria-hidden="true"
></i>
<span>{{ item.label }}</span>
<span style="font-weight: var(--p-button-label-font-weight)">{{
item.label
}}</span>
</RouterLink>
</template>
<template #end>
<UserInteraction />
<div style="display: flex; align-items: center; gap: 1rem">
<DarkModeToggle />
<UserInteraction />
</div>
</template>
</Menubar>
</template>

<style scoped>
.page-header {
border-radius: 0;
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
:deep(.p-menubar-start) {
gap: var(--p-menubar-gap);
}

@media screen and (max-width: 960px) {
Expand Down
6 changes: 6 additions & 0 deletions arches_lingo/src/arches_lingo/components/login/LoginLinks.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -13,11 +15,15 @@ const { $gettext } = useGettext();
as="a"
:label="$gettext('Register')"
:href="arches.urls.signup"
:severity="SECONDARY"
Copy link
Member Author

Choose a reason for hiding this comment

The 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>
6 changes: 3 additions & 3 deletions arches_lingo/src/arches_lingo/components/sidenav/SideNav.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ const items = ref([
display: flex;
flex-direction: column;
align-items: center;
min-width: 3rem;
min-width: var(--p-button-icon-only-width);
chrabyrd marked this conversation as resolved.
Show resolved Hide resolved
border-right: 1px solid var(--p-menubar-border-color);
}

.p-button {
min-height: 3rem;
min-width: 3rem;
min-height: var(--p-button-icon-only-width);
min-width: var(--p-button-icon-only-width);
border-radius: 0;
font-size: 1.5rem;
}
Expand Down
8 changes: 3 additions & 5 deletions arches_lingo/src/arches_lingo/components/user/UserInteraction.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')"
Copy link
Member Author

@jacobtylerwalls jacobtylerwalls Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The 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>
1 change: 1 addition & 0 deletions arches_lingo/src/arches_lingo/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { UserRefAndSetter } from "@/arches_lingo/types.ts";

export const ANONYMOUS = "anonymous";
export const ERROR = "error";
export const SECONDARY = "secondary";

export const DEFAULT_ERROR_TOAST_LIFE = 8000;
export const SEARCH_RESULTS_PER_PAGE = 25;
Expand Down
Loading