diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index 881e3c823..45fabf069 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -1,5 +1,5 @@ import { useRoadtripSettings } from '../../hooks/useRoadtripSettings' -import { assembleRoadtrip, carrierLegsFor, carrierSeam, foldRouteRun, isCarrierMode, seatCarrierStops, standsAsDay, terminalAssignmentId, viasLeaving, type CarrierSeam, type RoadtripStop, type RoadtripRoutes, type PlanDay, type QuietDay, type RoutedLeg } from '@trek/shared/roadtrip' +import { assembleRoadtrip, carrierLegsFor, carrierSeam, foldRouteRun, isCarrierMode, mergeRouteSegments, seatCarrierStops, standsAsDay, terminalAssignmentId, viasLeaving, type CarrierSeam, type RoadtripStop, type RoadtripRoutes, type PlanDay, type QuietDay, type RoutedLeg } from '@trek/shared/roadtrip' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { avoidedClasses, calculateRouteWithLegs, routeEngineFor, RoutingRefusedError, type RouteEngine } from '../Map/RouteCalculator' import { resolveLegMode } from '../Planner/legMode' @@ -65,10 +65,13 @@ const stopKey = (s: RoadtripStop): string => * seam round and found every seam already answered, so the drive between two days kept * its old road and its old minutes until the page was loaded again. */ -const seamShape = (from: RoadtripStop, viasByDay: Record, mode: string, avoidKey: string): string => +const seamShape = (from: RoadtripStop, viasByDay: Record, mode: string, avoidKey: string, unit: string): string => [ mode, avoidKey, + // The seam's printed texts are in this unit, so a switch asks for it again, the way a + // day's own run is asked for again. + unit, viasLeaving(from, viasByDay[from.ownerDayId] ?? []) .map(v => `${v.lat.toFixed(5)},${v.lng.toFixed(5)}`) .join('|'), @@ -492,7 +495,7 @@ export function useRoadtripRoutes( collectedMisses[day.dayId] = [...new Set([...(collectedMisses[day.dayId] ?? []), ...missed])] } } - const folded = foldRouteRun(run, stopAt, r, mode) + const folded = foldRouteRun(run, stopAt, r, mode, distanceUnit) // Remembered per leg, so a picker opened on one of them knows the line on the // map is not its engine's and does not read it as the road already taken. if (r.avoidance?.fellBack) for (const key of Object.keys(folded)) collectedStandIns.add(key) @@ -615,7 +618,7 @@ export function useRoadtripRoutes( // nothing at all: the first answer was cached under the pair, and dragging the // point changed the request nobody was going to send again. const mode = legModeOf(from, to, dayId) - const shape = seamShape(from, viasByDay, mode, avoidKey) + const shape = seamShape(from, viasByDay, mode, avoidKey, distanceUnit) const have = seamLegs[legKey(from, to)] const current = have?.shape === shape // What the drawn join could not avoid is flagged on the card it arrives on, the way @@ -649,7 +652,7 @@ export function useRoadtripRoutes( } } return { seams: out, seamMisses: misses } - }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays, legModeOf, avoidKey]) + }, [chains, plan, quietDays, window, legsByDay, seamLegs, rideLegs, viasByDay, connectDays, legModeOf, avoidKey, distanceUnit]) /** Each card's unavoided classes, its own run's and those of the join it is reached by. */ const missedOnCards = useMemo(() => { const out: Record = { ...missedByDay } @@ -718,14 +721,10 @@ export function useRoadtripRoutes( ) if (controller.signal.aborted) return // One leg per waypoint PAIR, so a shaped seam comes back in pieces and the rail - // wants the whole drive: summed here, exactly as a day run folds its via legs + // wants the whole drive: merged here, exactly as a day run folds its via legs // back onto the stop pair they belong to. if (!r.legs.length) continue - const merged = r.legs.length === 1 ? r.legs[0] : { - ...r.legs[0], - distance: r.legs.reduce((sum, l) => sum + (l.distance ?? 0), 0), - duration: r.legs.reduce((sum, l) => sum + (l.duration ?? 0), 0), - } + const merged = mergeRouteSegments(r.legs, distanceUnit) setSeamLegs(prev => ({ ...prev, [legKey(seam.from, seam.to)]: { diff --git a/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx index 008c70aeb..9821b16d5 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx @@ -1121,10 +1121,10 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { expect(rt.vias.addMany).not.toHaveBeenCalled() expect(rt.alt.close).not.toHaveBeenCalled() // Said to the bar as well, which keeps it beside the offers and announces it. + // Exactly this sentence: nothing about a ferry the offer never took. expect(rt.alt.settle).toHaveBeenCalledWith('The road trip’s router won’t follow this way, so it was not saved.') - expect(toasts).toContainEqual({ message: 'The road trip’s router won’t follow this way, so it was not saved.', type: 'error' }) - // Nothing about a ferry the offer never took. - expect(toasts).not.toContainEqual(expect.objectContaining({ message: expect.stringContaining('ferry') })) + // The desk's bar says it in its own status line, and a toast on top covered that line. + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-118: a ferry the router would not board says how to get it driven', async () => { @@ -1138,10 +1138,9 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { expect(rt.vias.addMany).not.toHaveBeenCalled() const ferryHint = 'This way crosses by ferry. Add the ferry as a transport booking and the drive follows it. If it lands the next day, put the stops across the water on that day.' - expect(toasts).toContainEqual({ message: ferryHint, type: 'info' }) // The ferry is the way out here, not the motorway the offer also leaves out. - expect(toasts).not.toContainEqual(expect.objectContaining({ message: expect.stringContaining('ticked') })) expect(rt.alt.settle).toHaveBeenCalledWith(`The road trip’s router won’t follow this way, so it was not saved. ${ferryHint}`) + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-135: a way that leaves a class out says the trip has to avoid it to be driven', async () => { @@ -1155,10 +1154,29 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { await act(async () => { await result.current.chooseRouteAlternative(1) }) expect(rt.vias.addMany).not.toHaveBeenCalled() - expect(toasts).toContainEqual({ - message: 'The road trip only drives this way with “Motorways” ticked under “Avoid where possible” in its settings.', - type: 'info', - }) + expect(rt.alt.settle).toHaveBeenCalledWith( + 'The road trip’s router won’t follow this way, so it was not saved. The road trip only drives this way with “Motorways” ticked under “Avoid where possible” in its settings.', + ) + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) + }) + + it('FE-TP-ROAD-139: on the phone a refused way is said as a toast too, its bar has no room for the sentence', async () => { + const desktopWidth = window.innerWidth + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: 390 }) + try { + routedDay() + openWith([detour({ hasFerry: true, avoids: 'motorway', engine: 'valhalla' })]) + rt.legRoute.mockResolvedValue(answer(RAIL_LINE, { hasFerry: false })) + const { result } = await renderRoadtrip() + + await act(async () => { await result.current.chooseRouteAlternative(1) }) + + expect(rt.vias.addMany).not.toHaveBeenCalled() + expect(toasts).toContainEqual({ message: 'The road trip’s router won’t follow this way, so it was not saved.', type: 'error' }) + expect(toasts).toContainEqual(expect.objectContaining({ message: expect.stringContaining('crosses by ferry'), type: 'info' })) + } finally { + Object.defineProperty(window, 'innerWidth', { configurable: true, writable: true, value: desktopWidth }) + } }) it('FE-TP-ROAD-136: the router own road on a leg OSRM drew in its place asks the rail for the leg again, and says so', async () => { @@ -1279,8 +1297,8 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { await act(async () => { await result.current.chooseRouteAlternative(1) }) expect(rt.vias.addMany).not.toHaveBeenCalled() - expect(rt.alt.settle).toHaveBeenCalled() - expect(toasts).toContainEqual({ message: 'The router is not answering right now.', type: 'error' }) + expect(rt.alt.settle).toHaveBeenCalledWith('The router is not answering right now.') + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-124: a router that fails while checking says so and writes nothing', async () => { @@ -1292,8 +1310,8 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => { await act(async () => { await result.current.chooseRouteAlternative(1) }) expect(rt.vias.addMany).not.toHaveBeenCalled() - expect(rt.alt.settle).toHaveBeenCalled() - expect(toasts).toContainEqual({ message: 'The router is not answering right now.', type: 'error' }) + expect(rt.alt.settle).toHaveBeenCalledWith('The router is not answering right now.') + expect(toasts.filter(x => /router|ferry|ticked/.test(x.message))).toEqual([]) }) it('FE-TP-ROAD-125: a check abandoned by closing the picker ends in silence', async () => { diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 0aa4001ba..dc50ea6ce 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -1671,7 +1671,9 @@ export function useTripPlanner() { } catch { if (signal.aborted) return routeAlternatives.settle(t('roadtrip.alt.failed')) - toast.error(t('roadtrip.alt.failed')) + // The desk's bar says it in its own status line; a toast on top covered that line + // and printed the same sentence twice. The phone's bar has no room for a sentence. + if (isMobile) toast.error(t('roadtrip.alt.failed')) return } if (signal.aborted) return @@ -1679,8 +1681,10 @@ export function useTripPlanner() { const refusal = t(proof.fellBack ? 'roadtrip.alt.failed' : 'roadtrip.alt.notHeld') const hint = proof.fellBack ? null : refusalHint(offer, proof.last, t) routeAlternatives.settle(hint ? `${refusal} ${hint}` : refusal) - toast.error(refusal, 6000) - if (hint) toast.info(hint, 8000) + if (isMobile) { + toast.error(refusal, 6000) + if (hint) toast.info(hint, 8000) + } return } @@ -1718,7 +1722,7 @@ export function useTripPlanner() { if (open.standIn) toast.info(t('roadtrip.alt.standIn'), 8000) // A picker opened on another leg while this was written belongs to that leg now. if (!signal.aborted) routeAlternatives.close() - }, [routeAlternatives, roadtripVias, roadtripRoutes, toast, t]) + }, [routeAlternatives, roadtripVias, roadtripRoutes, toast, t, isMobile]) /** * A click on the drawn route puts a via there, and the drive is redrawn through it. diff --git a/server/src/nest/roadtrip/roadtrip-plan.service.ts b/server/src/nest/roadtrip/roadtrip-plan.service.ts index 9414e733f..898099d4a 100644 --- a/server/src/nest/roadtrip/roadtrip-plan.service.ts +++ b/server/src/nest/roadtrip/roadtrip-plan.service.ts @@ -299,7 +299,7 @@ export class RoadtripPlanService { }); Object.assign( allLegs, - foldRouteRun(stops, stopAt, { coordinates: routed.leg.line, legs, vias: routed.leg.vias }, profile), + foldRouteRun(stops, stopAt, { coordinates: routed.leg.line, legs, vias: routed.leg.vias }, profile, distanceUnit), ); missedByDay[dayId] = [...new Set([...(missedByDay[dayId] ?? []), ...routed.avoidMissed])]; stops.forEach((stop, index) => { diff --git a/shared/src/roadtrip/routeRun.spec.ts b/shared/src/roadtrip/routeRun.spec.ts new file mode 100644 index 000000000..f4214949a --- /dev/null +++ b/shared/src/roadtrip/routeRun.spec.ts @@ -0,0 +1,74 @@ +/** + * ROADTRIP-ROUTERUN-001..003: a drive the router answered in pieces reads as one drive. + * + * A via cuts a stop-to-stop drive into one leg per waypoint pair. The numbers were summed + * back together, but the printed texts stayed those of the first piece, and the phone reads + * a segment's text before its numbers: a 3.9 mi drive into the next day said "1.9 mi in + * 6 min" (#2461). + */ +import type { RoadtripStop, RouteSegment } from './planning-types'; +import { formatDurationShort } from './roadtripModel'; +import { foldRouteRun, mergeRouteSegments, roadtripLegKey } from './routeRun'; +import { formatDistance } from './units'; + +import { describe, expect, it } from 'vitest'; + +const piece = (from: [number, number], to: [number, number], distance: number, duration: number): RouteSegment => ({ + mid: [(from[0] + to[0]) / 2, (from[1] + to[1]) / 2], + from, + to, + distance, + duration, + distanceText: formatDistance(distance / 1000, 'imperial'), + durationText: formatDurationShort(duration), + drivingText: formatDurationShort(duration), + walkingText: '', +}); + +const A: [number, number] = [52.37, 4.89]; +const VIA: [number, number] = [52.36, 4.9]; +const B: [number, number] = [52.35, 4.87]; + +describe('mergeRouteSegments', () => { + it('ROADTRIP-ROUTERUN-001: the pieces of one drive add up, and their texts are written from the sums', () => { + const first = piece(A, VIA, 2996, 417); + const second = piece(VIA, B, 3280, 423); + + const merged = mergeRouteSegments([first, second], 'imperial'); + + expect(merged.distance).toBe(6276); + expect(merged.duration).toBe(840); + expect(merged.distanceText).toBe(formatDistance(6.276, 'imperial')); + expect(merged.distanceText).not.toBe(first.distanceText); + expect(merged.durationText).toBe('14 min'); + expect(merged.drivingText).toBe('14 min'); + // The drive ends where its last piece does, not at the via. + expect(merged.from).toEqual(A); + expect(merged.to).toEqual(B); + }); + + it('ROADTRIP-ROUTERUN-002: a drive the router answered in one piece is handed on as it came', () => { + const only = piece(A, B, 3000, 600); + + expect(mergeRouteSegments([only], 'metric')).toBe(only); + }); +}); + +describe('foldRouteRun', () => { + it('ROADTRIP-ROUTERUN-003: a stop pair bent through a via gets the whole drive and its texts, in the reader unit', () => { + const stops = [ + { lat: A[0], lng: A[1] }, + { lat: B[0], lng: B[1] }, + ] as unknown as RoadtripStop[]; + const legs = [piece(A, VIA, 2996, 417), piece(VIA, B, 3280, 423)]; + + const folded = foldRouteRun(stops, [0, 2], { coordinates: [A, VIA, B], legs }, 'driving', 'metric'); + const seg = folded[roadtripLegKey(stops[0]!, stops[1]!)]!.seg; + + expect(seg.distance).toBe(6276); + expect(seg.duration).toBe(840); + expect(seg.distanceText).toBe(formatDistance(6.276, 'metric')); + expect(seg.durationText).toBe('14 min'); + expect(seg.mode).toBe('driving'); + }); +}); diff --git a/shared/src/roadtrip/routeRun.ts b/shared/src/roadtrip/routeRun.ts index 854fe7aef..09db903ca 100644 --- a/shared/src/roadtrip/routeRun.ts +++ b/shared/src/roadtrip/routeRun.ts @@ -1,17 +1,51 @@ import { lineMetres, sliceAtMeters, projectOntoRoute } from './corridor'; -import type { RoadtripStop, RouteSegment, RouteVia, RoutedLeg } from './planning-types'; -import { legIndexForAlong } from './roadtripModel'; +import type { DistanceUnit, RoadtripStop, RouteSegment, RouteVia, RoutedLeg } from './planning-types'; +import { formatDurationShort, legIndexForAlong } from './roadtripModel'; +import { formatDistance } from './units'; export const roadtripStopKey = (stop: RoadtripStop): string => `${stop.lat.toFixed(5)},${stop.lng.toFixed(5)},${stop.legMode ?? ''},${stop.incomingLegMode ?? ''}`; export const roadtripLegKey = (from: RoadtripStop, to: RoadtripStop): string => `${roadtripStopKey(from)}>${roadtripStopKey(to)}`; +/** The walking pace a segment's walking time is printed at, in metres per second. */ +const WALKING_METRES_PER_SECOND = 5000 / 3600; + +/** + * The pieces a via cuts one stop-to-stop drive into, read as that one drive again. + * + * The router answers one leg per pair of waypoints, so a drive bent through a via comes + * back in pieces. The numbers add up. The printed texts do not: carried over from the first + * piece, they had the phone print "1.9 mi in 6 min" under a 3.9 mi drive, because it reads + * a segment's text before its numbers. So the texts are written again from the sums, in the + * reader's unit, and the drive ends where its last piece does rather than at the first via. + */ +export function mergeRouteSegments(parts: RouteSegment[], unit: DistanceUnit): RouteSegment { + const first = parts[0]!; + if (parts.length === 1) return first; + const last = parts[parts.length - 1]!; + const distance = parts.reduce((sum, leg) => sum + (leg.distance ?? 0), 0); + const duration = parts.reduce((sum, leg) => sum + (leg.duration ?? 0), 0); + const durationText = formatDurationShort(duration); + return { + ...first, + to: last.to, + mid: [(first.from[0] + last.to[0]) / 2, (first.from[1] + last.to[1]) / 2], + distance, + duration, + distanceText: formatDistance(distance / 1000, unit), + durationText, + drivingText: durationText, + walkingText: formatDurationShort(distance / WALKING_METRES_PER_SECOND), + }; +} + export function foldRouteRun( stops: RoadtripStop[], stopAt: number[], route: { coordinates: [number, number][]; legs: RouteSegment[]; vias?: RouteVia[] }, mode: string, + unit: DistanceUnit, ): Record { const spine = route.coordinates.map(([lat, lng]) => ({ lat, lng })); const cuts: { key: string; from: number; to: number; seg: RouteSegment }[] = []; @@ -19,14 +53,7 @@ export function foldRouteRun( for (let index = 0; index < stops.length - 1; index++) { const parts = route.legs.slice(stopAt[index], stopAt[index + 1]); if (!parts.length) continue; - const merged = - parts.length === 1 - ? parts[0]! - : { - ...parts[0]!, - distance: parts.reduce((sum, leg) => sum + leg.distance, 0), - duration: parts.reduce((sum, leg) => sum + leg.duration, 0), - }; + const merged = mergeRouteSegments(parts, unit); const from = travelled; travelled += merged.distance; cuts.push({ key: roadtripLegKey(stops[index]!, stops[index + 1]!), from, to: travelled, seg: { ...merged, mode } });