fix(settings): make fresh-instance unit defaults internally consistent

A brand-new instance — no admin-set defaults, no saved value — rendered
temperatures in Fahrenheit and times on the 12-hour clock while distances
defaulted to metric, a mix that matches no locale. The store also seeded
'fahrenheit'/'12h' while DisplaySettingsTab's fallback said 'celsius', so the
two paths disagreed about the intended default in the first place.

Default to one system — celsius / metric / 24h — matching the already-metric
distance_unit. The unit defaults now live in a single exported DEFAULT_SETTINGS
that both the store and DisplaySettingsTab's fallback read, so they can't drift
apart again. Admin-set user defaults (getAdminUserDefaults) and any value a user
has already saved still take precedence; only the code-level default changed.
This commit is contained in:
Maurice
2026-07-18 19:53:28 +02:00
committed by Maurice
parent 322fb50c78
commit ba3733da4d
3 changed files with 58 additions and 29 deletions
@@ -1,7 +1,7 @@
import React, { useState, useEffect, useRef } from 'react'
import { Languages, Map, ChevronDown, Check } from 'lucide-react'
import { SUPPORTED_LANGUAGES, useTranslation } from '../../i18n'
import { useSettingsStore } from '../../store/settingsStore'
import { useSettingsStore, DEFAULT_SETTINGS } from '../../store/settingsStore'
import { useToast } from '../shared/Toast'
import CustomSelect from '../shared/CustomSelect'
import { SYMBOLS, currenciesWith } from '../Budget/BudgetPanel.constants'
@@ -12,8 +12,8 @@ export default function DisplaySettingsTab(): React.ReactElement {
const { settings, updateSetting } = useSettingsStore()
const { t } = useTranslation()
const toast = useToast()
const [tempUnit, setTempUnit] = useState<string>(settings.temperature_unit || 'celsius')
const [distanceUnit, setDistanceUnit] = useState<DistanceUnit>(settings.distance_unit || 'metric')
const [tempUnit, setTempUnit] = useState<string>(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit)
const [distanceUnit, setDistanceUnit] = useState<DistanceUnit>(settings.distance_unit || DEFAULT_SETTINGS.distance_unit)
const [langOpen, setLangOpen] = useState(false)
const langDropdownRef = useRef<HTMLDivElement | null>(null)
@@ -27,11 +27,11 @@ export default function DisplaySettingsTab(): React.ReactElement {
}, [langOpen])
useEffect(() => {
setTempUnit(settings.temperature_unit || 'celsius')
setTempUnit(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit)
}, [settings.temperature_unit])
useEffect(() => {
setDistanceUnit(settings.distance_unit || 'metric')
setDistanceUnit(settings.distance_unit || DEFAULT_SETTINGS.distance_unit)
}, [settings.distance_unit])
return (
+21
View File
@@ -0,0 +1,21 @@
import { describe, it, expect } from 'vitest'
import { DEFAULT_SETTINGS, useSettingsStore } from './settingsStore'
// A fresh instance sends no value for a setting an admin hasn't defaulted, so DEFAULT_SETTINGS
// is what a brand-new user actually sees. These guard against the two regressions in the
// original bug: unit defaults that mix measurement systems (°F alongside kilometres), and a
// store default that silently disagrees with DisplaySettingsTab's fallback.
describe('settings defaults', () => {
it('SETTINGS-DEFAULTS-001: the shipped unit defaults belong to one consistent system', () => {
expect(DEFAULT_SETTINGS.temperature_unit).toBe('celsius')
expect(DEFAULT_SETTINGS.distance_unit).toBe('metric')
expect(DEFAULT_SETTINGS.time_format).toBe('24h')
})
it('SETTINGS-DEFAULTS-002: the store initialises from DEFAULT_SETTINGS, the same constant DisplaySettingsTab falls back to, so the two cannot drift apart', () => {
const settings = useSettingsStore.getState().settings
expect(settings.temperature_unit).toBe(DEFAULT_SETTINGS.temperature_unit)
expect(settings.distance_unit).toBe(DEFAULT_SETTINGS.distance_unit)
expect(settings.time_format).toBe(DEFAULT_SETTINGS.time_format)
})
})
+32 -24
View File
@@ -21,31 +21,39 @@ interface SettingsState {
export const hasStoredLanguage = (): boolean =>
typeof localStorage !== 'undefined' && !!localStorage.getItem('app_language')
// The effective client-side defaults for a fresh instance. The server sends no value for
// a setting an admin hasn't defaulted (see settingsService.getAdminUserDefaults), so these
// are what a brand-new user actually sees. Keep them internally consistent — one
// measurement system, not °F alongside kilometres — and note that DisplaySettingsTab
// imports these same values for its fallbacks, so the store default and the UI fallback
// can't drift apart again.
export const DEFAULT_SETTINGS: Settings = {
map_tile_url: '',
dark_mode: false,
// Empty = no personal display currency, so Costs falls back to the trip's own.
default_currency: '',
language: localStorage.getItem('app_language') || 'en',
temperature_unit: 'celsius',
distance_unit: 'metric',
time_format: '24h',
show_place_description: false,
optimize_from_accommodation: true,
map_provider: 'leaflet',
map_poi_pill_enabled: true,
mapbox_access_token: '',
mapbox_style: 'mapbox://styles/mapbox/standard',
maplibre_style: '',
mapbox_3d_enabled: true,
mapbox_quality_mode: false,
dashboard_fx_from: 'EUR',
dashboard_fx_to: 'USD',
appearance: DEFAULT_APPEARANCE,
// dashboard_timezones is intentionally left unset so the widget can tell "never
// chosen" (fall back to home + defaults) from an explicitly emptied list.
}
export const useSettingsStore = create<SettingsState>((set, get) => ({
settings: {
map_tile_url: '',
dark_mode: false,
// Empty = no personal display currency, so Costs falls back to the trip's own.
default_currency: '',
language: localStorage.getItem('app_language') || 'en',
temperature_unit: 'fahrenheit',
distance_unit: 'metric',
time_format: '12h',
show_place_description: false,
optimize_from_accommodation: true,
map_provider: 'leaflet',
map_poi_pill_enabled: true,
mapbox_access_token: '',
mapbox_style: 'mapbox://styles/mapbox/standard',
maplibre_style: '',
mapbox_3d_enabled: true,
mapbox_quality_mode: false,
dashboard_fx_from: 'EUR',
dashboard_fx_to: 'USD',
appearance: DEFAULT_APPEARANCE,
// dashboard_timezones is intentionally left unset so the widget can tell "never
// chosen" (fall back to home + defaults) from an explicitly emptied list.
},
settings: { ...DEFAULT_SETTINGS },
isLoaded: false,
loadSettings: async () => {