From 6a6c7f1882b6905f7a35a5bda9b2b99e3432dcfe Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 17 Sep 2026 00:10:51 +0000 Subject: [PATCH] feat(roadtrip): other ways to drive a leg, on the phone The desk offers alternative routes from the drive band between two stops; the phone had nothing. A leg in the phone chain now carries one round button that asks the router for other ways and switches to the map, where a bar takes the stage bar's place: the ways as chips with their colour, time, distance and what sets them apart, and a confirm button. On glass there is no hover, so a tap on a chip or on a line only previews that road; confirm writes it. The button shows only where the desk offers it (a routed leg between chosen stops), for someone who may edit the day, and not offline, since vias are written online only. The picker could not have stayed open on a phone at all: the planner closed it whenever road trip mode was off, and that mode is off on a phone by design, so the next render shut it. The close now follows where the picker can be seen (road trip mode on the desk, the road trip tab on a phone), and useRouteAlternatives returns a memoised object. The rules both shells need are shared: legReroutable in the row model, alternativesPhase and alternativeSubline next to the overlays, and the desk bar and sidebar read them too with no visible change. Both map engines take an optional fitPadding, so the phone frames the leg between the day chips and the bar instead of under them. --- client/src/components/Map/MapView.test.tsx | 44 ++++ client/src/components/Map/MapView.tsx | 40 +++- client/src/components/Map/MapViewGL.test.tsx | 56 +++++ client/src/components/Map/MapViewGL.tsx | 21 +- .../Roadtrip/RoadtripAlternativesBar.tsx | 14 +- .../components/Roadtrip/RoadtripSidebar.tsx | 3 +- .../Roadtrip/alternativeOverlays.test.ts | 38 +++- .../Roadtrip/alternativeOverlays.ts | 37 ++++ .../Roadtrip/roadtripRowModel.test.ts | 31 ++- .../components/Roadtrip/roadtripRowModel.ts | 20 ++ .../Roadtrip/useRouteAlternatives.test.ts | 23 +- .../Roadtrip/useRouteAlternatives.ts | 10 +- .../src/mobile/screens/trip/map/MMapArea.tsx | 109 +++++++++- .../screens/trip/roadtrip/MRoadtripRows.tsx | 63 ++++-- .../screens/trip/roadtrip/MRoadtripTab.tsx | 35 ++- .../trip/roadtrip/MRtAlternativesBar.tsx | 178 +++++++++++++++ .../trip/roadtrip/useMRtAlternatives.ts | 136 ++++++++++++ .../useTripPlanner.roadtrip.test.tsx | 50 ++++- .../src/pages/tripPlanner/useTripPlanner.ts | 11 +- client/tests/helpers/mobileTrip.ts | 14 ++ .../tests/unit/mobile/trip/MMapArea.test.tsx | 119 +++++++++- .../unit/mobile/trip/MRoadtripRows.test.tsx | 51 ++++- .../unit/mobile/trip/MRoadtripTab.test.tsx | 119 +++++++++- .../mobile/trip/MRtAlternativesBar.test.tsx | 204 ++++++++++++++++++ .../mobile/trip/useMRtAlternatives.test.ts | 203 +++++++++++++++++ 25 files changed, 1572 insertions(+), 57 deletions(-) create mode 100644 client/src/mobile/screens/trip/roadtrip/MRtAlternativesBar.tsx create mode 100644 client/src/mobile/screens/trip/roadtrip/useMRtAlternatives.ts create mode 100644 client/tests/unit/mobile/trip/MRtAlternativesBar.test.tsx create mode 100644 client/tests/unit/mobile/trip/useMRtAlternatives.test.ts diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index a2836d124..2075c2805 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -1074,6 +1074,50 @@ describe('MapView bounds fitting', () => { rerender() expect(mapMock.fitBounds).not.toHaveBeenCalled() }) + + it('FE-COMP-MAPVIEW-078: a caller padding frames the focus points instead of the phone margin, and the day fit keeps its own', () => { + // A phone shell lays a chip rail over the top of the map and a bar plus the dock over + // the bottom. The flat margin put the ends of a framed leg under either of them. + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: 420 }) + const leg: [number, number][] = [[48.1, 2.1], [48.3, 2.4]] + const chrome = { top: 120, right: 16, bottom: 290, left: 16 } + const { rerender } = render() + expect(mapMock.fitBounds).not.toHaveBeenCalled() + + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(1) + expect(mapMock.fitBounds.mock.calls[0][1]).toMatchObject({ paddingTopLeft: [16, 120], paddingBottomRight: [16, 290] }) + + // Picking a day is not the caller's frame, so that fit keeps the phone margin. + const places = [buildMapPlace({ id: 1, lat: 48, lng: 2 }), buildMapPlace({ id: 2, lat: 48.2, lng: 2.2 })] + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(2) + expect(mapMock.fitBounds.mock.calls[1][1]).toMatchObject({ paddingTopLeft: [40, 20], paddingBottomRight: [40, 20] }) + + // And without one, the focus fit falls back to that margin too. + const stage: [number, number][] = [[47, 1], [47.5, 1.5]] + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(3) + expect(mapMock.fitBounds.mock.calls[2][1]).toMatchObject({ paddingTopLeft: [40, 20], paddingBottomRight: [40, 20] }) + }) + + it('FE-COMP-MAPVIEW-079: the caller padding is read by value, so only new numbers refit the points on screen', () => { + const leg: [number, number][] = [[48.1, 2.1], [48.3, 2.4]] + const { rerender } = render( + , + ) + expect(mapMock.fitBounds).toHaveBeenCalledTimes(1) + + // A parent that builds the object inline hands a new one on every render. The camera + // belongs to the traveller between fits, so that alone must not take it back. + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(1) + + // New numbers mean the chrome moved, so the same points are framed again around it. + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(2) + expect(mapMock.fitBounds.mock.calls[1][1]).toMatchObject({ paddingTopLeft: [16, 120], paddingBottomRight: [16, 98] }) + }) }) describe('MapView photo thumbnails', () => { diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index 6d63ec97a..6a043e37d 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -401,15 +401,33 @@ interface BoundsControllerProps { * needs that leg on screen, which is neither the day nor the trip. */ focusPoints?: [number, number][] + /** + * What the caller's own chrome covers while `focusPoints` is framed, in pixels per edge. + * + * `paddingOpts` knows the planner's panels and nothing else, and on a phone it is a flat + * margin. A shell that lays its own bars over the map passes what they cover, so the + * frame lands in the part still visible. Only the fit on `focusPoints` reads it. + * Compared by value: the same numbers in a new object do not refit, while new numbers + * refit the points already handed over, because the chrome they must clear has moved. + */ + fitPadding?: ViewportPadding } -function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMount = false, focusPoints }: BoundsControllerProps) { +/** A padding box as Leaflet's fit options spell it, corner pairs rather than edges. */ +function leafletPadding(box: ViewportPadding): L.FitBoundsOptions { + return { + paddingTopLeft: [box.left, box.top], + paddingBottomRight: [box.right, box.bottom], + } +} + +function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMount = false, focusPoints, fitPadding }: BoundsControllerProps) { const map = useMap() const prevFitKey = useRef(-1) const awaitingRoute = useRef(false) const fitRan = useRef(false) - const fitTo = useCallback((coords: [number, number][]) => { + const fitTo = useCallback((coords: [number, number][], padding: L.FitBoundsOptions = paddingOpts) => { if (coords.length === 0) return try { const bounds = L.latLngBounds(coords) @@ -428,7 +446,7 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo * (#1982). MapViewGL never had the nudge, so this also brings the two * renderers back into agreement. */ - map.fitBounds(bounds, { ...paddingOpts, maxZoom: 16, animate: true }) + map.fitBounds(bounds, { ...padding, maxZoom: 16, animate: true }) } } catch {} }, [map, paddingOpts]) @@ -460,14 +478,18 @@ function BoundsController({ places, routeCoords, fitKey, paddingOpts, framedOnMo fitTo([...places.map(p => [p.lat, p.lng] as [number, number]), ...routeCoords]) }, [routeCoords]) // eslint-disable-line react-hooks/exhaustive-deps + // The caller's padding as a value, so a parent that builds the object inline on every + // render does not move the camera each time it renders. + const fitPaddingKey = fitPadding ? [fitPadding.top, fitPadding.right, fitPadding.bottom, fitPadding.left].join(' ') : '' + // Frame whatever was handed over. Nothing happens when it empties, so closing the // picker leaves the map where the user left it rather than snapping back. useEffect(() => { if (!focusPoints?.length) return // A day fit that has not run yet must not overwrite this a moment later. awaitingRoute.current = false - fitTo(focusPoints) - }, [focusPoints]) // eslint-disable-line react-hooks/exhaustive-deps + fitTo(focusPoints, fitPadding ? leafletPadding(fitPadding) : paddingOpts) + }, [focusPoints, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps return null } @@ -687,6 +709,7 @@ export const MapView = memo(function MapView({ onRemoveVia, alternativeRoutes, focusPoints, + fitPadding, clusterLoosely = false, activeAlternative, onChooseAlternative, @@ -748,10 +771,7 @@ export const MapView = memo(function MapView({ } }, [leftWidth, rightWidth, hasInspector, hasDayDetail]) - const paddingOpts = useMemo((): L.FitBoundsOptions => ({ - paddingTopLeft: [paddingBox.left, paddingBox.top], - paddingBottomRight: [paddingBox.right, paddingBox.bottom], - }), [paddingBox]) + const paddingOpts = useMemo(() => leafletPadding(paddingBox), [paddingBox]) // Open framed on the places rather than on the caller's default, so a trip in Japan shows // Japan straight away instead of the world view followed by a flight across the planet. @@ -1070,7 +1090,7 @@ export const MapView = memo(function MapView({ )} - 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} framedOnMount={initialView.framed} focusPoints={focusPoints} /> + 0 ? dayPlaces : places} routeCoords={dayPlaces.length > 0 ? routeCoords : []} fitKey={fitKey} paddingOpts={paddingOpts} framedOnMount={initialView.framed} focusPoints={focusPoints} fitPadding={fitPadding} /> diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx index bd2ef5379..c10a58491 100644 --- a/client/src/components/Map/MapViewGL.test.tsx +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -2196,6 +2196,62 @@ describe('MapViewGL', () => { ) expect(glMap.addSource).not.toHaveBeenCalledWith('trip-satellite', expect.anything()) }) + + it('FE-COMP-MAPVIEWGL-085: a caller padding frames the focus points instead of the phone margin, and the day fit keeps its own', async () => { + // A phone shell lays a chip rail over the top of the map and a bar plus the dock over + // the bottom. The flat 40px margin put the ends of a framed leg under either of them. + const original = window.innerWidth + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: 420 }) + try { + const leg: [number, number][] = [[48.1, 2.1], [48.3, 2.4]] + const chrome = { top: 120, right: 16, bottom: 290, left: 16 } + const { rerender } = render() + await act(async () => {}) + expect(glMap.fitBounds).not.toHaveBeenCalled() + + rerender() + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(1) + expect(glMap.fitBounds.mock.calls[0][1]).toMatchObject({ padding: chrome, maxZoom: 15 }) + + // Picking a day is not the caller's frame, so that fit keeps the phone margin. + const places = [buildMapPlace({ id: 88, lat: 48.1, lng: 2.1 }), buildMapPlace({ id: 89, lat: 48.2, lng: 2.2 })] + rerender() + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(2) + expect(glMap.fitBounds.mock.calls[1][1]).toMatchObject({ padding: { top: 40, right: 20, bottom: 40, left: 20 } }) + + // And without one, the focus fit falls back to that margin too. + const stage: [number, number][] = [[47, 1], [47.5, 1.5]] + rerender() + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(3) + expect(glMap.fitBounds.mock.calls[2][1]).toMatchObject({ padding: { top: 40, right: 20, bottom: 40, left: 20 } }) + } finally { + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: original }) + } + }) + + it('FE-COMP-MAPVIEWGL-086: the caller padding is read by value, so only new numbers refit the points on screen', async () => { + const leg: [number, number][] = [[48.1, 2.1], [48.3, 2.4]] + const { rerender } = render( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(1) + + // A parent that builds the object inline hands a new one on every render. The camera + // belongs to the traveller between fits, so that alone must not take it back. + rerender() + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(1) + + // New numbers mean the chrome moved, so the same points are framed again around it. + rerender() + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(2) + expect(glMap.fitBounds.mock.calls[1][1]).toMatchObject({ padding: { top: 120, right: 16, bottom: 98, left: 16 } }) + }) }) describe('MapViewGL attribution', () => { const flushFrames = () => act(async () => { await new Promise(resolve => setTimeout(resolve, 40)) }) diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 48f8d5001..e72b138ef 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -40,7 +40,7 @@ import { resolveTrackColor, hasManualTrackColor } from './trackColors' import { buildPoiPopupHtml } from './placePopup' import { pluginsApi, type PluginMapMarker, type PluginMapLayer } from '../../api/client' import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, SATELLITE_TILE_URL, SATELLITE_TILE_ATTRIBUTION, SATELLITE_TILE_MAXZOOM } from '../../constants/mapDefaults' -import { computeMapViewport, TILE_SIZE_GL } from '../../utils/mapViewport' +import { computeMapViewport, TILE_SIZE_GL, type ViewportPadding } from '../../utils/mapViewport' function categoryIconSvg(iconName: string | null | undefined, size: number): string { const IconComponent = (iconName && CATEGORY_ICON_MAP[iconName]) || CATEGORY_ICON_MAP['MapPin'] @@ -185,6 +185,16 @@ interface Props { * needs that leg on screen, which is neither the day nor the trip. */ focusPoints?: [number, number][] + /** + * What the caller's own chrome covers while `focusPoints` is framed, in pixels per edge. + * + * The default padding knows this component's panels and nothing else, and on a phone it + * is a flat margin. A shell that lays its own bars over the map passes what they cover, + * so the frame lands in the part still visible. Only the fit on `focusPoints` reads it. + * Compared by value: the same numbers in a new object do not refit, while new numbers + * refit the points already handed over, because the chrome they must clear has moved. + */ + fitPadding?: ViewportPadding /** * Let markers stay apart longer than usual. * @@ -656,6 +666,7 @@ export function MapViewGL({ zoom = DEFAULT_MAP_ZOOM, fitKey = 0, focusPoints, + fitPadding, clusterLoosely = false, hazards, dawarichTrack = null, @@ -2206,6 +2217,10 @@ export function MapViewGL({ if (routeArrivedForPendingFit) pendingRouteFitRef.current = null }, [fitKey, routeFitKey]) // eslint-disable-line react-hooks/exhaustive-deps + // The caller's padding as a value, so a parent that builds the object inline on every + // render does not move the camera each time it renders. + const fitPaddingKey = fitPadding ? [fitPadding.top, fitPadding.right, fitPadding.bottom, fitPadding.left].join(' ') : '' + // Frame whatever was handed over. Nothing happens when it empties, so closing the // picker leaves the map where the user left it rather than snapping back. useEffect(() => { @@ -2217,13 +2232,13 @@ export function MapViewGL({ pendingRouteFitRef.current = null try { map.fitBounds(bounds, { - padding: paddingOpts, + padding: fitPadding ?? paddingOpts, maxZoom: 15, pitch: enableMapbox3d ? 45 : 0, duration: 400, }) } catch { /* noop */ } - }, [focusPoints]) // eslint-disable-line react-hooks/exhaustive-deps + }, [focusPoints, fitPaddingKey]) // eslint-disable-line react-hooks/exhaustive-deps // flyTo selected place useEffect(() => { diff --git a/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx b/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx index 7b9ce1473..02cb319be 100644 --- a/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx +++ b/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx @@ -4,9 +4,8 @@ import { useTranslation } from '../../i18n/TranslationContext' import { Tooltip } from '../shared/Tooltip' import { useSettingsStore } from '../../store/settingsStore' import { formatDistance } from '../../utils/units' -import { formatDurationShort } from './roadtripModel' import type { LegAlternatives } from './useRouteAlternatives' -import type { AlternativeOverlay } from './alternativeOverlays' +import { alternativesPhase, alternativeSubline, type AlternativeOverlay } from './alternativeOverlays' interface RoadtripAlternativesBarProps { open: LegAlternatives | null @@ -35,6 +34,7 @@ export default function RoadtripAlternativesBar({ const { t } = useTranslation() const distanceUnit = useSettingsStore(s => s.settings.distance_unit) if (!open) return null + const phase = alternativesPhase(open, overlays) return (
@@ -53,14 +53,14 @@ export default function RoadtripAlternativesBar({
- {open.loading ? ( + {phase === 'loading' ? (

{t('roadtrip.alt.loading')}

- ) : open.error ? ( + ) : phase === 'failed' ? (

{t('roadtrip.alt.failed')}

- ) : overlays.length < 2 ? ( + ) : phase === 'onlyOne' ? ( // One route back means there is genuinely only one sensible way to drive it.

{t('roadtrip.alt.onlyOne')}

) : ( @@ -88,9 +88,7 @@ export default function RoadtripAlternativesBar({ {formatDistance(alt.distance / 1000, distanceUnit)} - {alt.note - ? alt.note - : t('roadtrip.alt.slower', { time: formatDurationShort(alt.slowerThanQuickest) })} + {alternativeSubline(alt, time => t('roadtrip.alt.slower', { time }))} {/* Only where the drive time on the map came from the other engine. The diff --git a/client/src/components/Roadtrip/RoadtripSidebar.tsx b/client/src/components/Roadtrip/RoadtripSidebar.tsx index df5afdddd..4c53731f5 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.tsx @@ -17,6 +17,7 @@ import { formatDistance } from '../../utils/units' import { formatDate, formatClockTime } from '../../utils/formatters' import { formatDurationShort, isServiceStopType, serviceColor, type ScheduleEntry, type ScheduleWarning, refuelsRange } from './roadtripModel' import { STOP_KIND_BY_KEY } from './stopKinds' +import { legReroutable } from './roadtripRowModel' import { spurWorthLabelling } from './accessSpur' import StopKindPicker from './StopKindPicker' import StopFillPicker from './StopFillPicker' @@ -1416,7 +1417,7 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on {i < last && (!day.stops[i + 1].automaticNight || day.legs[i]?.distance !== 0) ? ( onAskAlternatives(day.dayId, i) : undefined} + onAskAlternatives={onAskAlternatives && legReroutable(day, i) ? () => onAskAlternatives(day.dayId, i) : undefined} alternativesOpen={openAlternatives?.dayId === day.dayId && openAlternatives.index === i} /> ) : null} diff --git a/client/src/components/Roadtrip/alternativeOverlays.test.ts b/client/src/components/Roadtrip/alternativeOverlays.test.ts index 3a2a78a44..18bb4cc70 100644 --- a/client/src/components/Roadtrip/alternativeOverlays.test.ts +++ b/client/src/components/Roadtrip/alternativeOverlays.test.ts @@ -1,10 +1,11 @@ import { describe, it, expect } from 'vitest' -import { buildAlternativeOverlays } from './alternativeOverlays' +import { alternativeSubline, alternativesPhase, buildAlternativeOverlays, type AlternativeOverlay } from './alternativeOverlays' +import { formatDurationShort } from './roadtripModel' import { ALT_PRIMARY, ALT_SECONDARY, ALT_LABEL_PRIMARY_BG, ALT_LABEL_SECONDARY_BG } from './alternativeColors' import type { RouteAlternative } from '../Map/RouteCalculator' /** - * FE-ALTOVL-001..010 — turning the router's answers into something drawable. + * FE-ALTOVL-001..017: turning the router's answers into something drawable. * * Two decisions live here and neither is cosmetic: which of the offered roads is * drawn as the one you are on, and where each label hangs. A label anchored on a @@ -221,3 +222,36 @@ describe('buildAlternativeOverlays', () => { expect(out[1].color).toBe(ALT_SECONDARY) }) }) + +describe('alternativesPhase', () => { + const two = buildAlternativeOverlays([alt({ coordinates: line(20, 0) }), alt({ coordinates: line(20, 1) })], LABELS) + + it('FE-ALTOVL-016: asking beats a failure, a failure beats an empty list, and only two roads are a choice', () => { + // A new question replaces the old answer and its error with it. + expect(alternativesPhase({ loading: true, error: true }, two)).toBe('loading') + expect(alternativesPhase({ loading: true, error: false }, [])).toBe('loading') + // A router that will not answer is not a leg with one sensible way. + expect(alternativesPhase({ loading: false, error: true }, [])).toBe('failed') + expect(alternativesPhase({ loading: false, error: true }, two)).toBe('failed') + expect(alternativesPhase({ loading: false, error: false }, [])).toBe('onlyOne') + expect(alternativesPhase({ loading: false, error: false }, two.slice(0, 1))).toBe('onlyOne') + expect(alternativesPhase({ loading: false, error: false }, two)).toBe('choose') + }) +}) + +describe('alternativeSubline', () => { + const overlay = (over: Partial): AlternativeOverlay => ({ + ...buildAlternativeOverlays([alt({ coordinates: line(20, 0) }), alt({ coordinates: line(20, 1) })], LABELS)[1], + ...over, + }) + + it('FE-ALTOVL-017: an offer is named by what it is, and only a plain one by how much slower it is', () => { + const slower = (time: string) => `${time} slower` + for (const note of ['No motorway', 'Current', 'Fastest']) { + expect(alternativeSubline(overlay({ note, slowerThanQuickest: 1800 }), slower)).toBe(note) + } + // The caller words the difference; the figure is the same short duration the map prints. + expect(alternativeSubline(overlay({ note: '', slowerThanQuickest: 1800 }), slower)) + .toBe(`${formatDurationShort(1800)} slower`) + }) +}) diff --git a/client/src/components/Roadtrip/alternativeOverlays.ts b/client/src/components/Roadtrip/alternativeOverlays.ts index 5171a99ab..8f03e5530 100644 --- a/client/src/components/Roadtrip/alternativeOverlays.ts +++ b/client/src/components/Roadtrip/alternativeOverlays.ts @@ -1,6 +1,7 @@ import { formatDurationShort } from './roadtripModel' import { ALT_PRIMARY, ALT_SECONDARY, ALT_LABEL_PRIMARY_BG, ALT_LABEL_SECONDARY_BG } from './alternativeColors' import type { RouteAlternative } from '../Map/RouteCalculator' +import type { LegAlternatives } from './useRouteAlternatives' /** One offered route, ready to draw: the line, its colour, and where its label sits. */ export interface AlternativeOverlay { @@ -154,3 +155,39 @@ export function buildAlternativeOverlays( } }) } + +/** What a picker has to say about one leg, whichever shell draws it. */ +export type AlternativesPhase = 'loading' | 'failed' | 'onlyOne' | 'choose' + +/** + * Which of its four states a picker is in. + * + * Both bars branch on this, so the desk and the phone cannot disagree about when a leg has + * a choice. Asking wins over a failure, because a new question replaces the old answer + * and its error with it. A failure wins over an empty list, so a router that will not + * answer is never read as a leg with only one sensible way. Fewer than two overlays is + * that single way: `buildAlternativeOverlays` draws nothing for one answer, and one road + * is not a choice. + */ +export function alternativesPhase( + open: Pick, + overlays: readonly AlternativeOverlay[], +): AlternativesPhase { + if (open.loading) return 'loading' + if (open.error) return 'failed' + return overlays.length < 2 ? 'onlyOne' : 'choose' +} + +/** + * The second line an offer is listed with: what this way is, or else how much slower. + * + * Its own note wins, for the reason the note exists: a road offered because the motorway + * was left out of it is taken for that, not for its minutes. Without one the line is the + * difference to the quickest, worded by the caller (a translation this module stays free + * of). The desk bar's docstring tells what two copies of one figure once did, the driven + * road called "Fastest" beside an offer called quicker, so both bars read this line here + * instead of each keeping its own. + */ +export function alternativeSubline(alt: AlternativeOverlay, slower: (time: string) => string): string { + return alt.note || slower(formatDurationShort(alt.slowerThanQuickest)) +} diff --git a/client/src/components/Roadtrip/roadtripRowModel.test.ts b/client/src/components/Roadtrip/roadtripRowModel.test.ts index 1b4f59b0d..3bee909ff 100644 --- a/client/src/components/Roadtrip/roadtripRowModel.test.ts +++ b/client/src/components/Roadtrip/roadtripRowModel.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { destinationCount, firstStopOfPlace, + legReroutable, pickWarning, roadtripRows, stageClocks, @@ -14,7 +15,7 @@ import { import type { ScheduleEntry, ScheduleWarning } from './roadtripModel' import type { RoadtripDay, RoadtripStop, RouteSegment } from '@trek/shared/roadtrip' -// FE-RTROW-001 to FE-RTROW-037 +// FE-RTROW-001 to FE-RTROW-039 function stop(name: string, over: Partial = {}): RoadtripStop { return { @@ -443,3 +444,31 @@ describe('firstStopOfPlace', () => { expect(firstStopOfPlace([day([stop('Nacht', { placeId: 303, automaticNight: { phase: 'end', fromDayNumber: 1 } })])], 303)).toBeNull() }) }) + +describe('legReroutable', () => { + it('FE-RTROW-038: a routed leg between two stops anybody chose can be offered other ways, a service stop included', () => { + const d = day([stop('Bremen'), stop('Aral', { stopType: 'fuel' }), stop('Kassel')]) + expect(legReroutable(d, 0)).toBe(true) + expect(legReroutable(d, 1)).toBe(true) + }) + + it('FE-RTROW-039: no leg into or out of an automatic night, none without a route, none past either end', () => { + // Bremen, Kassel, automatic night, Fulda: the rail offers the first leg only. + const withNight = day([stop('Bremen'), stop('Kassel'), night(), stop('Fulda')]) + expect(legReroutable(withNight, 0)).toBe(true) + // Into the marker where the window closed, and out of it again the morning after. + expect(legReroutable(withNight, 1)).toBe(false) + expect(legReroutable(withNight, 2)).toBe(false) + expect(legReroutable(day([night('start'), stop('Fulda')]), 0)).toBe(false) + + // A leg the router has not answered for yet has nothing to weigh an offer against. + const unrouted = day([stop('A'), stop('B'), stop('C')], { legs: [seg(0)] }) + expect(legReroutable(unrouted, 0)).toBe(true) + expect(legReroutable(unrouted, 1)).toBe(false) + + // The last stop has no leg after it, whatever the legs array happens to hold. + const extra = day([stop('A'), stop('B')], { legs: [seg(0), seg(1)] }) + expect(legReroutable(extra, 1)).toBe(false) + expect(legReroutable(extra, -1)).toBe(false) + }) +}) diff --git a/client/src/components/Roadtrip/roadtripRowModel.ts b/client/src/components/Roadtrip/roadtripRowModel.ts index 2d925d9b2..6c6404351 100644 --- a/client/src/components/Roadtrip/roadtripRowModel.ts +++ b/client/src/components/Roadtrip/roadtripRowModel.ts @@ -152,6 +152,26 @@ export function roadtripRows(day: RoadtripDay): RoadtripRow[] { return rows } +/** + * Whether the leg leaving stop `index` can be offered other ways of driving it. + * + * The desktop rail's rule, kept here so the phone chain offers the button on exactly the + * legs the rail does. A leg needs a route, because an unrouted one has nothing to weigh an + * offer against. And neither of its ends may be an automatic night: that point is the + * shell's own marker for where the daily window closed, a spot somewhere along the road + * rather than a stop anyone chose. The router's offers from or to it would reshape a + * stretch of a longer drive, and the via a choice writes is filed by the position of a + * stored stop, which the marker is not. The rail draws the band out of that marker + * without the control for the same reason. + */ +export function legReroutable(day: RoadtripDay, index: number): boolean { + return index >= 0 + && index < day.stops.length - 1 + && !!day.legs[index] + && !day.stops[index].automaticNight + && !day.stops[index + 1].automaticNight +} + /** Stops that carry a number, for a count that agrees with the numbering above. */ export function destinationCount(day: RoadtripDay): number { return day.stops.filter(s => !s.automaticNight && !isServiceStopType(s.stopType)).length diff --git a/client/src/components/Roadtrip/useRouteAlternatives.test.ts b/client/src/components/Roadtrip/useRouteAlternatives.test.ts index 4b2db3f24..4b43f35ec 100644 --- a/client/src/components/Roadtrip/useRouteAlternatives.test.ts +++ b/client/src/components/Roadtrip/useRouteAlternatives.test.ts @@ -12,7 +12,7 @@ import type { RoadtripStop } from './useRoadtripRoutes' import type { RoadtripVia } from '@trek/shared' /** - * FE-ALTHOOK-001..009 — asking for other ways of driving one leg. + * FE-ALTHOOK-001..010: asking for other ways of driving one leg. * * The case that carries the feature: when the leg already carries vias, the road * actually being driven is NOT among what the router offers for the two bare @@ -147,4 +147,25 @@ describe('useRouteAlternatives', () => { expect(signals[1].aborted).toBe(true) expect(result.current.open).toBeNull() }) + + it('FE-ALTHOOK-010: the state keeps its identity until the picker itself changes', async () => { + // The planner keys its close gate on this object. A new one per render ran that + // effect on every render, so any render at all was a chance to close the picker. + const { result, rerender } = renderHook(() => useRouteAlternatives()) + const idle = result.current + rerender() + expect(result.current).toBe(idle) + + act(() => { result.current.ask(4, 1, from, to, 'driving', []) }) + const asking = result.current + expect(asking).not.toBe(idle) + // The functions themselves never change, only the object carrying the new `open`. + expect(asking.ask).toBe(idle.ask) + expect(asking.close).toBe(idle.close) + + await waitFor(() => expect(result.current.open?.loading).toBe(false)) + const answered = result.current + rerender() + expect(result.current).toBe(answered) + }) }) diff --git a/client/src/components/Roadtrip/useRouteAlternatives.ts b/client/src/components/Roadtrip/useRouteAlternatives.ts index e1bee3b84..9d4c5fd67 100644 --- a/client/src/components/Roadtrip/useRouteAlternatives.ts +++ b/client/src/components/Roadtrip/useRouteAlternatives.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef, useState } from 'react' +import { useCallback, useMemo, useRef, useState } from 'react' import { calculateAlternatives, calculateRoute, type RouteAlternative } from '../Map/RouteCalculator' import type { RoadtripStop } from './useRoadtripRoutes' import type { RoadtripVia } from '@trek/shared' @@ -108,5 +108,11 @@ export function useRouteAlternatives(): RouteAlternativesState { }) }, []) - return { open, ask, close } + // One object for as long as nothing in it changes. The planner keys its close gate and + // the callbacks that ask, choose and focus on this state as a whole, so a fresh object + // per render re-ran that gate and rebuilt those callbacks on every render of the + // planner, whatever had caused it. Whether the picker stays open is still the gate's own + // condition to decide (a new `open` runs it either way); the memo only stops that work + // from repeating on renders that changed nothing about the picker. + return useMemo(() => ({ open, ask, close }), [open, ask, close]) } diff --git a/client/src/mobile/screens/trip/map/MMapArea.tsx b/client/src/mobile/screens/trip/map/MMapArea.tsx index 650cc7993..b55aa8b41 100644 --- a/client/src/mobile/screens/trip/map/MMapArea.tsx +++ b/client/src/mobile/screens/trip/map/MMapArea.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo, useRef, useState } from 'react' +import { useCallback, useLayoutEffect, useMemo, useRef, useState, type RefObject } 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' @@ -12,8 +12,10 @@ import { stageMapData } from '../../../../components/Roadtrip/stageMap' import { useRoadtripSettings } from '../../../../hooks/useRoadtripSettings' import { useSettingsStore } from '../../../../store/settingsStore' import { useTripStore } from '../../../../store/tripStore' +import { RT_ALT_BAR_LIFT } from '../roadtrip/useMRtAlternatives' import type { MMapAreaProps } from '../MTripShell' import type { Poi } from '../../../../components/Map/poiCategories' +import type { ViewportPadding } from '../../../../utils/mapViewport' /** One array, so an explore set switched off does not move every pin on the stage. */ const NO_POIS: Poi[] = [] @@ -25,6 +27,71 @@ const NO_POIS: Poi[] = [] */ const COMPASS_LEFT = MAP_LAYER_SWITCHER_INSET + MAP_ROUND_CONTROL_SIZE + 8 +/** The safe-area insets at the top and the bottom of the screen, in pixels. */ +interface SafeInsets { + top: number + bottom: number +} + +const NO_INSETS: SafeInsets = { top: 0, bottom: 0 } + +/** + * The safe-area insets as numbers, read off a probe that is padded by them. + * + * The frame handed to the map engine is pixels, and `env()` only resolves inside CSS. On a + * phone with a notch the chrome sits a status bar lower (and a home indicator higher) than + * its classes spell out, so a frame worked out without the insets would put the far end of + * a leg under the day chips. A computed padding is where the browser hands the resolved + * inset back. Read on mount and again on a resize, which is what turning the phone does. + */ +function useSafeInsets(probe: RefObject): SafeInsets { + const [insets, setInsets] = useState(NO_INSETS) + useLayoutEffect(() => { + const read = () => { + const el = probe.current + if (!el) return + const style = window.getComputedStyle(el) + const top = Number.parseFloat(style.paddingTop) || 0 + const bottom = Number.parseFloat(style.paddingBottom) || 0 + setInsets(prev => (prev.top === top && prev.bottom === bottom ? prev : { top, bottom })) + } + read() + window.addEventListener('resize', read) + return () => window.removeEventListener('resize', read) + }, [probe]) + return insets +} + +/** + * Where the roads offered for a leg are framed: in the strip of map between the day chips + * and the alternatives bar, rather than behind either of them. + * + * Both engines fit a phone with a flat margin that knows nothing of this shell's bars, and + * with the picker open those cover most of the screen: that margin would leave the leg's + * ends and their time pills under the chips and under a 174px bar. The numbers are the + * ones the classes on this screen are built from, so a change there has to be carried here. + * + * Top: the safe area, then `--m-safe-top`'s own 12px, the chip rail's 50px offset under it + * and its 42px height, then a 12px gap so a time pill at the far end is not flush against + * the rail. The search bar that sits below the rail on the stage steps away while the + * picker is open, so the rail is the lowest chrome up there. + * + * Bottom: the safe area, the dock's 74px (62px tall, 12px off the bottom), the bar's lift + * (its height and the 15px gap between the dock and its foot) and the 38px credit row + * over that. That is the very line `--bottom-nav-h` puts the round controls' band on, so + * the frame ends where they begin, and they only take the two corners of it. + * + * The sides keep the 20px both engines already give a phone: nothing floats there. + */ +function alternativesFitPadding(insets: SafeInsets): ViewportPadding { + return { + top: insets.top + 12 + 50 + 42 + 12, + right: 20, + bottom: insets.bottom + 74 + RT_ALT_BAR_LIFT + 38, + left: 20, + } +} + /** A camera focus the planner is holding, and the day that was on screen when it arrived. */ interface HeldFocus { points: readonly [number, number][] @@ -133,6 +200,20 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { if (focus !== heldFocus) setHeldFocus(focus) const focusPending = focus.live && planner.mapFocusPoints.length > 0 + // Handed over only once there are roads to frame, not as soon as the picker opens: the + // engines refit whatever they hold when this padding changes, and while the router is + // still thinking that is the stage, which would be flown out to a smaller frame just + // before the answer flies it to the leg. So the padding and the roads arrive in one + // render and the camera moves once, and on close both go in one render as well. + // Memoised for the reason the stage data is, the shell re-rendering on every store write. + const insetProbe = useRef(null) + const insets = useSafeInsets(insetProbe) + const framingOffers = onStage && planner.alternativeOverlays.length > 0 + const fitPadding = useMemo( + () => (framingOffers ? alternativesFitPadding(insets) : undefined), + [framingOffers, insets], + ) + /** * A pin on the road trip tab opens its stop: the sheet the chain row opens, not the * place inspector. @@ -201,9 +282,21 @@ export default function MMapArea({ planner, shell }: MMapAreaProps) { className="m-credit-corner absolute inset-0 isolate overflow-hidden bg-[color:var(--m-mapb)] [--m-map-floor:calc(env(safe-area-inset-bottom,0px)+74px+var(--m-stage-lift,0px))] [--bottom-nav-h:calc(var(--m-map-floor)+38px)]" // 76px is the stage bar's own height plus the gap it keeps on both sides, so the // credit lands one gap above the bar instead of on its top edge, and the round - // controls one credit row above that. - style={{ ['--m-stage-lift' as string]: onStage && mapActive ? '76px' : '0px' }} + // controls one credit row above that. While other ways of driving a leg are on offer + // their bar stands in that slot instead, taller, and the floor clears it the same way. + style={{ + ['--m-stage-lift' as string]: onStage && mapActive + ? (planner.routeAlternatives.open ? `${RT_ALT_BAR_LIFT}px` : '76px') + : '0px', + }} > + {/* Measures the safe area for the alternatives frame, see useSafeInsets. Out of the + flow and invisible, so it takes no room and catches no tap. */} +