From a7bd26722a7441609f39f2b315069ec2476866b9 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 10:22:29 +0200 Subject: [PATCH] refactor(settings): share the general settings save between desktop and phone Saving one preference with the error toast on failure lives in one useSettingSaver hook; the desktop tab drops its eleven inline copies of it. --- .../Settings/DisplaySettingsTab.tsx | 44 ++++++------------- .../Settings/useSettingSaver.test.ts | 40 +++++++++++++++++ .../components/Settings/useSettingSaver.ts | 25 +++++++++++ .../screens/settings/MSettingsGeneral.tsx | 14 +----- 4 files changed, 81 insertions(+), 42 deletions(-) create mode 100644 client/src/components/Settings/useSettingSaver.test.ts create mode 100644 client/src/components/Settings/useSettingSaver.ts diff --git a/client/src/components/Settings/DisplaySettingsTab.tsx b/client/src/components/Settings/DisplaySettingsTab.tsx index 8448fec3e..aea8f97bf 100644 --- a/client/src/components/Settings/DisplaySettingsTab.tsx +++ b/client/src/components/Settings/DisplaySettingsTab.tsx @@ -1,8 +1,7 @@ import React, { useState, useEffect, useRef } from 'react' import { Languages, Map, ChevronDown, Rocket } from 'lucide-react' import { SUPPORTED_LANGUAGES, useTranslation } from '../../i18n' -import { useSettingsStore, DEFAULT_SETTINGS } from '../../store/settingsStore' -import { useToast } from '../shared/Toast' +import { DEFAULT_SETTINGS } from '../../store/settingsStore' import CustomSelect from '../shared/CustomSelect' import { Segmented } from '../shared/dialogParts' import { fs } from '../shared/DialogShell' @@ -13,6 +12,7 @@ import { SYMBOLS, currenciesWith } from '../Budget/BudgetPanel.constants' import Section from './Section' import ToggleSwitch from './ToggleSwitch' import { ChoiceChips, SettingRow, SettingRows } from './settingsKit' +import { useSettingSaver } from './useSettingSaver' import { TRIP_TAB_IDS, TRIP_TAB_LABEL_KEYS } from '../../constants/tripTabs' import { DEFAULT_START_PAGE, DEFAULT_START_TRIP_TAB } from '../../utils/startDestination' import type { DistanceUnit } from '../../types' @@ -23,9 +23,8 @@ import { weekStartOptions } from '../../utils/calendarWeek' const SELECT_BOX = 'w-[260px] max-w-full' export default function DisplaySettingsTab(): React.ReactElement { - const { settings, updateSetting } = useSettingsStore() + const { settings, save } = useSettingSaver() const { t, locale } = useTranslation() - const toast = useToast() const [tempUnit, setTempUnit] = useState(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit) const [distanceUnit, setDistanceUnit] = useState(settings.distance_unit || DEFAULT_SETTINGS.distance_unit) const [langOpen, setLangOpen] = useState(false) @@ -48,19 +47,13 @@ export default function DisplaySettingsTab(): React.ReactElement { setDistanceUnit(settings.distance_unit || DEFAULT_SETTINGS.distance_unit) }, [settings.distance_unit]) - const saveOnOff = async (key: OnOffKey, value: boolean) => { - try { await updateSetting(key, value) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } - } + const saveOnOff = (key: OnOffKey, value: boolean) => save(key, value) const startPage = settings.start_page === 'active_trip' ? 'active_trip' : DEFAULT_START_PAGE const startTripTab = settings.start_trip_tab || DEFAULT_START_TRIP_TAB const currentLanguage = SUPPORTED_LANGUAGES.find(o => o.value === settings.language) || SUPPORTED_LANGUAGES[0] - const pickLanguage = async (value: string) => { - try { await updateSetting('language', value) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } - } + const pickLanguage = (value: string) => save('language', value) return ( <> @@ -79,8 +72,7 @@ export default function DisplaySettingsTab(): React.ReactElement { { value: 'active_trip', label: t('settings.startPageActiveTrip') }, ]} onChange={async value => { - try { await updateSetting('start_page', value) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('start_page', value) }} /> } @@ -96,8 +88,7 @@ export default function DisplaySettingsTab(): React.ReactElement { { - try { await updateSetting('start_trip_tab', String(v)) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('start_trip_tab', String(v)) }} options={TRIP_TAB_IDS.map(id => ({ value: id, label: t(TRIP_TAB_LABEL_KEYS[id]) }))} /> @@ -120,8 +111,7 @@ export default function DisplaySettingsTab(): React.ReactElement { { - try { await updateSetting('default_currency', String(v)) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('default_currency', String(v)) }} options={[ { value: '', label: t('settings.currencyTrip') }, @@ -182,8 +172,7 @@ export default function DisplaySettingsTab(): React.ReactElement { { - try { await updateSetting('place_language', String(v)) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('place_language', String(v)) }} options={[ { value: '', label: t('settings.placeLanguageApp') }, @@ -208,8 +197,7 @@ export default function DisplaySettingsTab(): React.ReactElement { ]} onChange={async value => { setTempUnit(value) - try { await updateSetting('temperature_unit', value) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('temperature_unit', value) }} /> } @@ -228,8 +216,7 @@ export default function DisplaySettingsTab(): React.ReactElement { ]} onChange={async value => { setDistanceUnit(value) - try { await updateSetting('distance_unit', value) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('distance_unit', value) }} /> } @@ -250,8 +237,7 @@ export default function DisplaySettingsTab(): React.ReactElement { label: <>{opt.short}{` (${opt.example})`}, }))} onChange={async value => { - try { await updateSetting('time_format', value) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('time_format', value) }} /> } @@ -266,8 +252,7 @@ export default function DisplaySettingsTab(): React.ReactElement { { - try { await updateSetting('week_start', String(v)) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('week_start', String(v)) }} options={weekStartOptions(locale)} /> @@ -288,8 +273,7 @@ export default function DisplaySettingsTab(): React.ReactElement { { - try { await updateSetting('preferred_nav_app', String(v)) } - catch (e: unknown) { toast.error(e instanceof Error ? e.message : t('common.error')) } + await save('preferred_nav_app', String(v)) }} options={preferredNavAppOptions(t)} /> diff --git a/client/src/components/Settings/useSettingSaver.test.ts b/client/src/components/Settings/useSettingSaver.test.ts new file mode 100644 index 000000000..ab1fcd4eb --- /dev/null +++ b/client/src/components/Settings/useSettingSaver.test.ts @@ -0,0 +1,40 @@ +// FE-COMP-SETTINGSAVER-001 onwards: the one preference save behind both general settings shells. +import { act, renderHook } from '@testing-library/react'; + +import { buildSettings } from '../../../tests/helpers/factories'; +import { resetAllStores, seedStore } from '../../../tests/helpers/store'; +import { useSettingsStore } from '../../store/settingsStore'; +import { useSettingSaver } from './useSettingSaver'; + +const toast = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn() })); +vi.mock('../shared/Toast', () => ({ useToast: () => toast })); +vi.mock('../../i18n', () => ({ useTranslation: () => ({ t: (k: string) => k }) })); + +const updateSetting = vi.fn(); + +beforeEach(() => { + resetAllStores(); + toast.error.mockReset(); + updateSetting.mockReset().mockResolvedValue(undefined); + seedStore(useSettingsStore, { settings: buildSettings({ time_format: '12h' }), updateSetting }); +}); + +describe('useSettingSaver', () => { + it('FE-COMP-SETTINGSAVER-001: hands out the current settings and saves one key', async () => { + const { result } = renderHook(() => useSettingSaver()); + expect(result.current.settings.time_format).toBe('12h'); + await act(() => result.current.save('time_format', '24h')); + expect(updateSetting).toHaveBeenCalledWith('time_format', '24h'); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('FE-COMP-SETTINGSAVER-002: a failure shows its message, or the generic one', async () => { + const { result } = renderHook(() => useSettingSaver()); + updateSetting.mockRejectedValueOnce(new Error('nope')); + await act(() => result.current.save('language', 'de')); + expect(toast.error).toHaveBeenLastCalledWith('nope'); + updateSetting.mockRejectedValueOnce('x'); + await act(() => result.current.save('language', 'de')); + expect(toast.error).toHaveBeenLastCalledWith('common.error'); + }); +}); diff --git a/client/src/components/Settings/useSettingSaver.ts b/client/src/components/Settings/useSettingSaver.ts new file mode 100644 index 000000000..7a530aebd --- /dev/null +++ b/client/src/components/Settings/useSettingSaver.ts @@ -0,0 +1,25 @@ +import { useTranslation } from '../../i18n'; +import { useSettingsStore } from '../../store/settingsStore'; +import type { Settings } from '../../types'; +import { useToast } from '../shared/Toast'; + +/** + * One user preference saved straight away, with the error as a toast when it fails. + * Behind both general settings shells (the desktop tab and the phone section), which + * read the current values from the `settings` this returns. + */ +export function useSettingSaver() { + const { settings, updateSetting } = useSettingsStore(); + const { t } = useTranslation(); + const toast = useToast(); + + const save = async (key: keyof Settings, value: Settings[keyof Settings]) => { + try { + await updateSetting(key, value); + } catch (e: unknown) { + toast.error(e instanceof Error ? e.message : t('common.error')); + } + }; + + return { settings, save }; +} diff --git a/client/src/mobile/screens/settings/MSettingsGeneral.tsx b/client/src/mobile/screens/settings/MSettingsGeneral.tsx index 5b9d5cad7..57dd848d2 100644 --- a/client/src/mobile/screens/settings/MSettingsGeneral.tsx +++ b/client/src/mobile/screens/settings/MSettingsGeneral.tsx @@ -1,8 +1,7 @@ import { useState } from 'react' import { ChevronDown, Languages, Map, Rocket } from 'lucide-react' import { useTranslation, SUPPORTED_LANGUAGES } from '../../../i18n' -import { useSettingsStore } from '../../../store/settingsStore' -import { useToast } from '../../../components/shared/Toast' +import { useSettingSaver } from '../../../components/Settings/useSettingSaver' import { SYMBOLS, currenciesWith } from '../../../components/Budget/BudgetPanel.constants' import { TRIP_TAB_IDS, TRIP_TAB_LABEL_KEYS, isTripTabId } from '../../../constants/tripTabs' import { DEFAULT_START_PAGE, DEFAULT_START_TRIP_TAB, type StartPage } from '../../../utils/startDestination' @@ -20,8 +19,7 @@ import { preferredNavAppOptions } from '../../../components/Planner/placeNavigat */ export default function MSettingsGeneral() { const { t, locale } = useTranslation() - const toast = useToast() - const { settings, updateSetting } = useSettingsStore() + const { settings, save } = useSettingSaver() const [currencyOpen, setCurrencyOpen] = useState(false) const [langOpen, setLangOpen] = useState(false) const [startTabOpen, setStartTabOpen] = useState(false) @@ -29,14 +27,6 @@ export default function MSettingsGeneral() { const navAppOptions = preferredNavAppOptions(t) const navAppValue = settings.preferred_nav_app || '' - const save = async (key: keyof Settings, value: Settings[keyof Settings]) => { - try { - await updateSetting(key, value) - } catch (e: unknown) { - toast.error(e instanceof Error ? e.message : t('common.error')) - } - } - const startPage: StartPage = settings.start_page === 'active_trip' ? 'active_trip' : DEFAULT_START_PAGE const startTripTab = isTripTabId(settings.start_trip_tab) ? settings.start_trip_tab : DEFAULT_START_TRIP_TAB const currency = settings.default_currency || ''