k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(roadtrip): the vias, the alternatives, the pacing and the missing warning

Browse Source 
Choosing another road on a leg that already carried a via appended the new
point behind the old one, so the day drove south to the first and north to the
second: a zigzag matching neither the line drawn under the button nor the
distance printed on it. The alternatives are computed for the two bare
endpoints, so the write has to replace the leg, which is what the direct branch
beside it already did.

A day's vias were only loaded while road-trip mode was being looked at, and the
three handlers that re-anchor them after a stop is added, removed or reordered
are reachable from the place inspector in both modes. With the switch off they
computed their correction from an empty list and wrote nothing, so a deleted
stop left the detour on a leg nobody chose, for the traveller and for every
collaborator. They are loaded whenever the addon is on now.

Seam routing paced itself by sleeping after each request, and storing a result
shortened the key its own effect depends on: React tore the effect down
mid-sleep, the cleanup aborted it, and the next request went out immediately.
Thirteen seams left at round-trip speed, the shared routing hosts refused the
tail, and a refused seam changes no state so nothing ever retried it. The wait
now happens before the request, against a clock that outlives the effect.

The "avoidance not honoured" badge could never appear on the rail: the field
behind it was published every round and never written. Only the second engine
reports what it achieved against what was asked, and the rail plans in the
browser, so it never saw the server's own copy — a motorway crossing read as a
motorway-free drive.

