From 9de18634fe057fa72e05d1f5dda8894c3fb50d85 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 16 Sep 2026 22:49:44 +0000 Subject: [PATCH] fix(roadtrip): a place tapped on the phone stage map opens its stop The bar over the road trip map names a place, but a tap on it switched to the whole day's chain and the place never opened. It now opens that stop's sheet, the one its row in the chain opens. Its name, clock and tap come off one row (stageEnd), so the bar no longer pairs the destination with the automatic day end's clock (Kyoto Station 22:00 while the head card said 12:40). The header's list switch stays the way back to the chain. A pin on the stage went through the plan tab's marker click, which moves the place selection and brought up the plan tab's place card. On the road trip tab a pin now opens its stop on the stage card, or the first routed visit in the all days view, and only a place no routed day stops at still gets the place card. The handler keeps one identity for the life of the map. "Show on map" in the stop sheet and the up next card move the camera through focusRoadtripPoint instead of the selection, picking the stop's card first when it is not on screen. A pending focus now lets go once the day moves, so later stages frame their own drive again. Stage pins come from the trip store instead of the plan tab's map list. That list carries the plan tab's day declutter and place filters, so after the all days switch and back most pins of the next stage were missing, and the unplanned filter or hidden service stops removed stops the chain draws. --- .../Roadtrip/roadtripRowModel.test.ts | 52 +++++- .../components/Roadtrip/roadtripRowModel.ts | 42 +++++ .../src/mobile/screens/trip/map/MMapArea.tsx | 92 +++++++-- .../screens/trip/roadtrip/MRoadtripTab.tsx | 55 +++--- .../screens/trip/roadtrip/MRtStopSheet.tsx | 7 +- .../screens/trip/roadtrip/useMRoadtrip.ts | 36 +++- .../tests/unit/mobile/trip/MMapArea.test.tsx | 175 +++++++++++++++++- .../unit/mobile/trip/MRoadtripTab.test.tsx | 64 ++++++- .../unit/mobile/trip/MRtStopSheet.test.tsx | 32 +++- 9 files changed, 493 insertions(+), 62 deletions(-) diff --git a/client/src/components/Roadtrip/roadtripRowModel.test.ts b/client/src/components/Roadtrip/roadtripRowModel.test.ts index c7f19fda0..1b4f59b0d 100644 --- a/client/src/components/Roadtrip/roadtripRowModel.test.ts +++ b/client/src/components/Roadtrip/roadtripRowModel.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it } from 'vitest' import { destinationCount, + firstStopOfPlace, pickWarning, roadtripRows, stageClocks, + stageEnd, stageOf, upNextStop, type RoadtripRow, @@ -12,7 +14,7 @@ import { import type { ScheduleEntry, ScheduleWarning } from './roadtripModel' import type { RoadtripDay, RoadtripStop, RouteSegment } from '@trek/shared/roadtrip' -// FE-RTROW-001 to FE-RTROW-033 +// FE-RTROW-001 to FE-RTROW-037 function stop(name: string, over: Partial = {}): RoadtripStop { return { @@ -393,3 +395,51 @@ describe('stageClocks', () => { expect(stageClocks(roadtripRows(unsplit))).toEqual({ start: '18:30', arrive: '10:00' }) }) }) + +describe('stageEnd', () => { + it('FE-RTROW-034: names the last stop the chain draws with its own arrival, not the automatic day end after it', () => { + // The reported bar: Kyoto Station reached at 12:40 sat beside 22:00, where the window closed. + const d = day([stop('Bremen'), stop('Aral', { stopType: 'fuel' }), stop('Kyoto Station'), night()], { + schedule: { entries: [entry('09:00'), entry('10:30'), entry('12:40'), entry('22:00')], warnings: [] }, + }) + const end = stageEnd(roadtripRows(d)) + expect(end?.stop.name).toBe('Kyoto Station') + expect(end?.time).toBe('12:40') + + // A service stop is still a stop the drive ends at, so it is the one named. + const fuelLast = timedDay(['09:00', '11:00'], [stop('A'), stop('Aral', { stopType: 'fuel' })]) + expect(stageEnd(roadtripRows(fuelLast))?.stop.name).toBe('Aral') + }) + + it('FE-RTROW-035: a stage drawn with nothing but its night markers, and no rows at all, end nowhere', () => { + expect(stageEnd(roadtripRows(day([night('start'), night('end')])))).toBeNull() + expect(stageEnd([])).toBeNull() + }) +}) + +describe('firstStopOfPlace', () => { + it('FE-RTROW-036: on one card, a place visited twice answers with its first visit and a night on it never does', () => { + // A loop day: out of the hotel in the morning, back to it at night. + const loop = day([ + stop('Nacht', { placeId: 300, assignmentId: 90, automaticNight: { phase: 'start', fromDayNumber: 1 } }), + stop('Hotel', { placeId: 300, assignmentId: 1 }), + stop('Museum', { placeId: 301, assignmentId: 2 }), + stop('Hotel', { placeId: 300, assignmentId: 3 }), + ]) + expect(firstStopOfPlace([loop], 300)?.assignmentId).toBe(1) + expect(firstStopOfPlace([loop], 301)?.assignmentId).toBe(2) + }) + + it('FE-RTROW-037: across days it keeps day order, a card handed alone ignores the days before it, and no stop gives null', () => { + const monday = day([stop('Town', { placeId: 301, assignmentId: 11 })], { dayId: 7 }) + const tuesday = day([stop('Town', { placeId: 301, assignmentId: 21 }), stop('Lake', { placeId: 302, assignmentId: 22 })], { dayId: 8 }) + expect(firstStopOfPlace([monday, tuesday], 301)?.assignmentId).toBe(11) + expect(firstStopOfPlace([tuesday], 301)?.assignmentId).toBe(21) + expect(firstStopOfPlace([monday, tuesday], 302)?.assignmentId).toBe(22) + + expect(firstStopOfPlace([monday, tuesday], 999)).toBeNull() + expect(firstStopOfPlace([], 301)).toBeNull() + // Only an automatic night sits on this position, which is not a stop anybody chose. + expect(firstStopOfPlace([day([stop('Nacht', { placeId: 303, automaticNight: { phase: 'end', fromDayNumber: 1 } })])], 303)).toBeNull() + }) +}) diff --git a/client/src/components/Roadtrip/roadtripRowModel.ts b/client/src/components/Roadtrip/roadtripRowModel.ts index e01600551..2d925d9b2 100644 --- a/client/src/components/Roadtrip/roadtripRowModel.ts +++ b/client/src/components/Roadtrip/roadtripRowModel.ts @@ -197,6 +197,48 @@ export function stageClocks(rows: readonly RoadtripRow[]): { start: string | nul return { start, arrive } } +/** + * The stop a stage ends on: the last one the chain draws, a service stop included. + * + * The map half names it in its bar and opens it on a tap, so the name, the clock beside it + * and the sheet the tap brings up all come off this one row and cannot name three different + * things. The clock used to be read off the schedule on its own, which found the automatic + * day end whenever the window closed after the last stop: a station reached at 12:40 sat + * beside 22:00, the moment the day ran out rather than anything that happens there. + * + * Null for a stage without a single stop, one drawn with nothing but its night markers. + * There is nothing for a tap to open then, and the bar says so by not being a button. + */ +export function stageEnd(rows: readonly RoadtripRow[]): StopRow | null { + for (let i = rows.length - 1; i >= 0; i--) { + const row = rows[i] + if (row.kind === 'stop') return row + } + return null +} + +/** + * The stop a place's pin stands for on the road trip map: its first visit in the days given. + * + * A pin is drawn once per place, while a place can be several stops, like the hotel a loop + * day leaves in the morning and comes back to at night, or a town the drive passes on two + * days. Over a stage the caller hands in that one card, so a pin opens the visit on the + * stage being looked at, however many days before it stop at the same place. Over the + * whole drive it hands in every routed day, and the first visit in day order is the one a + * traveller reading the drive from its start reaches first. + * + * An automatic night can sit on a place's position, but nobody chose to stop there, so it + * never answers. Null when none of the days stops at the place, which is a pin the road + * trip cannot explain and the caller hands to the plan's own place inspector instead. + */ +export function firstStopOfPlace(days: readonly RoadtripDay[], placeId: number): RoadtripStop | null { + for (const day of days) { + const stop = day.stops.find(s => !s.automaticNight && s.placeId === placeId) + if (stop) return stop + } + return null +} + /** * The next destination the plan still owes you, measured against the clock. * diff --git a/client/src/mobile/screens/trip/map/MMapArea.tsx b/client/src/mobile/screens/trip/map/MMapArea.tsx index d00c47cf2..23a0ad7fd 100644 --- a/client/src/mobile/screens/trip/map/MMapArea.tsx +++ b/client/src/mobile/screens/trip/map/MMapArea.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react' +import { useCallback, useMemo, useRef, useState } from 'react' import { MapViewAuto } from '../../../../components/Map/MapViewAuto' import { MapCompassPill, type CompassMap } from '../../../../components/Map/MapCompassPill' import { MAP_LAYER_SWITCHER_INSET, MAP_ROUND_CONTROL_SIZE } from '../../../../components/Map/MapLayerSwitcher' @@ -7,10 +7,11 @@ import { DawarichTrailPill } from '../../../../components/Map/DawarichTrailPill' import PoiCategoryPill from '../../../../components/Map/PoiCategoryPill' import { usePoiExplore } from '../../../../components/Map/usePoiExplore' import { useMergedMapPois } from '../../../../components/Map/useMergedMapPois' -import { stageOf } from '../../../../components/Roadtrip/roadtripRowModel' +import { firstStopOfPlace, stageOf } from '../../../../components/Roadtrip/roadtripRowModel' import { stageMapData } from '../../../../components/Roadtrip/stageMap' import { useRoadtripSettings } from '../../../../hooks/useRoadtripSettings' import { useSettingsStore } from '../../../../store/settingsStore' +import { useTripStore } from '../../../../store/tripStore' import type { MMapAreaProps } from '../MTripShell' import type { Poi } from '../../../../components/Map/poiCategories' @@ -24,6 +25,29 @@ const NO_POIS: Poi[] = [] */ const COMPASS_LEFT = MAP_LAYER_SWITCHER_INSET + MAP_ROUND_CONTROL_SIZE + 8 +/** A camera focus the planner is holding, and the day that was on screen when it arrived. */ +interface HeldFocus { + points: readonly [number, number][] + dayId: number | null + live: boolean +} + +/** + * Lets go of a pending focus once the day moves off the one it arrived with. + * + * The planner keeps a point it was asked to show until its next routing round, whatever + * day is picked in the meantime, and a pending focus wins over the stage's own frame. So + * after "show on map" every stage swiped to afterwards stayed on that one point instead of + * framing its drive. Coming back to the day does not bring the point back either: the + * traveller has moved on since, and the camera belongs to the stage they came back to. A + * new focus is a new array, held again from the day it arrives together with. + */ +function holdFocus(prev: HeldFocus, points: readonly [number, number][], dayId: number | null): HeldFocus { + if (prev.points !== points) return { points, dayId, live: true } + if (prev.live && prev.dayId !== dayId) return { points, dayId, live: false } + return prev +} + /** * Fullscreen map layer of the mobile trip screen (plan tab). Stays mounted for * the whole plan-tab lifetime — the plan timeline / places browser overlays @@ -40,9 +64,10 @@ const COMPASS_LEFT = MAP_LAYER_SWITCHER_INSET + MAP_ROUND_CONTROL_SIZE + 8 * button on the right, all riding the --bottom-nav-h contract the map already reads * so they cannot drift apart. * - * Marker data honours the shared places category filter (#1541) because - * planner.mapPlaces is derived from tripStore's placesCategoryFilter — the - * same set the places browser renders, so the two can't desync. + * On the plan tab, marker data honours the shared places category filter (#1541) + * because planner.mapPlaces is derived from tripStore's placesCategoryFilter, the + * same set the places browser renders, so the two can't desync. The road trip + * stage draws the stops of its chain instead, see `stagePlaces`. */ export default function MMapArea({ planner, shell }: MMapAreaProps) { const poi = usePoiExplore() @@ -51,6 +76,7 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { const distanceUnit = useSettingsStore(s => s.settings.distance_unit) const dayColorsOn = useRoadtripSettings(s => s.roadtrip_day_colors, planner.tripId) + const tripPlaces = useTripStore(s => s.places) // One instance, two tabs. `mapFront` is true whenever the map is the front layer // in either of them, so the floating chrome below keys off that rather than off @@ -80,13 +106,56 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { // Only what the stage carries, so a trip's other 200 pins stay off a screen that // is answering one question. Without a stage (the all-days view) every planned // place comes back, which is what the drive looks like end to end. + // + // Out of the trip store rather than `planner.mapPlaces`, which is the plan tab's map. + // That list drops the pins of every day its declutter has put away, and this tab moves + // the day without touching the declutter (a swipe, or a chip tap while the plan tab sits + // on its list): after the all days switch and back, the next stage picked came up with + // most of its pins missing. It also runs the places browser's filters, where 'unplanned' + // hides every stop a drive has, and it lacks the service stops hidden from the day lists + // that the chain still draws. The category filter stays off the stage too, on purpose: + // the stage is one day's chain on a map, and a stop the chain lists with no pin under its + // line, hidden by a control on another tab, reads as a broken map rather than a filter. const stagePlaces = useMemo( () => stageMap && stage - ? planner.mapPlaces.filter(p => stageMap.placeIds.has(p.id)) + ? tripPlaces.filter(p => p.lat != null && p.lng != null && stageMap.placeIds.has(p.id)) : planner.roadtripMapPlaces, - [stageMap, stage, planner.mapPlaces, planner.roadtripMapPlaces], + [stageMap, stage, tripPlaces, planner.roadtripMapPlaces], ) + // Computed during render rather than in an effect, so the frame handed over below is + // already the right one in the render a day change happens in. See holdFocus. + const [heldFocus, setHeldFocus] = useState( + () => ({ points: planner.mapFocusPoints, dayId: planner.selectedDayId, live: true }), + ) + const focus = holdFocus(heldFocus, planner.mapFocusPoints, planner.selectedDayId) + if (focus !== heldFocus) setHeldFocus(focus) + const focusPending = focus.live && planner.mapFocusPoints.length > 0 + + /** + * A pin on the road trip tab opens its stop: the sheet the chain row opens, not the + * place inspector. + * + * The plan tab's marker click moves the planner's place selection, and the inspector + * opens off that selection, so a tap on a stop of the drive came up with the plan tab's + * card over the map and no word about the stop. Over a stage the stop is found on that + * card, over the whole drive on the first routed day that stops there (firstStopOfPlace). + * A pin no routed day stops at still gets the inspector: the road trip has nothing to + * say about it, and the place does. + * + * One handler for the life of the map, reading the latest values through a ref. Leaflet + * rebuilds every marker whenever this identity moves, and the shell re-renders on every + * store write and on every sheet it opens, this handler's own included. + */ + const latest = useRef({ planner, shell, stage }) + latest.current = { planner, shell, stage } + const openStagePin = useCallback((placeId?: number) => { + const { planner: now, shell: chrome, stage: card } = latest.current + const stop = placeId == null ? null : firstStopOfPlace(card ? [card] : now.roadtripRoutes.days, placeId) + if (stop) chrome.openSheet('rtstop', { dayId: stop.ownerDayId, assignmentId: stop.assignmentId }) + else now.handleMarkerClick(placeId) + }, []) + /** * The pins, and they are not the same question on the two tabs. * @@ -131,10 +200,11 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { route={stageMap ? stageMap.lines : planner.overviewActive ? planner.tripOverview.lines : planner.route} routeColors={stageMap ? stageMap.lineColors : planner.overviewActive ? planner.tripOverview.lineColors : undefined} accessLines={stageMap ? stageMap.accessLines : undefined} - // A hit somebody tapped in the search sheet, or the stations the fuel search is - // offering, take the camera; with nothing pending the stage frames itself. + // A hit somebody tapped in the search sheet, a stop shown from its sheet, or the + // stations the fuel search is offering take the camera while their day is on + // screen; with nothing pending the stage frames itself. focusPoints={stageMap - ? (planner.mapFocusPoints.length ? planner.mapFocusPoints : stageMap.focusPoints) + ? (focusPending ? planner.mapFocusPoints : stageMap.focusPoints) : planner.overviewActive ? planner.tripOverview.focusPoints : undefined} routeVias={onStage ? planner.roadtripMapVias : planner.routeVias} showTransitRoutes={onStage ? false : planner.transitRoutesShown} @@ -144,7 +214,7 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { selectedDayId={planner.selectedDayId} routeSegments={onStage ? undefined : planner.overviewActive ? planner.tripOverview.segments : planner.routeSegments} selectedPlaceId={planner.selectedPlaceId} - onMarkerClick={planner.handleMarkerClick} + onMarkerClick={onStage ? openStagePin : planner.handleMarkerClick} // Tap on empty map = deselect, same contract as desktop. onMapClick={planner.handleMapClick} // The chip rail names a day at all times on mobile, so a place dropped on diff --git a/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx b/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx index 5bd5b8f86..aec9b405a 100644 --- a/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx +++ b/client/src/mobile/screens/trip/roadtrip/MRoadtripTab.tsx @@ -1,7 +1,7 @@ import { useRef } from 'react' import { AlertTriangle, MapPin, Navigation } from 'lucide-react' import { useMPlanDaySwipe } from '../plan/useMPlanDaySwipe' -import { useMRoadtrip } from './useMRoadtrip' +import { showStopOnMap, useMRoadtrip } from './useMRoadtrip' import { useMRtCorridor } from './useMRtCorridor' import MRtCorridorBar from './MRtCorridorBar' import { RtAutoRow, RtDryRow, RtLegRow, RtSpillRow, RtStopRow, type RowChrome } from './MRoadtripRows' @@ -92,7 +92,7 @@ export default function MRoadtripTab({ planner, shell }: MTripTabPanelProps) { {/* `rt` is handed down rather than looked up again: useMRoadtrip owns a 30s interval and a network subscription, and a second call would run a second pair of them for the same screen. */} - + ) @@ -178,7 +178,7 @@ export default function MRoadtripTab({ planner, shell }: MTripTabPanelProps) { )} - {rt.upNext && } + {rt.upNext && }
{rt.rows.map((row, i) => { @@ -221,11 +221,16 @@ export default function MRoadtripTab({ planner, shell }: MTripTabPanelProps) { * One line rather than a sheet that can be dragged open. A drag sheet brings a panel * height the map fit has to account for and lifts the locate and style buttons with * it, which is a bigger change than this half of the screen needs to be useful. + * + * It reads as a place, so a tap opens that place's stop, the same sheet its row in the + * chain opens. It used to switch to the chain instead, which broke the promise its name + * makes; the header's list switch is the way back to the chain. Name, clock and tap all + * come off one row (see stageEnd), so the bar can never name one stop and open another. */ -function StageBar({ planner, shell, rt }: { +function StageBar({ planner, rt, onOpen }: { planner: MTripTabPanelProps['planner'] - shell: MTripTabPanelProps['shell'] rt: ReturnType + onOpen: (row: StopRow) => void }) { const { t } = planner const unit = useSettingsStore(s => s.settings.distance_unit) @@ -245,32 +250,33 @@ function StageBar({ planner, shell, rt }: { } const tint = dayColorsOn ? dayColor(stage.dayNumber) : null - const named = stage.stops.filter(s => !s.automaticNight) - const last = named.length ? named[named.length - 1] : undefined + const end = rt.end - return ( - + ) + const box = 'flex w-full items-center gap-2.5 rounded-[22px] border border-[color:var(--m-cbr)] bg-[color:var(--m-card)] px-[14px] py-[11px] text-left shadow-[0_16px_44px_-14px_rgba(0,0,0,.35)] backdrop-blur-[24px] backdrop-saturate-[1.6]' + + // A stage without a single stop has nothing for a tap to open, so a plain block rather + // than a dead button, the rule the whole-drive line above follows too. + if (!end) return
{inner}
+ return } /** @@ -281,10 +287,11 @@ function StageBar({ planner, shell, rt }: { * planned time stays put and the delta sits beside it, because the planned time is the * number you want to compare against. */ -function UpNext({ planner, shell, rt, onOpen }: { +function UpNext({ planner, shell, rt, stageDayId, onOpen }: { planner: MTripTabPanelProps['planner'] shell: MTripTabPanelProps['shell'] rt: ReturnType + stageDayId: number onOpen: (row: StopRow) => void }) { const { t } = planner @@ -333,10 +340,8 @@ function UpNext({ planner, shell, rt, onOpen }: {