mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26: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 React, { useState, useEffect, useRef } from 'react'
|
||||||
import { Languages, Map, ChevronDown, Check } from 'lucide-react'
|
import { Languages, Map, ChevronDown, Check } from 'lucide-react'
|
||||||
import { SUPPORTED_LANGUAGES, useTranslation } from '../../i18n'
|
import { SUPPORTED_LANGUAGES, useTranslation } from '../../i18n'
|
||||||
import { useSettingsStore } from '../../store/settingsStore'
|
import { useSettingsStore, DEFAULT_SETTINGS } from '../../store/settingsStore'
|
||||||
import { useToast } from '../shared/Toast'
|
import { useToast } from '../shared/Toast'
|
||||||
import CustomSelect from '../shared/CustomSelect'
|
import CustomSelect from '../shared/CustomSelect'
|
||||||
import { SYMBOLS, currenciesWith } from '../Budget/BudgetPanel.constants'
|
import { SYMBOLS, currenciesWith } from '../Budget/BudgetPanel.constants'
|
||||||
@@ -12,8 +12,8 @@ export default function DisplaySettingsTab(): React.ReactElement {
|
|||||||
const { settings, updateSetting } = useSettingsStore()
|
const { settings, updateSetting } = useSettingsStore()
|
||||||
const { t } = useTranslation()
|
const { t } = useTranslation()
|
||||||
const toast = useToast()
|
const toast = useToast()
|
||||||
const [tempUnit, setTempUnit] = useState<string>(settings.temperature_unit || 'celsius')
|
const [tempUnit, setTempUnit] = useState<string>(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit)
|
||||||
const [distanceUnit, setDistanceUnit] = useState<DistanceUnit>(settings.distance_unit || 'metric')
|
const [distanceUnit, setDistanceUnit] = useState<DistanceUnit>(settings.distance_unit || DEFAULT_SETTINGS.distance_unit)
|
||||||
const [langOpen, setLangOpen] = useState(false)
|
const [langOpen, setLangOpen] = useState(false)
|
||||||
const langDropdownRef = useRef<HTMLDivElement | null>(null)
|
const langDropdownRef = useRef<HTMLDivElement | null>(null)
|
||||||
|
|
||||||
@@ -27,11 +27,11 @@ export default function DisplaySettingsTab(): React.ReactElement {
|
|||||||
}, [langOpen])
|
}, [langOpen])
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setTempUnit(settings.temperature_unit || 'celsius')
|
setTempUnit(settings.temperature_unit || DEFAULT_SETTINGS.temperature_unit)
|
||||||
}, [settings.temperature_unit])
|
}, [settings.temperature_unit])
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setDistanceUnit(settings.distance_unit || 'metric')
|
setDistanceUnit(settings.distance_unit || DEFAULT_SETTINGS.distance_unit)
|
||||||
}, [settings.distance_unit])
|
}, [settings.distance_unit])
|
||||||
|
|
||||||
return (
|
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 =>
|
export const hasStoredLanguage = (): boolean =>
|
||||||
typeof localStorage !== 'undefined' && !!localStorage.getItem('app_language')
|
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) => ({
|
export const useSettingsStore = create<SettingsState>((set, get) => ({
|
||||||
settings: {
|
settings: { ...DEFAULT_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.
|
|
||||||
},
|
|
||||||
isLoaded: false,
|
isLoaded: false,
|
||||||
|
|
||||||
loadSettings: async () => {
|
loadSettings: async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user