Two smaller ones. `trip.toast.loadError` has been called since the first commit
and exists in no locale, so both call sites printed the key itself; it is
written now in all 23. And the phone shell honoured the road-trip session flag
while having no rail, no drive lines and no switch to leave it, so narrowing a
desktop window took the trip overview away and listed every booked night twice.
This commit is contained in:
Maurice
2026-09-13 23:14:00 +02:00
parent c8cb88133f
commit 61880ce7d6
26 changed files with 99 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/components/Roadtrip/useRoadtripRoutes.ts
+30 -1
View File
@@ -332,6 +332,20 @@ export function useRoadtripRoutes(
if (s) daySnaps[stopKey(stop)] = s
})
}
// What was asked for against what the road turned out to be. Only
// the second engine reports it, and only when it answered: an OSRM
// fallback leaves `avoidance` absent, which is the honest reading
// of "the weighting never happened". Collected here because the
// rail plans in the browser and never sees the server's own copy
// of this field — without it the "not honoured" badge could not
// appear at all, and a motorway-free drive that is not one read as
// if the setting had held.
if (r.avoidance) {
const missed = r.avoidance.asked.filter(cls => !r.avoidance!.achieved.includes(cls))
if (missed.length) {
collectedMisses[day.dayId] = [...new Set([...(collectedMisses[day.dayId] ?? []), ...missed])]
}
}
Object.assign(dayLegs, foldRouteRun(run, stopAt, r, mode))
return
} catch (err) {
@@ -443,6 +457,8 @@ export function useRoadtripRoutes(
return out
}, [chains, plan, quietDays, window, legsByDay, seamLegs, viasByDay, connectDays])
const seamKey = seams.map(s => `${legKey(s.from, s.to)}#${seamShape(s.from, viasByDay)}`).join(';')
/** When the last seam request went out, across every run of the effect below. */
const lastSeamRequestAt = useRef(0)
useEffect(() => {
if (!seams.length) return
@@ -450,6 +466,20 @@ export function useRoadtripRoutes(
void (async () => {
for (const seam of seams) {
if (controller.signal.aborted) return
// Paced before the request, against a clock that outlives this effect.
//
// Waiting AFTER one instead spaced nothing: storing a seam changes
// seamLegs, which is a dependency of the seams memo, which shortens
// seamKey, which is this effect's first dependency — so React tore the
// effect down mid-wait, the cleanup aborted the sleep, and the next run
// fired straight away. Thirteen seams went out at round-trip speed, the
// shared routing hosts answered the tail with 429, and a refused seam
// writes no state, so nothing changed to make the effect try again: the
// map drew the trip in pieces and the totals came back short.
const since = performance.now() - lastSeamRequestAt.current
if (since < REQUEST_SPACING_MS) await sleep(REQUEST_SPACING_MS - since, controller.signal)
if (controller.signal.aborted) return
lastSeamRequestAt.current = performance.now()
const mode = resolveLegMode(
{ isPlace: true, leg_transport_mode: seam.from.legMode },
{ isPlace: true, incoming_leg_transport_mode: seam.to.incomingLegMode },
@@ -496,7 +526,6 @@ export function useRoadtripRoutes(
// invented. Retried on the next run of this effect.
if (controller.signal.aborted) return
}
await sleep(REQUEST_SPACING_MS, controller.signal)
}
})()
return () => controller.abort()
client/src/pages/tripPlanner/useTripPlanner.roadtrip.test.tsx
+7 -3
View File
@@ -963,7 +963,7 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => {
expect(rt.alt.close).not.toHaveBeenCalled()
})
it('FE-TP-ROAD-040: another road is stored as a via where it differs most, not as a polyline', async () => {
it('FE-TP-ROAD-040: another road replaces the leg it reshapes, as a via rather than a polyline', async () => {
routedDay()
rt.alt.open = {
dayId: 5, index: 0, loading: false, error: false,
@@ -973,7 +973,11 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => {
await act(async () => { await result.current.chooseRouteAlternative(0) })
expect(rt.vias.add).toHaveBeenCalledWith(5, 0, 53.1, 11.9)
// Replacing, not appending: the alternatives were computed for the two bare
// endpoints, so a leg that still carries its old via routes somewhere the
// preview never drew.
expect(rt.vias.addMany).toHaveBeenCalledWith(5, [{ after_order_index: 0, lat: 53.1, lng: 11.9 }], [0])
expect(rt.vias.add).not.toHaveBeenCalled()
expect(rt.alt.close).toHaveBeenCalled()
})
@@ -983,7 +987,7 @@ describe('useTripPlanner road trip: other ways of driving a leg', () => {
dayId: 5, index: 0, loading: false, error: false,
routes: [{ divergence: { lat: 53.1, lng: 11.9 } }],
}
rt.vias.add.mockRejectedValue(new Error('rejected'))
rt.vias.addMany.mockRejectedValue(new Error('rejected'))
const { result } = await renderRoadtrip()
rt.alt.close.mockClear()
client/src/pages/tripPlanner/useTripPlanner.ts
+39 -10
View File
@@ -128,7 +128,23 @@ export function useTripPlanner() {
// drive-first reading of the same trip. Per trip and per session, like the tab choice:
// someone planning a road trip stays in it across reloads without it leaking into
// their next, non-driving trip.
const [roadtripMode, setRoadtripMode] = useState<boolean>(() => sessionStorage.getItem(`trip-roadtrip-${tripId}`) === '1')
const [storedRoadtripMode, setRoadtripMode] = useState<boolean>(() => sessionStorage.getItem(`trip-roadtrip-${tripId}`) === '1')
// Declared here rather than with the other layout state further down, because
// road-trip mode is decided on it and the assignment and place lists below are
// decided on that. One subscriber for the whole hook.
const [isMobile, setIsMobile] = useState(() => window.innerWidth < 768)
useEffect(() => {
const mq = window.matchMedia('(max-width: 767px)')
const handler = (e: MediaQueryListEvent) => setIsMobile(e.matches)
mq.addEventListener('change', handler)
return () => mq.removeEventListener('change', handler)
}, [])
// The phone shell has no road-trip surface at all: no rail, no drive lines, and
// no switch to turn the mode back off. Narrowing a desktop window past the
// phone breakpoint used to carry the flag across anyway, which took the trip
// overview pill away with nothing in its place and listed every booked night
// twice. The flag is kept, so widening the window again returns to the drive.
const roadtripMode = storedRoadtripMode && !isMobile
// Two reasons a stop can be road-trip-only, and they are not the same reason.
//
// The switch is the traveller's: it hides the petrol stations and rest areas
@@ -488,13 +504,6 @@ export function useTripPlanner() {
}, [alwaysShowRoutesDefault])
const [mapTransportDetail, setMapTransportDetail] = useState<Reservation | null>(null)
const [isMobile, setIsMobile] = useState(() => window.innerWidth < 768)
useEffect(() => {
const mq = window.matchMedia('(max-width: 767px)')
const handler = (e: MediaQueryListEvent) => setIsMobile(e.matches)
mq.addEventListener('change', handler)
return () => mq.removeEventListener('change', handler)
}, [])
// Layout is width-driven (isMobile); the drag bridge is pointer-driven (isTouch).
// Conflating them is what left a tablet's places list undraggable-but-unscrollable (#1432).
const isTouch = useIsTouch()
@@ -618,7 +627,14 @@ export function useTripPlanner() {
for (const boundary of dayBoundaries.boundaries) await dayBoundaries.save(boundary.day_number, null)
} catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) }
} : undefined
const roadtripVias = useRoadtripVias(tripId, roadtripActive)
// Fed whenever the addon is on, not only while the mode is being looked at.
// The three handlers that re-anchor a day's vias — assign, remove, reorder —
// are reachable from the place inspector in both modes, and with an empty list
// they computed an empty correction and wrote nothing while the server kept
// every via pointing at a position that had moved. Deleting a stop with the
// switch off then left the detour on the wrong leg, for the traveller and for
// a collaborator who never turned the mode on at all.
const roadtripVias = useRoadtripVias(tripId, !!enabledAddons.roadtrip)
const refuel = useRefuelSearch()
const roadtripRoutes = useRoadtripRoutes(
tripId,
@@ -1382,8 +1398,21 @@ export function useTripPlanner() {
const day = roadtripRoutes.days.find(d => d.dayId === open.dayId)
const stop = day?.stops[open.index]
if (!day || !stop) return
const ownerDayId = stop.ownerDayId ?? day.dayId
const legIndex = stop.ownerIndex ?? open.index
try {
await roadtripVias.add(stop.ownerDayId ?? day.dayId, stop.ownerIndex ?? open.index, alt.divergence.lat, alt.divergence.lng)
// Replaces the leg rather than appending to it. The alternatives under the
// button were computed for the two bare endpoints — the road already being
// driven is offered separately as `current` — so a leg that already carries
// a via cannot produce the line the preview drew. Appending put the new
// point behind the old one and routed A, south to the old via, north to the
// new one, then B: a zigzag matching neither the preview nor the distance
// printed on it. Same write the direct branch above already uses.
await roadtripVias.addMany(
ownerDayId,
[{ after_order_index: legIndex, lat: alt.divergence.lat, lng: alt.divergence.lng }],
[legIndex],
)
routeAlternatives.close()
} catch (err: unknown) {
toast.error(err instanceof Error ? err.message : t('common.unknownError'))