Maintenance: Desktop view - App theme defaulted to light mode on initial setup

app/frontend/apps/desktop/components/Form/fields/FieldDate/FieldDateTimeInput.vue

@@ -10,7 +10,8 @@ import type { DateTimeContext } from '#shared/components/Form/fields/FieldDate/t
 import { useDateTime } from '#shared/components/Form/fields/FieldDate/useDateTime.ts'
 import { EnumTextDirection } from '#shared/graphql/types.ts'
 import { i18n } from '#shared/i18n.ts'
-import { useAppTheme } from '#shared/stores/theme.ts'
+
+import { useThemeStore } from '#desktop/stores/theme.ts'
 import '@vuepic/vue-datepicker/dist/main.css'
 
 interface Props {
@@ -59,9 +60,9 @@ const inputIcon = computed(() => {
 
 const picker = ref<DatePickerInstance>()
 
-const { theme } = storeToRefs(useAppTheme())
+const { currentTheme } = storeToRefs(useThemeStore())
 
-const dark = computed(() => theme.value === 'dark')
+const dark = computed(() => currentTheme.value === 'dark')
 </script>
 
 <template>

app/frontend/apps/desktop/components/layout/LayoutSidebar/AvatarMenu/AvatarMenuAppearanceItem.vue

@@ -1,14 +1,15 @@
 <!-- Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/ -->
 
 <script setup lang="ts">
-import { ref } from 'vue'
+import { storeToRefs } from 'pinia'
+import { computed, ref } from 'vue'
 
 import CommonPopoverMenuItem, {
   type Props,
 } from '#desktop/components/CommonPopover/CommonPopoverMenuItem.vue'
 import ThemeSwitch from '#desktop/components/ThemeSwitch/ThemeSwitch.vue'
 import type { ThemeSwitchInstance } from '#desktop/components/ThemeSwitch/types.ts'
-import { useThemeUpdate } from '#desktop/pages/personal-setting/composables/useThemeUpdate.ts'
+import { useThemeStore } from '#desktop/stores/theme.ts'
 
 defineProps<Props>()
 
@@ -18,7 +19,14 @@ const cycleThemeSwitchValue = () => {
   themeSwitch.value?.cycleValue()
 }
 
-const { currentTheme } = useThemeUpdate()
+const themeStore = useThemeStore()
+const { updateTheme } = themeStore
+const { currentTheme } = storeToRefs(themeStore)
+
+const modelTheme = computed({
+  get: () => currentTheme.value,
+  set: (theme) => updateTheme(theme),
+})
 </script>
 
 <template>
@@ -29,7 +37,7 @@ const { currentTheme } = useThemeUpdate()
   <div class="flex items-center px-2">
     <ThemeSwitch
       ref="themeSwitch"
-      v-model="currentTheme"
+      v-model="modelTheme"
       class="hover:outline-blue-300 focus:outline-blue-600 hover:focus:outline-blue-600 dark:hover:outline-blue-950 dark:focus:outline-blue-900 dark:hover:focus:outline-blue-900"
       size="small"
     />

app/frontend/apps/desktop/components/layout/LayoutSidebar/AvatarMenu/__tests__/AvaterMenuAppearanceItem.spec.ts

@@ -10,16 +10,16 @@ import { useSessionStore } from '#shared/stores/session.ts'
 
 import AvatarMenuAppearanceItem from '../AvatarMenuAppearanceItem.vue'
 
-describe('avatar menu apperance item', () => {
-  beforeEach(() => {
+describe('avatar menu appearance item', () => {
+  it('renders menu item with switcher', async () => {
     mockUserCurrent({
       lastname: 'Doe',
       firstname: 'John',
-      preferences: {},
+      preferences: {
+        theme: EnumAppearanceTheme.Dark,
+      },
     })
-  })
 
-  it('renders menu item with switcher', async () => {
     const view = renderComponent(AvatarMenuAppearanceItem, {
       props: {
         label: 'Appearance',
@@ -29,14 +29,14 @@ describe('avatar menu apperance item', () => {
     expect(view.getByText('Appearance')).toBeInTheDocument()
     const appearanceSwitch = view.getByRole('checkbox', { name: 'Dark Mode' })
 
-    expect(appearanceSwitch).toBePartiallyChecked()
+    expect(appearanceSwitch).toBeChecked()
 
     await view.events.click(appearanceSwitch)
 
-    expect(appearanceSwitch).toBeChecked()
+    expect(appearanceSwitch).not.toBeChecked()
 
     const session = useSessionStore()
 
-    expect(session.user?.preferences?.theme).toBe(EnumAppearanceTheme.Dark)
+    expect(session.user?.preferences?.theme).toBe(EnumAppearanceTheme.Light)
   })
 })

app/frontend/apps/desktop/main.ts

@@ -14,7 +14,6 @@ import { useAuthenticationStore } from '#shared/stores/authentication.ts'
 import initializeStore from '#shared/stores/index.ts'
 import { useLocaleStore } from '#shared/stores/locale.ts'
 import { useSessionStore } from '#shared/stores/session.ts'
-import { useAppTheme } from '#shared/stores/theme.ts'
 
 import { twoFactorConfigurationPluginLookup } from '#desktop/entities/two-factor-configuration/plugins/index.ts'
 import { initializeForm, initializeFormFields } from '#desktop/form/index.ts'
@@ -23,6 +22,7 @@ import { initializeGlobalComponentStyles } from '#desktop/initializer/initialize
 import { ensureAfterAuth } from '#desktop/pages/authentication/after-auth/composable/useAfterAuthPlugins.ts'
 import initializeRouter from '#desktop/router/index.ts'
 import initializeApolloClient from '#desktop/server/apollo/index.ts'
+import { useThemeStore } from '#desktop/stores/theme.ts'
 
 import App from './AppDesktop.vue'
 
@@ -72,7 +72,7 @@ export const mountApp = async () => {
   }
 
   // sync theme so the store is initialized and user (if exists) and DOM have the same value
-  useAppTheme().syncTheme()
+  useThemeStore().syncTheme()
 
   if (VITE_TEST_MODE) {
     await import('#shared/initializer/initializeFakeTimer.ts')

app/frontend/apps/desktop/pages/personal-setting/__tests__/personal-setting-appearance.spec.ts

@@ -1,20 +1,59 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
 import { visitView } from '#tests/support/components/visitView.ts'
+import { mockUserCurrent } from '#tests/support/mock-userCurrent.ts'
 
-import { waitForUserCurrentAppearanceMutationCalls } from '../graphql/mutations/userCurrentAppearance.mocks.ts'
+import { EnumAppearanceTheme } from '#shared/graphql/types.ts'
+
+import { waitForUserCurrentAppearanceMutationCalls } from '#desktop/pages/personal-setting/graphql/mutations/userCurrentAppearance.mocks.ts'
 
 describe('appearance page', () => {
+  it('should have dark theme set', async () => {
+    mockUserCurrent({
+      preferences: {
+        theme: EnumAppearanceTheme.Dark,
+      },
+    })
+
+    const view = await visitView('/personal-setting/appearance')
+
+    expect(view.getByRole('radio', { checked: true })).toHaveTextContent('dark')
+  })
+
+  it('should have light theme set', async () => {
+    mockUserCurrent({
+      preferences: {
+        theme: EnumAppearanceTheme.Light,
+      },
+    })
+
+    const view = await visitView('/personal-setting/appearance')
+
+    expect(view.getByRole('radio', { checked: true })).toHaveTextContent(
+      'light',
+    )
+  })
+
   it('update appearance to dark', async () => {
+    mockUserCurrent({
+      preferences: {
+        theme: EnumAppearanceTheme.Light,
+      },
+    })
     const view = await visitView('/personal-setting/appearance')
 
+    expect(view.getByLabelText('Light')).toBeChecked()
+
     const darkMode = view.getByText('Dark')
     const lightMode = view.getByText('Light')
     const syncWithComputer = view.getByText('Sync with computer')
 
     await view.events.click(darkMode)
+
     expect(view.getByLabelText('Dark')).toBeChecked()
+
     const calls = await waitForUserCurrentAppearanceMutationCalls()
+
     expect(calls.at(-1)?.variables).toEqual({ theme: 'dark' })
     expect(window.matchMedia('(prefers-color-scheme: light)').matches).toBe(
       false,
@@ -22,12 +61,14 @@ describe('appearance page', () => {
 
     await view.events.click(lightMode)
     await vi.waitUntil(() => calls.length === 2)
+
     expect(calls.at(-1)?.variables).toEqual({ theme: 'light' })
     expect(window.matchMedia('(prefers-color-scheme: dark)').matches).toBe(
       false,
     )
 
     await view.events.click(syncWithComputer)
+
     expect(view.getByLabelText('Sync with computer')).toBeChecked()
   })
 })

app/frontend/apps/desktop/pages/personal-setting/composables/__tests__/useThemeUpdate.spec.ts

@@ -1,79 +0,0 @@
-// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
-
-import { flushPromises } from '@vue/test-utils'
-
-import { mockUserCurrent } from '#tests/support/mock-userCurrent.ts'
-
-import { EnumAppearanceTheme } from '#shared/graphql/types.ts'
-
-import { mockUserCurrentAppearanceMutation } from '../../graphql/mutations/userCurrentAppearance.mocks.ts'
-import { useThemeUpdate } from '../useThemeUpdate.ts'
-
-describe('useThemeUpdate', () => {
-  beforeEach(() => {
-    mockUserCurrent({
-      lastname: 'Doe',
-      firstname: 'John',
-      preferences: {},
-    })
-  })
-
-  it('should fallback to auto when no theme present', () => {
-    const { currentTheme } = useThemeUpdate()
-
-    expect(currentTheme.value).toBe('auto')
-  })
-
-  it('should change theme value', async () => {
-    const mockerUserCurrentAppearanceUpdate = mockUserCurrentAppearanceMutation(
-      {
-        userCurrentAppearance: {
-          success: true,
-        },
-      },
-    )
-
-    const { currentTheme, savingTheme } = useThemeUpdate()
-
-    currentTheme.value = EnumAppearanceTheme.Dark
-    expect(currentTheme.value).toBe(EnumAppearanceTheme.Dark)
-
-    expect(savingTheme.value).toBe(true)
-
-    const mockCalls = await mockerUserCurrentAppearanceUpdate.waitForCalls()
-    expect(mockCalls).toHaveLength(1)
-
-    await flushPromises()
-    expect(savingTheme.value).toBe(false)
-    expect(currentTheme.value).toBe(EnumAppearanceTheme.Dark)
-  })
-
-  it('should change theme value back to old value when update fails', async () => {
-    const mockerUserCurrentAppearanceUpdate = mockUserCurrentAppearanceMutation(
-      {
-        userCurrentAppearance: {
-          errors: [
-            {
-              message: 'Failed to update.',
-            },
-          ],
-        },
-      },
-    )
-
-    const { currentTheme, savingTheme } = useThemeUpdate()
-
-    currentTheme.value = EnumAppearanceTheme.Dark
-
-    expect(currentTheme.value).toBe(EnumAppearanceTheme.Dark)
-    expect(savingTheme.value).toBe(true)
-
-    const mockCalls = await mockerUserCurrentAppearanceUpdate.waitForCalls()
-    expect(mockCalls).toHaveLength(1)
-
-    await flushPromises()
-
-    expect(savingTheme.value).toBe(false)
-    expect(currentTheme.value).toBe(EnumAppearanceTheme.Auto)
-  })
-})

app/frontend/apps/desktop/pages/personal-setting/composables/useThemeUpdate.ts

@@ -1,70 +0,0 @@
-// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
-
-import { computed, ref } from 'vue'
-
-import {
-  NotificationTypes,
-  useNotifications,
-} from '#shared/components/CommonNotifications/index.ts'
-import { EnumAppearanceTheme } from '#shared/graphql/types.ts'
-import MutationHandler from '#shared/server/apollo/handler/MutationHandler.ts'
-import { useSessionStore } from '#shared/stores/session.ts'
-
-import { useUserCurrentAppearanceMutation } from '../graphql/mutations/userCurrentAppearance.api.ts'
-
-export const useThemeUpdate = (showSuccessNotification = false) => {
-  const setThemeMutation = new MutationHandler(
-    useUserCurrentAppearanceMutation(),
-    {
-      errorNotificationMessage: __('The appearance could not be updated.'),
-    },
-  )
-
-  const savingTheme = ref(false)
-
-  const session = useSessionStore()
-  const { notify } = useNotifications()
-
-  const setTheme = async (theme: string) => {
-    const oldTheme = session.user?.preferences?.theme
-
-    // Do it before the mutation to avoid a laggy UI.
-    session.setUserPreference('theme', theme)
-
-    return setThemeMutation
-      .send({ theme: theme as EnumAppearanceTheme })
-      .catch(() => {
-        session.setUserPreference('theme', oldTheme)
-      })
-  }
-
-  const currentTheme = computed({
-    get: () => session.user?.preferences?.theme || 'auto',
-    set: (value: EnumAppearanceTheme) => {
-      if (value === session.user?.preferences?.theme || savingTheme.value) {
-        return
-      }
-
-      savingTheme.value = true
-      setTheme(value)
-        .then(() => {
-          if (showSuccessNotification) {
-            notify({
-              id: 'theme-update',
-              message: __('Profile appearance updated successfully.'),
-              type: NotificationTypes.Success,
-            })
-          }
-        })
-        .finally(() => {
-          savingTheme.value = false
-        })
-    },
-  })
-
-  return {
-    currentTheme,
-    savingTheme,
-    setTheme,
-  }
-}

app/frontend/apps/desktop/pages/personal-setting/views/PersonalSettingAppearance.vue

@@ -1,12 +1,36 @@
 <!-- Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/ -->
 
 <script setup lang="ts">
+import { storeToRefs } from 'pinia'
+import { computed } from 'vue'
+
+import {
+  NotificationTypes,
+  useNotifications,
+} from '#shared/components/CommonNotifications/index.ts'
+
 import LayoutContent from '#desktop/components/layout/LayoutContent.vue'
+import { useThemeStore } from '#desktop/stores/theme.ts'
 
 import { useBreadcrumb } from '../composables/useBreadcrumb.ts'
-import { useThemeUpdate } from '../composables/useThemeUpdate.ts'
 
-const { currentTheme, savingTheme } = useThemeUpdate(true)
+const { notify } = useNotifications()
+const themeStore = useThemeStore()
+const { updateTheme } = themeStore
+const { currentTheme, savingTheme } = storeToRefs(themeStore)
+
+const modelTheme = computed({
+  get: () => currentTheme.value,
+  set: (theme) => {
+    updateTheme(theme).then(() => {
+      notify({
+        id: 'theme-update',
+        message: __('Your theme has been updated.'),
+        type: NotificationTypes.Success,
+      })
+    })
+  },
+})
 
 const themeOptions = [
   {
@@ -39,7 +63,7 @@ const { breadcrumbItems } = useBreadcrumb(__('Appearance'))
   <LayoutContent :breadcrumb-items="breadcrumbItems" width="narrow">
     <div class="mb-4">
       <FormKit
-        v-model="currentTheme"
+        v-model="modelTheme"
         type="radioList"
         name="theme"
         :label="__('Theme')"

app/frontend/apps/desktop/stores/__tests__/theme.spec.ts

@@ -0,0 +1,148 @@
+// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+import { flushPromises } from '@vue/test-utils'
+import { createPinia, setActivePinia, storeToRefs } from 'pinia'
+
+import { mockUserCurrent } from '#tests/support/mock-userCurrent.ts'
+
+import { EnumAppearanceTheme } from '#shared/graphql/types.ts'
+
+import { mockUserCurrentAppearanceMutation } from '#desktop/pages/personal-setting/graphql/mutations/userCurrentAppearance.mocks.ts'
+import { useThemeStore } from '#desktop/stores/theme.ts'
+
+const mockUserTheme = (theme: string | undefined) => {
+  mockUserCurrent({
+    preferences: {
+      theme,
+    },
+  })
+}
+
+const haveDOMTheme = (theme: string | undefined) => {
+  const root = document.querySelector(':root') as HTMLElement
+  if (!theme) {
+    root.removeAttribute('data-theme')
+  } else {
+    root.dataset.theme = theme
+  }
+}
+
+const getDOMTheme = () => {
+  const root = document.querySelector(':root') as HTMLElement
+  return root.dataset.theme
+}
+
+const addEventListener = vi.fn()
+
+const haveMediaTheme = (theme: string) => {
+  window.matchMedia = (rule) =>
+    ({
+      matches: rule === '(prefers-color-scheme: dark)' && theme === 'dark',
+      addEventListener,
+    }) as any
+}
+
+describe('useThemeStore', () => {
+  beforeEach(() => {
+    setActivePinia(createPinia())
+    mockUserCurrent({
+      lastname: 'Doe',
+      firstname: 'John',
+      preferences: {},
+    })
+
+    const { syncTheme } = useThemeStore()
+    syncTheme()
+
+    haveDOMTheme(undefined)
+    haveMediaTheme('light')
+  })
+
+  it('should fallback to auto when no theme present', () => {
+    const { currentTheme } = useThemeStore()
+
+    expect(currentTheme).toBe(EnumAppearanceTheme.Auto)
+  })
+
+  it('changes app theme', async () => {
+    const mockerUserCurrentAppearanceUpdate = mockUserCurrentAppearanceMutation(
+      {
+        userCurrentAppearance: {
+          success: true,
+        },
+      },
+    )
+
+    const themeStore = useThemeStore()
+    const { updateTheme } = themeStore
+    const { currentTheme, savingTheme } = storeToRefs(themeStore)
+
+    await updateTheme(EnumAppearanceTheme.Dark)
+
+    expect(currentTheme.value).toBe(EnumAppearanceTheme.Dark)
+
+    const mockCalls = await mockerUserCurrentAppearanceUpdate.waitForCalls()
+    expect(mockCalls).toHaveLength(1)
+
+    await flushPromises()
+    expect(savingTheme.value).toBe(false)
+    expect(currentTheme.value).toBe(EnumAppearanceTheme.Dark)
+  })
+
+  it('should change theme value back to old value when update fails', async () => {
+    const mockerUserCurrentAppearanceUpdate = mockUserCurrentAppearanceMutation(
+      {
+        userCurrentAppearance: {
+          errors: [
+            {
+              message: 'Failed to update.',
+            },
+          ],
+        },
+      },
+    )
+
+    const themStore = useThemeStore()
+    const { updateTheme } = themStore
+    const { currentTheme, savingTheme } = storeToRefs(themStore)
+
+    await updateTheme(EnumAppearanceTheme.Dark)
+
+    expect(currentTheme.value).toBe(EnumAppearanceTheme.Auto)
+
+    const mockCalls = await mockerUserCurrentAppearanceUpdate.waitForCalls()
+    expect(mockCalls).toHaveLength(1)
+
+    await flushPromises()
+
+    expect(savingTheme.value).toBe(false)
+    expect(currentTheme.value).toBe(EnumAppearanceTheme.Auto)
+  })
+
+  // describe('when user has a preference', () => {
+  it('when user has no theme preference, takes from media', () => {
+    const { syncTheme } = useThemeStore()
+    syncTheme()
+
+    const { currentTheme } = useThemeStore()
+
+    expect(currentTheme).toBe('auto')
+  })
+
+  it("changes in media don't affect theme", async () => {
+    mockUserTheme(EnumAppearanceTheme.Dark)
+    haveMediaTheme(EnumAppearanceTheme.Dark)
+
+    const { syncTheme, currentTheme } = useThemeStore()
+    syncTheme()
+
+    expect(currentTheme).toBe(EnumAppearanceTheme.Dark)
+    expect(getDOMTheme()).toBe(EnumAppearanceTheme.Dark)
+
+    haveMediaTheme(EnumAppearanceTheme.Light)
+    addEventListener.mock.calls[0][1]()
+
+    expect(currentTheme).toBe(EnumAppearanceTheme.Dark)
+    expect(getDOMTheme()).toBe(EnumAppearanceTheme.Dark)
+  })
+})

app/frontend/apps/desktop/stores/theme.ts

@@ -0,0 +1,113 @@
+// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+import { acceptHMRUpdate, defineStore } from 'pinia'
+import { computed, ref, watch } from 'vue'
+
+import { EnumAppearanceTheme } from '#shared/graphql/types.ts'
+import MutationHandler from '#shared/server/apollo/handler/MutationHandler.ts'
+import { useSessionStore } from '#shared/stores/session.ts'
+
+import { useUserCurrentAppearanceMutation } from '#desktop/pages/personal-setting/graphql/mutations/userCurrentAppearance.api.ts'
+
+type AppThemeName = EnumAppearanceTheme.Light | EnumAppearanceTheme.Dark
+
+const getRoot = () => document.querySelector(':root') as HTMLElement
+
+const getPreferredTheme = (): AppThemeName => {
+  return window.matchMedia('(prefers-color-scheme: dark)').matches
+    ? EnumAppearanceTheme.Dark
+    : EnumAppearanceTheme.Light
+}
+
+const sanitizeTheme = (theme: string): AppThemeName => {
+  if (['dark', 'light'].includes(theme)) return theme as AppThemeName
+  return getPreferredTheme()
+}
+
+const saveDOMTheme = (theme: AppThemeName) => {
+  getRoot().dataset.theme = theme
+}
+
+export const useThemeStore = defineStore('theme', () => {
+  const session = useSessionStore()
+
+  const savingTheme = ref(false)
+
+  const setThemeMutation = new MutationHandler(
+    useUserCurrentAppearanceMutation(),
+    {
+      errorNotificationMessage: __('The appearance could not be updated.'),
+    },
+  )
+
+  const saveTheme = (newTheme: AppThemeName) => {
+    const sanitizedTheme = sanitizeTheme(newTheme)
+    saveDOMTheme(sanitizedTheme)
+  }
+
+  const setTheme = async (theme: string) => {
+    const oldTheme = session.user?.preferences?.theme
+
+    session.setUserPreference('theme', theme)
+
+    return setThemeMutation
+      .send({ theme: theme as EnumAppearanceTheme })
+      .catch(() => {
+        session.setUserPreference('theme', oldTheme)
+      })
+  }
+
+  const currentTheme = computed(
+    () => session.user?.preferences?.theme || 'auto',
+  )
+
+  const updateTheme = async (value: EnumAppearanceTheme) => {
+    try {
+      if (value === session.user?.preferences?.theme || savingTheme.value)
+        return
+
+      savingTheme.value = true
+
+      await setTheme(value)
+    } finally {
+      savingTheme.value = false
+    }
+  }
+
+  // sync theme in case HTML value was not up-to-date when we loaded user preferences
+  const syncTheme = () => {
+    if (currentTheme.value !== 'auto') return saveDOMTheme(currentTheme.value)
+    const theme = getPreferredTheme()
+    saveDOMTheme(theme)
+  }
+
+  // Update based on global system level preference
+  window
+    .matchMedia('(prefers-color-scheme: dark)')
+    .addEventListener('change', () => {
+      // don't override preferred theme if user has already selected one
+      const theme = (currentTheme.value as AppThemeName) || getPreferredTheme()
+      saveTheme(theme)
+    })
+
+  // in case user changes the theme in another tab
+  watch(
+    () => currentTheme.value,
+    (newTheme) => {
+      if (newTheme) {
+        saveTheme(newTheme as AppThemeName)
+      }
+    },
+  )
+
+  return {
+    savingTheme,
+    currentTheme,
+    updateTheme,
+    syncTheme,
+  }
+})
+
+if (import.meta.hot) {
+  import.meta.hot.accept(acceptHMRUpdate(useThemeStore, import.meta.hot))
+}