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. */} +