fix: address review feedback on the distance unit setting

- server: allow distance_unit as an admin default (+ value validation) so the Admin "Default User Settings" toggle persists instead of returning 400 - i18n: add settings.distance to all 20 locales and translate the labels through t() instead of hardcoding "Distance" - route legs: include the unit in the OSRM cache key and recompute on a unit switch, so map and sidebar distances refresh and never mix units - keep wind speed tied to the temperature unit — a distance setting must not silently flip existing Fahrenheit users from mph to km/h - restore the sub-1km metres reading for metric, convert GPX elevation to feet for imperial, and format distances with a '.' decimal in every locale - add units.test.ts
2026-06-30 18:46:00 +00:00 · 2026-06-26 17:45:56 +02:00
parent 94dca8cad7
commit 3d65bb0c12
32 changed files with 109 additions and 23 deletions
@@ -214,7 +214,7 @@ export default function DefaultUserSettingsTab(): React.ReactElement {
      </OptionRow>

      {/* Distance */}
-      <OptionRow label={<>Distance <ResetButton field="distance_unit" /></>}>
+      <OptionRow label={<>{t('settings.distance')} <ResetButton field="distance_unit" /></>}>
        {([
          { value: 'metric', label: 'km Metric' },
          { value: 'imperial', label: 'mi Imperial' },
@@ -240,7 +240,9 @@ export async function calculateRouteWithLegs(
  }

  const coords = waypoints.map((p) => `${p.lng},${p.lat}`).join(';')
-  const cacheKey = `${profile}:${coords}`
+  // The cached result carries formatted leg distances, so the active distance unit is
+  // part of the key — otherwise switching km↔mi would return stale text (#1300).
+  const cacheKey = `${profile}:${getDistanceUnit()}:${coords}`
  const cached = routeCache.get(cacheKey)
  if (cached) return cached

@@ -402,9 +402,9 @@ describe('DayDetailPanel', () => {
    await screen.findByText('20:15');
  });

-  it('FE-PLANNER-DAYDETAIL-027: weather chips show imperial wind speed', async () => {
+  it('FE-PLANNER-DAYDETAIL-027: weather chips show Fahrenheit wind speed', async () => {
    seedStore(useSettingsStore, {
-      settings: { time_format: '24h', temperature_unit: 'celsius', distance_unit: 'imperial', blur_booking_codes: false },
+      settings: { time_format: '24h', temperature_unit: 'fahrenheit', blur_booking_codes: false },
    });
    server.use(
      http.get('/api/weather/detailed', () =>
@@ -14,7 +14,6 @@ import { getLocaleForLanguage, useTranslation } from '../../i18n'
 import type { Day, Place, Category, Reservation, AssignmentsMap } from '../../types'
 import { isDayInAccommodationRange } from '../../utils/dayOrder'
 import { splitReservationDateTime } from '../../utils/formatters'
-import { convertDistance } from '../../utils/units'
 import { useDayDetail } from './useDayDetail'

 const WEATHER_ICON_MAP = {
@@ -69,7 +68,6 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri
  const tripObj = useTripStore((s) => s.trip)
  const canEditDays = can('day_edit', tripObj)
  const isFahrenheit = useSettingsStore(s => s.settings.temperature_unit) === 'fahrenheit'
-  const distanceUnit = useSettingsStore(s => s.settings.distance_unit) || 'metric'
  const is12h = useSettingsStore(s => s.settings.time_format) === '12h'
  const blurCodes = useSettingsStore(s => s.settings.blur_booking_codes)
  const fmtTime = (v) => {
@@ -78,8 +76,6 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri
    return formatTime12(v, is12h)
  }
  const unit = isFahrenheit ? '°F' : '°C'
-  const formatWindSpeed = (kmh: number) =>
-    distanceUnit === 'imperial' ? `${Math.round(convertDistance(kmh, distanceUnit))} mph` : `${Math.round(kmh)} km/h`
  const collapsed = collapsedProp
  const toggleCollapse = () => onToggleCollapse?.()
  const {
@@ -176,7 +172,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri
                    <Chip icon={CloudRain} value={`${weather.precipitation_sum.toFixed(1)} mm`} />
                  )}
                  {weather.wind_max != null && (
-                    <Chip icon={Wind} value={formatWindSpeed(weather.wind_max)} />
+                    <Chip icon={Wind} value={isFahrenheit ? `${Math.round(weather.wind_max * 0.621371)} mph` : `${Math.round(weather.wind_max)} km/h`} />
                  )}
                  {weather.sunrise && <Chip icon={Sunrise} value={weather.sunrise} />}
                  {weather.sunset && <Chip icon={Sunset} value={weather.sunset} />}
@@ -154,6 +154,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
  const [routeLegs, setRouteLegs] = useState<Record<number, RouteSegment>>({})
  const [hotelLegs, setHotelLegs] = useState<{ top?: { seg: RouteSegment; name: string }; bottom?: { seg: RouteSegment; name: string } }>({})
  const optimizeFromAccommodation = useSettingsStore(s => s.settings.optimize_from_accommodation)
+  // Recompute the hotel/route legs when the user flips km↔mi so the connector
+  // distances refresh instead of showing stale cached text (#1300).
+  const distanceUnit = useSettingsStore(s => s.settings.distance_unit)
  const legsAbortRef = useRef<AbortController | null>(null)
  const [draggingId, setDraggingId] = useState(null)
  const [lockedIds, setLockedIds] = useState(new Set())
@@ -470,7 +473,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {

      if (!controller.signal.aborted) { setRouteLegs(map); setHotelLegs(hotel) }
    })()
-  }, [selectedDayId, routeShown, routeProfile, mergedItemsMap, accommodations, days, optimizeFromAccommodation])
+  }, [selectedDayId, routeShown, routeProfile, mergedItemsMap, accommodations, days, optimizeFromAccommodation, distanceUnit])

  const openAddNote = (dayId, e) => {
    e?.stopPropagation()
@@ -12,7 +12,7 @@ import { useToast } from '../shared/Toast'
 import { useTranslation } from '../../i18n'
 import type { Place, Category, Day, Assignment, Reservation, TripFile, AssignmentsMap } from '../../types'
 import { splitReservationDateTime, formatTime } from '../../utils/formatters'
-import { formatDistance } from '../../utils/units'
+import { formatDistance, formatElevation } from '../../utils/units'

 const detailsCache = new Map()

@@ -784,14 +784,14 @@ function PlaceExtras({ openingHours, weekdayIndex, hoursExpanded, setHoursExpand
                      <>
                        <div className="text-content" style={{ display: 'flex', alignItems: 'center', gap: 4, fontSize: 12, fontWeight: 600 }}>
                          <Mountain size={12} color="#22c55e" />
-                          {Math.round(maxEle)} m
+                          {formatElevation(maxEle, distanceUnit)}
                        </div>
                        <div className="text-content" style={{ display: 'flex', alignItems: 'center', gap: 4, fontSize: 12, fontWeight: 600 }}>
                          <Mountain size={12} color="#ef4444" />
-                          {Math.round(minEle)} m
+                          {formatElevation(minEle, distanceUnit)}
                        </div>
                        <div className="text-content-muted" style={{ fontSize: 12 }}>
-                          ↑{Math.round(totalUp)} m &nbsp;↓{Math.round(totalDown)} m
+                          ↑{formatElevation(totalUp, distanceUnit)} &nbsp;↓{formatElevation(totalDown, distanceUnit)}
                        </div>
                      </>
                    )}
@@ -208,7 +208,7 @@ export default function DisplaySettingsTab(): React.ReactElement {

      {/* Distance */}
      <div>
-        <label className="block text-sm font-medium mb-2 text-content-secondary">Distance</label>
+        <label className="block text-sm font-medium mb-2 text-content-secondary">{t('settings.distance')}</label>
        <div className="flex gap-3">
          {([
            { value: 'metric', label: 'km Metric' },
@@ -25,6 +25,9 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu
  // Draw the day's accommodation bookend legs (hotel → first stop, last stop →
  // hotel) unless the user turned the setting off — same gate as the sidebar.
  const optimizeFromAccommodation = useSettingsStore((s) => s.settings.optimize_from_accommodation)
+  // Recompute when the user flips km↔mi so leg distances (formatted at compute time)
+  // refresh instead of showing stale cached text (#1300).
+  const distanceUnit = useSettingsStore((s) => s.settings.distance_unit)

  const updateRouteForDay = useCallback(async (dayId: number | null) => {
    if (routeAbortRef.current) routeAbortRef.current.abort()
@@ -164,7 +167,7 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu
      // Aborted (day changed) — newer call owns the state. Anything else: keep straight lines.
      if (!(err instanceof Error) || err.name !== 'AbortError') setRouteSegments([])
    }
-  }, [enabled, profile, accommodations, optimizeFromAccommodation])
+  }, [enabled, profile, accommodations, optimizeFromAccommodation, distanceUnit])

  // Stable signature for transport reservations on the selected day — changes when a transport
  // is added, removed, or repositioned, ensuring route recalc fires even on transport-only reorders.
@@ -188,7 +191,7 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu
    if (!selectedDayId) { setRoute(null); setRouteSegments([]); return }
    updateRouteForDay(selectedDayId)
  // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [selectedDayId, selectedDayAssignments, transportSignature, enabled, profile, accommodations, optimizeFromAccommodation])
+  }, [selectedDayId, selectedDayAssignments, transportSignature, enabled, profile, accommodations, optimizeFromAccommodation, distanceUnit])

  return { route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay }
 }
@@ -361,12 +361,13 @@ function BoardingPassHero({ trip, bundle, locale, onOpen, onEdit, onCopy, onArch
 // ── Atlas / stats row ────────────────────────────────────────────────────────
 function formatCompactDistance(value: number): string {
  const safeValue = Number.isFinite(value) ? Math.max(0, value) : 0
+  // String() keeps a '.' decimal regardless of locale (no "1,5k" in non-English UIs).
  if (safeValue >= 1000) {
-    return `${(safeValue / 1000).toLocaleString(undefined, { maximumFractionDigits: 1 })}k`
+    return `${String(Math.round(safeValue / 100) / 10)}k`
  }
  const rounded = Math.round(safeValue * 10) / 10
  if (safeValue > 0 && rounded === 0) return '<0.1'
-  return rounded.toLocaleString(undefined, { maximumFractionDigits: 1 })
+  return String(rounded)
 }

 function AtlasStats({ stats }: { stats: TravelStats | null }): React.ReactElement {
@@ -0,0 +1,46 @@
+import { describe, it, expect } from 'vitest'
+import { convertDistance, formatDistance, getDistanceUnitLabel } from './units'
+
+describe('units', () => {
+  describe('getDistanceUnitLabel', () => {
+    it('returns km for metric and mi for imperial', () => {
+      expect(getDistanceUnitLabel('metric')).toBe('km')
+      expect(getDistanceUnitLabel('imperial')).toBe('mi')
+    })
+  })
+
+  describe('convertDistance', () => {
+    it('keeps kilometres for metric', () => {
+      expect(convertDistance(10, 'metric')).toBe(10)
+    })
+    it('converts kilometres to miles for imperial', () => {
+      expect(convertDistance(10, 'imperial')).toBeCloseTo(6.21371, 4)
+    })
+    it('clamps negative and non-finite input to 0', () => {
+      expect(convertDistance(-5, 'imperial')).toBe(0)
+      expect(convertDistance(NaN, 'metric')).toBe(0)
+      expect(convertDistance(Infinity, 'metric')).toBe(0)
+    })
+  })
+
+  describe('formatDistance', () => {
+    it('shows metres below 1 km for metric', () => {
+      expect(formatDistance(0.3, 'metric')).toBe('300 m')
+      expect(formatDistance(0.05, 'metric')).toBe('50 m')
+    })
+    it('shows kilometres at or above 1 km for metric', () => {
+      expect(formatDistance(1.5, 'metric')).toBe('1.5 km')
+      expect(formatDistance(10, 'metric')).toBe('10 km')
+    })
+    it('shows miles for imperial', () => {
+      expect(formatDistance(10, 'imperial')).toBe('6.2 mi')
+    })
+    it('shows <0.1 for a tiny imperial distance', () => {
+      expect(formatDistance(0.05, 'imperial')).toBe('<0.1 mi')
+    })
+    it('clamps negative and non-finite input to 0', () => {
+      expect(formatDistance(-1, 'metric')).toBe('0 m')
+      expect(formatDistance(NaN, 'imperial')).toBe('0 mi')
+    })
+  })
+})
@@ -1,22 +1,35 @@
 import type { DistanceUnit } from '../types'

 const KM_TO_MI = 0.621371
+const M_TO_FT = 3.28084

 export function getDistanceUnitLabel(unit: DistanceUnit): 'km' | 'mi' {
  return unit === 'imperial' ? 'mi' : 'km'
 }

+/** Formats an elevation in metres as feet for imperial, so it doesn't mix with mi distances. */
+export function formatElevation(meters: number, unit: DistanceUnit): string {
+  const safe = Number.isFinite(meters) ? meters : 0
+  return unit === 'imperial' ? `${Math.round(safe * M_TO_FT)} ft` : `${Math.round(safe)} m`
+}
+
 export function convertDistance(km: number, unit: DistanceUnit): number {
  const safeKm = Number.isFinite(km) ? Math.max(0, km) : 0
  return unit === 'imperial' ? safeKm * KM_TO_MI : safeKm
 }

 export function formatDistance(km: number, unit: DistanceUnit): string {
-  const value = convertDistance(km, unit)
+  const safeKm = Number.isFinite(km) ? Math.max(0, km) : 0
+  // Metric keeps a metres reading below 1 km (e.g. "300 m"), matching the route
+  // connectors; imperial has no sub-mile unit, so short hops just show "0.x mi".
+  if (unit === 'metric' && safeKm < 1) {
+    return `${Math.round(safeKm * 1000)} m`
+  }
+  const value = convertDistance(safeKm, unit)
  const label = getDistanceUnitLabel(unit)
  const rounded = Math.round(value * 10) / 10
-  const text = value > 0 && rounded === 0
-    ? '<0.1'
-    : rounded.toLocaleString(undefined, { maximumFractionDigits: 1 })
+  // String() keeps a '.' decimal regardless of locale, matching the rest of the app
+  // (toFixed elsewhere) and avoiding "1,5 km" in non-English environments.
+  const text = value > 0 && rounded === 0 ? '<0.1' : String(rounded)
  return `${text} ${label}`
 }