mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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:
@@ -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 (
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user