From 9f3e1f8c83d78ff99db024365730cec26d356ead Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 20:57:33 +0000 Subject: [PATCH] refactor(weather): share the day weather anchor and icon map between desktop and phone The day plan, the day panel, the phone timeline and the phone day sheet read the day local weather anchor from dayWeatherAnchor, and every weather icon comes from one map in weatherIcons. --- .../src/components/Planner/DayDetailPanel.tsx | 10 ++-- .../src/components/Planner/DayPlanSidebar.tsx | 6 +-- .../src/components/Weather/WeatherWidget.tsx | 16 +----- .../components/Weather/weatherIcons.test.ts | 35 +++++++++++++ client/src/components/Weather/weatherIcons.ts | 19 +++++++ .../screens/trip/plan/MPlanTimeline.tsx | 3 +- .../screens/trip/plan/planTimelineModel.ts | 11 ---- .../screens/trip/plan/useMPlanTimeline.ts | 14 ++--- .../mobile/screens/trip/sheets/MDaySheet.tsx | 14 +++-- client/src/pages/TripPlannerPage.tsx | 9 ++-- client/src/utils/dayWeather.test.ts | 51 +++++++++++++++++++ client/src/utils/dayWeather.ts | 32 ++++++++++++ .../mobile/trip/planTimelineModel.test.ts | 15 +----- 13 files changed, 164 insertions(+), 71 deletions(-) create mode 100644 client/src/components/Weather/weatherIcons.test.ts create mode 100644 client/src/components/Weather/weatherIcons.ts create mode 100644 client/src/utils/dayWeather.test.ts create mode 100644 client/src/utils/dayWeather.ts diff --git a/client/src/components/Planner/DayDetailPanel.tsx b/client/src/components/Planner/DayDetailPanel.tsx index 267999cd6..302425955 100644 --- a/client/src/components/Planner/DayDetailPanel.tsx +++ b/client/src/components/Planner/DayDetailPanel.tsx @@ -1,5 +1,5 @@ import React, { useState, useEffect, useRef, useId, type ReactNode } from 'react' -import { X, Sun, Cloud, CloudRain, CloudSnow, CloudDrizzle, CloudLightning, Wind, Droplets, Sunrise, Sunset, Hotel, Calendar, MapPin, LogIn, LogOut, Pencil, ChevronsDown, ChevronsUp, ArrowRight, Check, Moon, type LucideIcon } from 'lucide-react' +import { X, CloudRain, Wind, Droplets, Sunrise, Sunset, Hotel, Calendar, MapPin, LogIn, LogOut, Pencil, ChevronsDown, ChevronsUp, ArrowRight, Check, Moon, type LucideIcon } from 'lucide-react' import type { WeatherResult } from '@trek/shared' import { usePluginViewContributions, PluginCardFooter } from '../Plugins/PluginContributions' import { usePluginStore } from '../../store/pluginStore' @@ -24,14 +24,10 @@ import { INPUT, PANEL, EditorField, AddRowButton, PillSelect } from '../shared/d import { BOX, Eyebrow, Field, RoundAction, TypeTile, toneOf, toneColor, toneTint } from './bookings/bookingParts' import { SoftPill, TimePill, tintOf } from './planParts' import { typeInfo } from './bookings/bookingsModel' - -const WEATHER_ICON_MAP: Record = { - Clear: Sun, Clouds: Cloud, Rain: CloudRain, Drizzle: CloudDrizzle, - Thunderstorm: CloudLightning, Snow: CloudSnow, Mist: Wind, Fog: Wind, Haze: Wind, -} +import { weatherIconFor } from '../Weather/weatherIcons' function WIcon({ main, size = 14 }: { main: string; size?: number }) { - const Icon = WEATHER_ICON_MAP[main] || Cloud + const Icon = weatherIconFor(main) return } diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 9990bb092..187655c36 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -51,6 +51,7 @@ import { withinDriveRange } from '@trek/shared/roadtrip' import { formatDate, formatTime, formatMoneySum, splitReservationDateTime } from '../../utils/formatters' import { dayHeadingParts } from '../../utils/dayLabel' import { pendingStayIds } from '../../utils/pendingStays' +import { dayWeatherAnchor } from '../../utils/dayWeather' import { planCosts } from './planCosts' import { useDayNotes } from '../../hooks/useDayNotes' import { useExchangeRates } from '../../hooks/useExchangeRates' @@ -1806,10 +1807,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP // Day-local anchor only (#2167): the day's first located stop, else the // hotel you wake up in. No trip-wide fallback — on a roadtrip that // silently showed another city's weather with nothing naming the place. - const weatherHotel = loc == null ? dayBookends.morning : undefined - const wLat = loc?.place?.lat ?? weatherHotel?.place_lat - const wLng = loc?.place?.lng ?? weatherHotel?.place_lng - const weatherName = loc?.place?.name ?? weatherHotel?.place_name ?? null + const { lat: wLat, lng: wLng, name: weatherName } = dayWeatherAnchor(da, () => dayBookends.morning) const hasWeather = !!(day.date && wLat != null && wLng != null) const dayAccs = accommodations.filter(a => isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days)) // Sort: check-out first, then ongoing stays, then check-in last diff --git a/client/src/components/Weather/WeatherWidget.tsx b/client/src/components/Weather/WeatherWidget.tsx index f7d399ed8..6e4f07f0d 100644 --- a/client/src/components/Weather/WeatherWidget.tsx +++ b/client/src/components/Weather/WeatherWidget.tsx @@ -1,20 +1,8 @@ import { useState, useEffect } from 'react' -import { Sun, Cloud, CloudRain, CloudSnow, CloudDrizzle, CloudLightning, Wind } from 'lucide-react' import { fetchWeather } from '../../services/weatherQueue' import { useSettingsStore } from '../../store/settingsStore' import { Tooltip } from '../shared/Tooltip' - -const WEATHER_ICON_MAP = { - Clear: Sun, - Clouds: Cloud, - Rain: CloudRain, - Drizzle: CloudDrizzle, - Thunderstorm: CloudLightning, - Snow: CloudSnow, - Mist: Wind, - Fog: Wind, - Haze: Wind, -} +import { weatherIconFor } from './weatherIcons' interface WeatherIconProps { main: string @@ -22,7 +10,7 @@ interface WeatherIconProps { } function WeatherIcon({ main, size = 13 }: WeatherIconProps) { - const Icon = WEATHER_ICON_MAP[main] || Cloud + const Icon = weatherIconFor(main) return } diff --git a/client/src/components/Weather/weatherIcons.test.ts b/client/src/components/Weather/weatherIcons.test.ts new file mode 100644 index 000000000..80e31279e --- /dev/null +++ b/client/src/components/Weather/weatherIcons.test.ts @@ -0,0 +1,35 @@ +// FE-WEATHER-ICONS-001 to -002: the one condition-to-icon map behind the day plan's +// weather badge, the desktop day panel and the phone's timeline and day sheet. +// FE-WEATHER-ICONS-001 is the case FE-MOB-PTLM-041 held while the map lived in the +// phone's timeline model. +import { Cloud, CloudDrizzle, CloudLightning, CloudRain, CloudSnow, Sun, Wind } from 'lucide-react'; +import { describe, expect, it } from 'vitest'; + +import { WEATHER_ICON_MAP, weatherIconFor } from './weatherIcons'; + +describe('weatherIcons', () => { + it('FE-WEATHER-ICONS-001: maps the known conditions and defaults to a cloud', () => { + expect(weatherIconFor('Clear')).toBe(Sun); + expect(weatherIconFor('Rain')).toBe(CloudRain); + expect(weatherIconFor('Thunderstorm')).toBe(CloudLightning); + expect(weatherIconFor('Snow')).toBe(CloudSnow); + expect(weatherIconFor('Haze')).toBe(Wind); + expect(weatherIconFor('Tornado')).toBe(Cloud); + expect(weatherIconFor(undefined)).toBe(Cloud); + expect(weatherIconFor('')).toBe(Cloud); + }); + + it('FE-WEATHER-ICONS-002: covers every condition the forecast names', () => { + expect(WEATHER_ICON_MAP).toEqual({ + Clear: Sun, + Clouds: Cloud, + Rain: CloudRain, + Drizzle: CloudDrizzle, + Thunderstorm: CloudLightning, + Snow: CloudSnow, + Mist: Wind, + Fog: Wind, + Haze: Wind, + }); + }); +}); diff --git a/client/src/components/Weather/weatherIcons.ts b/client/src/components/Weather/weatherIcons.ts new file mode 100644 index 000000000..7aa47a95f --- /dev/null +++ b/client/src/components/Weather/weatherIcons.ts @@ -0,0 +1,19 @@ +import { Cloud, CloudDrizzle, CloudLightning, CloudRain, CloudSnow, Sun, Wind, type LucideIcon } from 'lucide-react'; + +/** The icon for each weather condition the forecast names. */ +export const WEATHER_ICON_MAP: Record = { + Clear: Sun, + Clouds: Cloud, + Rain: CloudRain, + Drizzle: CloudDrizzle, + Thunderstorm: CloudLightning, + Snow: CloudSnow, + Mist: Wind, + Fog: Wind, + Haze: Wind, +}; + +/** The icon for a forecast's condition; an unknown or missing one shows a cloud. */ +export function weatherIconFor(main: string | undefined): LucideIcon { + return (main && WEATHER_ICON_MAP[main]) || Cloud; +} diff --git a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx index a587f982e..d5dec8ed4 100644 --- a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx +++ b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx @@ -7,7 +7,8 @@ import { fmtTransitDuration } from '../../../../components/Planner/transitDispla import { routeModeIcon } from '../../../../components/Planner/routeModes' import { formatTime } from '../../../../utils/formatters' import { useMPlanTimeline, type MPlanTimelineController } from './useMPlanTimeline' -import { cityPillsForDay, weatherIconFor } from './planTimelineModel' +import { cityPillsForDay } from './planTimelineModel' +import { weatherIconFor } from '../../../../components/Weather/weatherIcons' import type { HotelChip, PlanRow } from './planTimelineModel' import { useMPlanDragReorder } from './useMPlanDragReorder' import { useTouchDragBridge } from '../../../../hooks/useTouchDragBridge' diff --git a/client/src/mobile/screens/trip/plan/planTimelineModel.ts b/client/src/mobile/screens/trip/plan/planTimelineModel.ts index 87696d063..76ea73304 100644 --- a/client/src/mobile/screens/trip/plan/planTimelineModel.ts +++ b/client/src/mobile/screens/trip/plan/planTimelineModel.ts @@ -1,5 +1,3 @@ -import { Cloud, CloudDrizzle, CloudLightning, CloudRain, CloudSnow, Sun, Wind } from 'lucide-react' -import type { LucideIcon } from 'lucide-react' import { TRANSPORT_TYPES, getAssignmentReservations, getDisplayTimeForDay, getSpanPhase, getTransportRouteEndpoints, hidesOnMiddleDay, parseTimeToMinutes, @@ -338,12 +336,3 @@ export function breaksChronology( .filter((m): m is number => m != null) return times.some((m, i) => i > 0 && m < times[i - 1]) } - -const WEATHER_ICON_MAP: Record = { - Clear: Sun, Clouds: Cloud, Rain: CloudRain, Drizzle: CloudDrizzle, - Thunderstorm: CloudLightning, Snow: CloudSnow, Mist: Wind, Fog: Wind, Haze: Wind, -} - -export function weatherIconFor(main: string | undefined): LucideIcon { - return (main && WEATHER_ICON_MAP[main]) || Cloud -} diff --git a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts index 768601bf0..432381cec 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanTimeline.ts @@ -4,6 +4,7 @@ import { useRouteCalculation } from '../../../../hooks/useRouteCalculation' import { assignmentsApi, reservationsApi, weatherApi } from '../../../../api/client' import { useRouteModeOptions } from '../../../../components/Planner/routeModes' import { getDayBookendHotels } from '../../../../utils/dayOrder' +import { dayWeatherAnchor } from '../../../../utils/dayWeather' import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport } from '../../../../utils/dayMerge' import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder, type DayCarrier } from '../lib/dayRoute' import { buildTransitLeg, buildTransitNameIndex, type TransitLeg } from '../../../../components/Planner/transitLeg' @@ -107,13 +108,12 @@ export function useMPlanTimeline(planner: TripPlanner) { // ── Weather chip — anchored to the day's first located stop, else its hotel ── const weatherAnchor = useMemo<{ lat: number; lng: number; name: string | null } | null>(() => { - const located = dayAssignments.find(a => a.place?.lat != null && a.place?.lng != null) - if (located) return { lat: located.place!.lat!, lng: located.place!.lng!, name: located.place!.name ?? null } - const hotel = day ? getDayBookendHotels(day, days, tripAccommodations).morning : undefined - if (hotel && hotel.place_lat != null && hotel.place_lng != null) { - return { lat: hotel.place_lat, lng: hotel.place_lng, name: hotel.place_name ?? null } - } - return null + const { lat, lng, name } = dayWeatherAnchor( + dayAssignments, + () => (day ? getDayBookendHotels(day, days, tripAccommodations).morning : undefined), + a => a.place?.lat != null && a.place?.lng != null, + ) + return lat != null && lng != null ? { lat, lng, name } : null }, [day, dayAssignments, days, tripAccommodations]) const [weather, setWeather] = useState(null) diff --git a/client/src/mobile/screens/trip/sheets/MDaySheet.tsx b/client/src/mobile/screens/trip/sheets/MDaySheet.tsx index 1a66b45d4..f745d2ba2 100644 --- a/client/src/mobile/screens/trip/sheets/MDaySheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MDaySheet.tsx @@ -18,7 +18,8 @@ import { dayCoMapsUrl, dayGoogleMapsUrl, optimizeDayOrder } from '../lib/dayRout import GoogleMapsIcon from '../../../../components/shared/GoogleMapsIcon' import { BlurredCode } from '../../../../components/shared/BookingCode' import { splitNoteTime } from '../lib/dayNotes' -import { weatherIconFor } from '../plan/planTimelineModel' +import { weatherIconFor } from '../../../../components/Weather/weatherIcons' +import { dayWeatherAnchor } from '../../../../utils/dayWeather' import type { Assignment, DayNote, Reservation } from '../../../../types' import { Eyebrow, INNER_CLS, StatBox, TileHeader, displayTime } from './MTripSheetUi' @@ -61,13 +62,10 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) { // hotel you wake up in (unconditional bookend lookup, mirroring useMPlanTimeline). // Never a place from another day — on a roadtrip that silently showed another // city's weather with nothing naming the place. - const locatedPlace = dayAssignments.find(a => a.place?.lat && a.place?.lng)?.place - const weatherHotel = day && !locatedPlace - ? getDayBookendHotels(day, planner.days, planner.tripAccommodations).morning - : undefined - const lat = locatedPlace?.lat ?? weatherHotel?.place_lat ?? null - const lng = locatedPlace?.lng ?? weatherHotel?.place_lng ?? null - const weatherPlaceName = locatedPlace?.name ?? weatherHotel?.place_name ?? null + const { lat, lng, name: weatherPlaceName } = dayWeatherAnchor( + dayAssignments, + () => (day ? getDayBookendHotels(day, planner.days, planner.tripAccommodations).morning : undefined), + ) const [weather, setWeather] = useState(null) const [weatherLoading, setWeatherLoading] = useState(false) diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 95a211db5..129559a1e 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -32,6 +32,7 @@ import type { BudgetItem } from '../types' import PluginFrame from '../components/Plugins/PluginFrame' import ErrorBoundary from '../components/shared/ErrorBoundary' import { getDayBookendHotels } from '../utils/dayOrder' +import { dayWeatherAnchor } from '../utils/dayWeather' import TripWarningsBanner from '../components/Planner/TripWarningsBanner' import Navbar from '../components/Layout/Navbar' import HelpAnchor from '../components/Help/HelpAnchor' @@ -538,11 +539,9 @@ function TripPlannerPageDesktop(): React.ReactElement | null { // Day-local weather anchor only (#2167): first located stop of THIS day, // else the hotel you wake up in (unconditional bookend lookup, mirroring // useMPlanTimeline) — never a place from another day. - const locatedPlace = dayAssignments.find(a => a.place?.lat && a.place?.lng)?.place - const weatherHotel = locatedPlace ? undefined : getDayBookendHotels(currentDay, days, tripAccommodations).morning - const weatherLat = locatedPlace?.lat ?? weatherHotel?.place_lat ?? null - const weatherLng = locatedPlace?.lng ?? weatherHotel?.place_lng ?? null - const weatherPlaceName = locatedPlace?.name ?? weatherHotel?.place_name ?? null + const { lat: weatherLat, lng: weatherLng, name: weatherPlaceName } = dayWeatherAnchor( + dayAssignments, () => getDayBookendHotels(currentDay, days, tripAccommodations).morning, + ) return ( + buildAssignment({ place: buildPlace({ lat, lng, name }) }); + +describe('dayWeatherAnchor', () => { + it('FE-UTIL-DAYWEATHER-001: the first stop with coordinates wins and the hotel is never looked up', () => { + const wakeUpHotel = vi.fn(() => HOTEL); + const anchor = dayWeatherAnchor( + [stop(null, null, 'Unlocated'), stop(48.1, 11.5, 'Museum'), stop(1, 2)], + wakeUpHotel + ); + expect(anchor).toEqual({ lat: 48.1, lng: 11.5, name: 'Museum' }); + expect(wakeUpHotel).not.toHaveBeenCalled(); + }); + + it('FE-UTIL-DAYWEATHER-002: without a located stop the hotel you wake up in stands in', () => { + expect(dayWeatherAnchor([stop(null, 3)], () => HOTEL)).toEqual({ lat: 46.2, lng: 7.1, name: 'Hotel Alpina' }); + expect(dayWeatherAnchor([], () => ({ place_lat: 1, place_lng: 2, place_name: null }))).toEqual({ + lat: 1, + lng: 2, + name: null, + }); + }); + + it('FE-UTIL-DAYWEATHER-003: neither a stop nor a hotel leaves the day without an anchor', () => { + expect(dayWeatherAnchor([stop(null, null)], () => undefined)).toEqual({ lat: null, lng: null, name: null }); + }); + + it('FE-UTIL-DAYWEATHER-004: by default a zero coordinate does not count as located', () => { + expect(hasWeatherCoords(stop(0, 9))).toBe(false); + expect(hasWeatherCoords(stop(9, 9))).toBe(true); + expect(hasWeatherCoords(buildAssignment({ place: undefined as never }))).toBe(false); + expect(dayWeatherAnchor([stop(0, 9, 'Equator'), stop(5, 6, 'Inland')], () => HOTEL).name).toBe('Inland'); + }); + + it('FE-UTIL-DAYWEATHER-005: the phone timeline counts any set coordinate, zero included', () => { + const anyCoords = (a: ReturnType) => a.place?.lat != null && a.place?.lng != null; + expect(dayWeatherAnchor([stop(0, 9, 'Equator'), stop(5, 6, 'Inland')], () => HOTEL, anyCoords)).toEqual({ + lat: 0, + lng: 9, + name: 'Equator', + }); + }); +}); diff --git a/client/src/utils/dayWeather.ts b/client/src/utils/dayWeather.ts new file mode 100644 index 000000000..da9bad525 --- /dev/null +++ b/client/src/utils/dayWeather.ts @@ -0,0 +1,32 @@ +import type { Accommodation, Assignment } from '../types'; + +/** Where a day's forecast is read, and the name the weather is captioned with. */ +export interface DayWeatherAnchor { + lat: number | null; + lng: number | null; + name: string | null; +} + +/** A stop the day plan reads the weather at: both coordinates set and neither of them zero. */ +export const hasWeatherCoords = (a: Assignment): boolean => !!(a.place?.lat && a.place?.lng); + +/** + * The day-local weather anchor (#2167): the day's first stop that `isLocated` + * accepts, else the hotel you wake up in. Never a place from another day: on a + * road trip that silently showed another city's weather with nothing naming the + * place. The hotel is only looked up when no stop qualifies, and where you wake up + * is a fact about the day, so callers look it up whatever the routing settings say. + */ +export function dayWeatherAnchor( + stops: Assignment[], + wakeUpHotel: () => Pick | undefined, + isLocated: (a: Assignment) => boolean = hasWeatherCoords +): DayWeatherAnchor { + const located = stops.find(isLocated)?.place; + const hotel = located ? undefined : wakeUpHotel(); + return { + lat: located?.lat ?? hotel?.place_lat ?? null, + lng: located?.lng ?? hotel?.place_lng ?? null, + name: located?.name ?? hotel?.place_name ?? null, + }; +} diff --git a/client/tests/unit/mobile/trip/planTimelineModel.test.ts b/client/tests/unit/mobile/trip/planTimelineModel.test.ts index ff8872b35..53b986b51 100644 --- a/client/tests/unit/mobile/trip/planTimelineModel.test.ts +++ b/client/tests/unit/mobile/trip/planTimelineModel.test.ts @@ -1,8 +1,7 @@ import { describe, expect, it, vi } from 'vitest' -import { Cloud, CloudLightning, CloudRain, CloudSnow, Sun, Wind } from 'lucide-react' import { breaksChronology, buildPlanRows, cityPillsForDay, findUpNext, getTransitMeta, hotelChipsForDay, - hotelLegsForDay, itemHasTime, parseReservationMeta, transportSubtitle, weatherIconFor, + hotelLegsForDay, itemHasTime, parseReservationMeta, transportSubtitle, type TransportEntry, } from '../../../../src/mobile/screens/trip/plan/planTimelineModel' import { getDisplayTimeForDay, getMergedItems, getTransportForDay, type MergedItem } from '../../../../src/utils/dayMerge' @@ -847,15 +846,3 @@ describe('planTimelineModel — chronology helpers', () => { expect(getDisplayTime).toHaveBeenCalledWith(bus.data, 2) }) }) - -describe('planTimelineModel — weatherIconFor', () => { - it('FE-MOB-PTLM-041: maps the known conditions and defaults to a cloud', () => { - expect(weatherIconFor('Clear')).toBe(Sun) - expect(weatherIconFor('Rain')).toBe(CloudRain) - expect(weatherIconFor('Thunderstorm')).toBe(CloudLightning) - expect(weatherIconFor('Snow')).toBe(CloudSnow) - expect(weatherIconFor('Haze')).toBe(Wind) - expect(weatherIconFor('Tornado')).toBe(Cloud) - expect(weatherIconFor(undefined)).toBe(Cloud) - }) -})