mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(roadtrip): stop one gesture writing twice, and one search running too early
Five things, all of them a click or a drag doing something the traveller did not ask for. The GL via handle committed a move on every pointerup. Any button started the drag and there was no movement threshold, so a plain click wrote the via to wherever inside the 17px handle the pointer happened to be — at a low zoom that is kilometres, and the write re-routes the whole trip. A right-click ran the drag AND the delete, so one gesture sent two writes; on the platforms where contextmenu comes first, the move landed on an id that no longer existed and reported a failure for a deletion that had worked. Leaflet gets this right for free from `dragend`, which is why the two renderers behaved differently for the same gesture. Primary button only now, with a threshold, and pointercancel puts the pin back instead of committing — the browser fires it when the captured element is torn out of the DOM, which is exactly what a delete does. With the alternatives picker open, one click on the route fired both handlers. Layer registrations are independent in MapLibre, and every alternative starts and ends at the same two stops — when the leg already carries vias the current road is itself one of the offers, so the overlap is the whole leg. The click dropped a via AND chose a different road: two writes and two trip reloads for a gesture made once. The corridor search could be started before the day had routed. The corridor is then built from the straight line between the stops rather than the roads driven, and the routing answer landing a second later changes the line, which clears the search mid-flight and drops every result — no error, no partial result, just the idle empty state again. Routing runs one day at a time about a second apart, so the window is wide open on a long trip. Leaving road trip mode left the alternatives drawn on the map. The switch is in the sidebar and reachable while the bar is open, and the overlay depends only on the picker, so pale blue routes with drive-time pills stayed on an ordinary planner map with no road trip UI left to dismiss them from. Clearing a leg's vias swallowed its failure and closed the picker anyway. Every neighbouring write in that hook reports one. The leg still carried its via and still routed the old way, so the traveller believed they had undone the detour. Walking and cycling legs were counted as driving. A day may legitimately mix modes — splitIntoRuns exists for that — and a two hour hike was reported as "over the longest drive allowed", its minutes went into the day's driving time and its kilometres onto the tank, where a ferry crossing could trip a range warning outright. A leg with no mode still counts, because every leg was a drive before the field existed. The road-trip domain also says out loud that it is online-only, which the repo asks for rather than leaving it to be discovered. A via anchors by POSITION, so a queued write replayed after somebody else moved a stop would re-pin the drive onto a leg nobody chose, silently and hours later; the offline story needs the anchor to be an assignment id first. Until then the surfaces disable the gesture instead of accepting one and dropping it. Six theme-lint markers move onto the line they exempt. The linter suppresses per line, so a marker in the comment block above suppressed nothing and the six road-signage colours the authors documented as intentional were counted anyway.
This commit is contained in:
@@ -711,10 +711,23 @@ export function MapViewGL({
|
||||
// sits on that surface. Stopping only pointerdown left both running at once: the
|
||||
// handle moved AND the map slid out from under it.
|
||||
const swallow = (e: Event) => e.stopPropagation()
|
||||
// Where the gesture began, so a click can be told from a drag. Without it
|
||||
// every pointerup committed a move to wherever inside the 17px handle the
|
||||
// pointer happened to be: at a low zoom that is kilometres, and the write
|
||||
// re-routes the whole trip. Leaflet gets this for free from `dragend`,
|
||||
// which is why the two renderers behaved differently for one gesture.
|
||||
let startedAt: { x: number; y: number } | null = null
|
||||
const MOVED_ENOUGH = 4
|
||||
const onDown = (e: PointerEvent) => {
|
||||
// Primary button only. Right-clicking ran onDown, then contextmenu ->
|
||||
// remove AND pointerup -> move, so one gesture sent two writes; on the
|
||||
// platforms where contextmenu comes first, the move landed on an id that
|
||||
// no longer existed and reported a failure for a deletion that worked.
|
||||
if (e.button !== 0) return
|
||||
e.stopPropagation()
|
||||
e.preventDefault()
|
||||
dragging = true
|
||||
startedAt = { x: e.clientX, y: e.clientY }
|
||||
el.setPointerCapture(e.pointerId)
|
||||
el.style.cursor = 'grabbing'
|
||||
}
|
||||
@@ -728,10 +741,29 @@ export function MapViewGL({
|
||||
if (!dragging) return
|
||||
dragging = false
|
||||
el.style.cursor = 'grab'
|
||||
const from = startedAt
|
||||
startedAt = null
|
||||
const travelled = from ? Math.hypot(e.clientX - from.x, e.clientY - from.y) : 0
|
||||
if (travelled < MOVED_ENOUGH) {
|
||||
// A click, not a drag. Put the pin back where the via actually is: the
|
||||
// move handler has been following the pointer across the handle.
|
||||
pin.setLngLat([via.lng, via.lat])
|
||||
return
|
||||
}
|
||||
const rect = map.getContainer().getBoundingClientRect()
|
||||
const at = map.unproject([e.clientX - rect.left, e.clientY - rect.top])
|
||||
viaHandlersRef.current.onMoveVia?.(via.day_id, via.id, at.lat, at.lng)
|
||||
}
|
||||
// The browser fires this when the captured element is torn out of the DOM,
|
||||
// which is exactly what a delete does. Committing a position there wrote a
|
||||
// move for a via that was being removed.
|
||||
const onCancel = () => {
|
||||
if (!dragging) return
|
||||
dragging = false
|
||||
startedAt = null
|
||||
el.style.cursor = 'grab'
|
||||
pin.setLngLat([via.lng, via.lat])
|
||||
}
|
||||
const onContext = (e: MouseEvent) => {
|
||||
e.preventDefault()
|
||||
viaHandlersRef.current.onRemoveVia?.(via.day_id, via.id)
|
||||
@@ -739,7 +771,7 @@ export function MapViewGL({
|
||||
el.addEventListener('pointerdown', onDown)
|
||||
el.addEventListener('pointermove', onMove)
|
||||
el.addEventListener('pointerup', onUp)
|
||||
el.addEventListener('pointercancel', onUp)
|
||||
el.addEventListener('pointercancel', onCancel)
|
||||
el.addEventListener('contextmenu', onContext)
|
||||
el.addEventListener('mousedown', swallow)
|
||||
el.addEventListener('touchstart', swallow, { passive: true })
|
||||
@@ -748,7 +780,7 @@ export function MapViewGL({
|
||||
el.removeEventListener('pointerdown', onDown)
|
||||
el.removeEventListener('pointermove', onMove)
|
||||
el.removeEventListener('pointerup', onUp)
|
||||
el.removeEventListener('pointercancel', onUp)
|
||||
el.removeEventListener('pointercancel', onCancel)
|
||||
el.removeEventListener('contextmenu', onContext)
|
||||
el.removeEventListener('mousedown', swallow)
|
||||
el.removeEventListener('touchstart', swallow)
|
||||
@@ -870,8 +902,17 @@ export function MapViewGL({
|
||||
useEffect(() => {
|
||||
const map = mapRef.current
|
||||
if (!map || !mapReady || !onRouteClick) return
|
||||
const onClick = (e: { lngLat: { lat: number; lng: number }; preventDefault?: () => void }) => {
|
||||
const onClick = (e: { lngLat: { lat: number; lng: number }; point?: { x: number; y: number }; preventDefault?: () => void }) => {
|
||||
e.preventDefault?.()
|
||||
// Layer handlers are independent: MapLibre evaluates each registration
|
||||
// against the same click, so a point that hits both the offered-route band
|
||||
// and the current-route band fires both. Every alternative starts and ends
|
||||
// at the same two stops, and when the leg already carries vias the current
|
||||
// road is itself one of the offers, so the overlap is the whole leg. One
|
||||
// click then dropped a via AND chose a different road, two writes and two
|
||||
// trip reloads for a gesture the user made once. The picker is a modal
|
||||
// choice about that leg; dropping a via mid-choice is not offered anywhere.
|
||||
if (e.point && map.queryRenderedFeatures?.(e.point, { layers: ['route-alt-hit'] })?.length) return
|
||||
onRouteClick(e.lngLat.lat, e.lngLat.lng)
|
||||
}
|
||||
const enter = () => { map.getCanvas().style.cursor = 'copy' }
|
||||
|
||||
@@ -66,7 +66,7 @@ export default function RoadtripCategoryPicker({ keys, meta, selected, onToggle
|
||||
key={k}
|
||||
className="grid h-[18px] w-[18px] place-items-center rounded-md"
|
||||
// theme-lint-disable — the road-signage palette in `roadtripModel`.
|
||||
style={{ background: serviceColor(k), color: '#fff' }}
|
||||
style={{ background: serviceColor(k), color: '#fff' }} // theme-lint-disable — road-signage palette
|
||||
>
|
||||
<Icon size={10} strokeWidth={2.2} aria-hidden />
|
||||
</span>
|
||||
@@ -105,8 +105,7 @@ export default function RoadtripCategoryPicker({ keys, meta, selected, onToggle
|
||||
>
|
||||
<span
|
||||
className="grid h-[22px] w-[22px] shrink-0 place-items-center rounded-[7px]"
|
||||
// theme-lint-disable — see above.
|
||||
style={on ? { background: serviceColor(key), color: '#fff' } : { background: `${serviceColor(key)}1f`, color: serviceColor(key) }}
|
||||
style={on ? { background: serviceColor(key), color: '#fff' } : { background: `${serviceColor(key)}1f`, color: serviceColor(key) }} // theme-lint-disable — road-signage palette
|
||||
>
|
||||
<Icon size={12} strokeWidth={2} aria-hidden />
|
||||
</span>
|
||||
|
||||
@@ -31,7 +31,10 @@ const day = (dayId: number, dayNumber: number): RoadtripDay => ({
|
||||
legs: [],
|
||||
schedule: { entries: [], warnings: [] },
|
||||
legVias: [], driveWarnings: [], dayWarning: null,
|
||||
geometry: [],
|
||||
// Routed. Searching before the day has a geometry builds the corridor from the
|
||||
// straight line between the stops, and the routing answer landing a second
|
||||
// later clears the search mid-flight — so the button waits for this.
|
||||
geometry: [[53.55, 9.99], [52.52, 13.4]] as [number, number][],
|
||||
distance: 0,
|
||||
duration: 0,
|
||||
})
|
||||
@@ -120,6 +123,27 @@ describe('RoadtripCorridorPanel', () => {
|
||||
expect(c.search.search).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-PANEL-004: a day that has not routed yet cannot be searched along', () => {
|
||||
// Until the day routes, the corridor is the straight line between the stops,
|
||||
// so the boxes march across whatever lies between them instead of along the
|
||||
// roads driven. Worse, the routing answer landing a second later changes the
|
||||
// line, which clears the search mid-flight and drops every result with no
|
||||
// error and no explanation. Routing runs one day at a time, about a second
|
||||
// apart, so on a long trip that window is wide open.
|
||||
const unrouted = { ...day(1, 1), geometry: [] as [number, number][] }
|
||||
const c = corridor({ day: unrouted })
|
||||
wrap(<RoadtripCorridorPanel corridor={c} routes={routes([unrouted])} />)
|
||||
|
||||
expect(screen.getByRole('button', { name: /search/i })).toBeDisabled()
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-PANEL-005: nor while the routing is still running', () => {
|
||||
const c = corridor()
|
||||
wrap(<RoadtripCorridorPanel corridor={c} routes={{ ...routes([day(1, 1)]), loading: true }} />)
|
||||
|
||||
expect(screen.getByRole('button', { name: /search/i })).toBeDisabled()
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-PANEL-002: nothing selected means nothing to search for', () => {
|
||||
const c = corridor({ categories: [] })
|
||||
wrap(<RoadtripCorridorPanel corridor={c} routes={routes([day(1, 1)])} />)
|
||||
|
||||
@@ -173,7 +173,7 @@ function ResultGroup({ category, pois, dayId, insertIndexFor, onAddPoi }: {
|
||||
<span
|
||||
className="grid h-[22px] w-[22px] shrink-0 place-items-center rounded-[7px]"
|
||||
// theme-lint-disable — see ResultBadge.
|
||||
style={{ background: color, color: '#fff' }}
|
||||
style={{ background: color, color: '#fff' }} // theme-lint-disable — road-signage palette
|
||||
>
|
||||
<Icon size={12} strokeWidth={2} aria-hidden />
|
||||
</span>
|
||||
@@ -244,7 +244,19 @@ export default function RoadtripCorridorPanel({ corridor, routes, onAddPoi }: Ro
|
||||
|
||||
const filteredToNothing = search.results.length > 0 && corridor.visible.length === 0
|
||||
|
||||
const canSearch = !search.loading && corridor.categories.length > 0 && (corridor.day?.stops.length ?? 0) > 1
|
||||
// The day has to have routed. Until it does, the corridor is built from the
|
||||
// straight line between the stops, so the boxes march across whatever lies
|
||||
// between them rather than along the roads actually driven — and when the
|
||||
// routing answer lands a second later the line changes, which clears the
|
||||
// search mid-flight and drops every result with no error and no explanation.
|
||||
// The routing runs one day at a time, about a second apart, so on a long trip
|
||||
// that window is wide open for the day somebody picks.
|
||||
const dayRouted = (corridor.day?.geometry.length ?? 0) > 1
|
||||
const canSearch = !search.loading
|
||||
&& !routes.loading
|
||||
&& dayRouted
|
||||
&& corridor.categories.length > 0
|
||||
&& (corridor.day?.stops.length ?? 0) > 1
|
||||
const progressPct = search.progress.total
|
||||
? Math.round((search.progress.done / search.progress.total) * 100)
|
||||
: 0
|
||||
|
||||
@@ -416,7 +416,7 @@ function ServiceStop({ stop, entry, driveFindings, selected, onSelect, onEditSta
|
||||
// theme-lint-disable — the road-signage palette in `roadtripModel`, shared
|
||||
// with the corridor list and the map pin so one kind of stop looks like
|
||||
// itself wherever it turns up.
|
||||
style={{ background: serviceColor(stop.stopType), color: '#fff' }}
|
||||
style={{ background: serviceColor(stop.stopType), color: '#fff' }} // theme-lint-disable — road-signage palette
|
||||
>
|
||||
<Icon size={12} strokeWidth={2.1} aria-hidden />
|
||||
</span>
|
||||
@@ -425,7 +425,7 @@ function ServiceStop({ stop, entry, driveFindings, selected, onSelect, onEditSta
|
||||
<span
|
||||
className={DISC}
|
||||
// theme-lint-disable — same palette, read-only.
|
||||
style={{ background: serviceColor(stop.stopType), color: '#fff' }}
|
||||
style={{ background: serviceColor(stop.stopType), color: '#fff' }} // theme-lint-disable — road-signage palette
|
||||
>
|
||||
<Icon size={12} strokeWidth={2.1} aria-label={label} />
|
||||
</span>
|
||||
|
||||
@@ -105,7 +105,7 @@ export default function StopKindPicker({ anchor, current, onPick, onClose }: {
|
||||
// theme-lint-disable — the road-signage palette from `roadtripModel`, the
|
||||
// same one the rail disc and the map pin use, so the choice looks like
|
||||
// what it will become.
|
||||
style={{ background: color, color: '#fff', ...(on ? { boxShadow: `0 0 0 2px ${color}` } : {}) }}
|
||||
style={{ background: color, color: '#fff', ...(on ? { boxShadow: `0 0 0 2px ${color}` } : {}) }} // theme-lint-disable — road-signage palette
|
||||
>
|
||||
<Icon size={14} strokeWidth={2.2} aria-hidden />
|
||||
</span>
|
||||
|
||||
@@ -490,3 +490,58 @@ describe('sectionAnchors', () => {
|
||||
expect(sectionAnchors([])).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('deriveDriveWarnings — what counts as driving', () => {
|
||||
const drive = (minutes: number, km: number) =>
|
||||
({ duration: minutes * 60, distance: km * 1000, mode: 'driving' })
|
||||
const onFoot = (minutes: number, km: number) =>
|
||||
({ duration: minutes * 60, distance: km * 1000, mode: 'walking' })
|
||||
|
||||
it('FE-ROADTRIP-MODEL-085: a walk to a viewpoint is not over the longest drive allowed', () => {
|
||||
// A day may legitimately mix modes — splitIntoRuns exists for exactly that —
|
||||
// and a two hour hike reported as a leg finding reads as a fault in a plan
|
||||
// that has none.
|
||||
const out = deriveDriveWarnings(
|
||||
[drive(60, 80), onFoot(120, 6)],
|
||||
[false, false, false],
|
||||
{ legMinutes: 90, dayMinutes: null, rangeKm: null },
|
||||
0,
|
||||
)
|
||||
expect(out.warnings).toEqual([])
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-MODEL-086: walked kilometres are not on the tank', () => {
|
||||
// Nor does walking fill it: the budget is left exactly as the drive left it.
|
||||
const out = deriveDriveWarnings(
|
||||
[drive(60, 80), onFoot(120, 60), drive(30, 30)],
|
||||
[false, false, false, false],
|
||||
{ legMinutes: null, dayMinutes: null, rangeKm: 100 },
|
||||
0,
|
||||
)
|
||||
// 80 then 110 — the 60 walked kilometres never entered the budget.
|
||||
expect(out.warnings).toEqual([{ index: 3, code: 'range', sinceKm: 110 }])
|
||||
expect(out.carryKm).toBe(110)
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-MODEL-087: a walk does not count toward the driving time of the day', () => {
|
||||
const out = deriveDriveWarnings(
|
||||
[drive(60, 80), onFoot(180, 9)],
|
||||
[false, false, false],
|
||||
{ legMinutes: null, dayMinutes: 90, rangeKm: null },
|
||||
0,
|
||||
)
|
||||
expect(out.day).toBeNull()
|
||||
})
|
||||
|
||||
it('FE-ROADTRIP-MODEL-088: a leg with no mode is still a drive', () => {
|
||||
// Every leg was a drive before the field existed, so an absent mode counts.
|
||||
// Reading it the other way round would silently switch the warnings off.
|
||||
const out = deriveDriveWarnings(
|
||||
[{ duration: 120 * 60, distance: 200 * 1000 }],
|
||||
[false, false],
|
||||
{ legMinutes: 90, dayMinutes: null, rangeKm: null },
|
||||
0,
|
||||
)
|
||||
expect(out.warnings).toEqual([{ index: 1, code: 'leg', overMinutes: 30 }])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -329,7 +329,16 @@ export function sumLegSeconds(legSeconds: (number | undefined)[]): number {
|
||||
* schedule abandons its cursor: a distance we do not have cannot be added to one we do.
|
||||
*/
|
||||
export function deriveDriveWarnings(
|
||||
legs: ({ duration?: number; distance?: number } | undefined)[],
|
||||
/**
|
||||
* `mode` decides whether a leg counts at all. A day may legitimately mix
|
||||
* modes — splitIntoRuns exists for exactly that — and a two hour walk to a
|
||||
* viewpoint is not two hours of driving, nor are its kilometres on the tank.
|
||||
* Counting them reported a hike as "over the longest drive allowed" and let a
|
||||
* ferry crossing trip a range warning. Anything that is not a driving profile
|
||||
* is skipped; an absent mode counts, because every leg was a drive before the
|
||||
* field existed.
|
||||
*/
|
||||
legs: ({ duration?: number; distance?: number; mode?: string } | undefined)[],
|
||||
/** Whether the stop at each index refuels. One entry longer than `legs`. */
|
||||
refuelsAt: boolean[],
|
||||
limits: DriveLimits,
|
||||
@@ -345,6 +354,11 @@ export function deriveDriveWarnings(
|
||||
const leg = legs[i]
|
||||
// The stop this leg arrives at. Both findings are about what is true on arrival.
|
||||
const at = i + 1
|
||||
// Not a drive: neither its minutes nor its kilometres belong in findings
|
||||
// about the driving. The budget is left exactly as it was — walking to a
|
||||
// viewpoint and back does not use fuel, and it does not fill the tank
|
||||
// either.
|
||||
if (leg && leg.mode !== undefined && leg.mode !== 'driving') continue
|
||||
const seconds = leg?.duration
|
||||
if (typeof seconds === 'number') {
|
||||
totalSeconds += seconds
|
||||
|
||||
@@ -49,6 +49,7 @@ function viasStub(over: Partial<RoadtripVias> = {}): RoadtripVias {
|
||||
byDay: {},
|
||||
trackByDay: {},
|
||||
stale: false,
|
||||
editable: true,
|
||||
add: vi.fn(),
|
||||
addMany: vi.fn().mockResolvedValue(undefined),
|
||||
move: vi.fn(),
|
||||
|
||||
@@ -1,8 +1,27 @@
|
||||
import { useCallback, useEffect, useState } from 'react'
|
||||
import { roadtripApi } from '../../api/client'
|
||||
import { useNetworkMode } from '../../hooks/useNetworkMode'
|
||||
import { isEmptyReanchoring, type Reanchoring } from './roadtripModel'
|
||||
import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared'
|
||||
|
||||
/**
|
||||
* Via points are DELIBERATELY online-only, which the repo asks to be said out
|
||||
* loud rather than left to be discovered.
|
||||
*
|
||||
* Everything else on this screen goes through a repo and the mutation queue:
|
||||
* optimistic Dexie write, temporary negative id, idempotent replay on
|
||||
* reconnect. A via cannot follow that shape as it stands, because its anchor is
|
||||
* a POSITION rather than an id — `after_order_index` counts stops as the routing
|
||||
* request builds them — so a queued write replayed after somebody else has
|
||||
* added or removed a stop would re-pin the drive onto a leg nobody chose, and it
|
||||
* would do so silently, hours later. The correct offline story needs the anchor
|
||||
* to be an assignment id first, which is a change to the wire contract and to
|
||||
* the migration, not to this hook.
|
||||
*
|
||||
* Until then the honest behaviour is to say so: `editable` is false with no
|
||||
* network, so the map stops offering a drag it cannot keep, instead of failing
|
||||
* one write at a time and losing the edit.
|
||||
*/
|
||||
export interface RoadtripVias {
|
||||
/** Every via of the trip, keyed by day. */
|
||||
byDay: Record<number, RoadtripVia[]>
|
||||
@@ -24,6 +43,14 @@ export interface RoadtripVias {
|
||||
* them. The list is kept and the staleness is said instead.
|
||||
*/
|
||||
stale: boolean
|
||||
/**
|
||||
* Whether shaping the drive is possible right now.
|
||||
*
|
||||
* False with no network: the domain is online-only (see the note above this
|
||||
* interface), so the surfaces disable the gesture rather than accepting one
|
||||
* and dropping it.
|
||||
*/
|
||||
editable: boolean
|
||||
add: (dayId: number, afterOrderIndex: number, lat: number, lng: number) => Promise<void>
|
||||
/**
|
||||
* Lay a chain of vias on one day, optionally clearing the legs it fills first.
|
||||
@@ -76,6 +103,9 @@ export function useRoadtripVias(tripId: number | string | null, active: boolean)
|
||||
* quietly showing a trip that has lost its detours.
|
||||
*/
|
||||
const [stale, setStale] = useState(false)
|
||||
// Read through the app's single source of truth for connectivity, which also
|
||||
// covers the offline switch a user can set with the network still up.
|
||||
const { offline } = useNetworkMode()
|
||||
|
||||
const group = useCallback((vias: RoadtripVia[]) => {
|
||||
const next: Record<number, RoadtripVia[]> = {}
|
||||
@@ -159,5 +189,5 @@ export function useRoadtripVias(tripId: number | string | null, active: boolean)
|
||||
await reload()
|
||||
}, [tripId, reload])
|
||||
|
||||
return { byDay, trackByDay, stale, add, addMany, move, remove, reanchor }
|
||||
return { byDay, trackByDay, stale, editable: !offline, add, addMany, move, remove, reanchor }
|
||||
}
|
||||
|
||||
@@ -1034,6 +1034,15 @@ export function useTripPlanner() {
|
||||
if (!routeAlternatives.open) setHighlightedAlternative(null)
|
||||
}, [routeAlternatives.open])
|
||||
|
||||
// Leaving road trip mode closes it too. The switch sits in the left sidebar and
|
||||
// is reachable while the bar is open over the map, and the overlay depends only
|
||||
// on the picker — so flipping the mode off left pale blue alternatives, their
|
||||
// casings and their drive-time pills drawn on an ordinary planner map, with no
|
||||
// road trip UI left to dismiss them from.
|
||||
useEffect(() => {
|
||||
if (!roadtripActive) routeAlternatives.close()
|
||||
}, [roadtripActive, routeAlternatives])
|
||||
|
||||
/**
|
||||
* The offered routes as the map draws them: line, colour, and the label that sits on
|
||||
* the road. Built here rather than in the page so the page stays a wiring container
|
||||
@@ -1103,7 +1112,19 @@ export function useTripPlanner() {
|
||||
const dayIndex = stop && day ? day.stops.indexOf(stop) : -1
|
||||
// Clearing the leg in one write. One delete per via meant a full trip re-route
|
||||
// between each of them, so undoing a detour with three vias drew three routes.
|
||||
if (dayIndex >= 0) await roadtripVias.addMany(open.dayId, [], [dayIndex]).catch(() => {})
|
||||
//
|
||||
// Reported like every other write in this hook. Swallowing it closed the
|
||||
// picker on a leg that still carries its via and still routes the old way,
|
||||
// so the traveller believed they had undone the detour — and because the
|
||||
// request failed, not even the reload ran to contradict them.
|
||||
if (dayIndex >= 0) {
|
||||
try {
|
||||
await roadtripVias.addMany(open.dayId, [], [dayIndex])
|
||||
} catch (err: unknown) {
|
||||
toast.error(err instanceof Error ? err.message : t('common.unknownError'))
|
||||
return
|
||||
}
|
||||
}
|
||||
routeAlternatives.close()
|
||||
return
|
||||
}
|
||||
|
||||
@@ -447,7 +447,8 @@ describe('Tool: search_pois', () => {
|
||||
expect(data.pois[0].name).toBe('Chez Nous');
|
||||
expect(data.source).toBe('openstreetmap');
|
||||
expect(trekNearbyMock).toHaveBeenCalled();
|
||||
expect(MapsService.prototype.searchOverpassPois).toHaveBeenCalledWith('restaurant', BBOX, 'fr');
|
||||
// The caller's per-category budget rides along on the fallback too.
|
||||
expect(MapsService.prototype.searchOverpassPois).toHaveBeenCalledWith('restaurant', BBOX, 'fr', 60);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -497,7 +498,9 @@ describe('Tool: search_pois', () => {
|
||||
expect(lat).toBeCloseTo(48.86, 5);
|
||||
expect(lng).toBeCloseTo(2.345, 5);
|
||||
expect(opts?.category).toBe('restaurant,casual_eatery,fast_food');
|
||||
expect(opts?.limit).toBe(50);
|
||||
// Sixty per category, the same allowance the Overpass path spends, rather
|
||||
// than a number this branch made up for itself.
|
||||
expect(opts?.limit).toBe(60);
|
||||
expect(opts?.radius).toBeGreaterThan(300);
|
||||
expect(opts?.radius).toBeLessThanOrEqual(20000);
|
||||
});
|
||||
@@ -532,7 +535,7 @@ describe('Tool: search_pois', () => {
|
||||
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'search_pois', arguments: { category: 'museum', bbox: BBOX } });
|
||||
expect(MapsService.prototype.searchOverpassPois).toHaveBeenCalledWith('museum', BBOX, undefined);
|
||||
expect(MapsService.prototype.searchOverpassPois).toHaveBeenCalledWith('museum', BBOX, undefined, 60);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -2698,7 +2698,9 @@ describe('controller-facing wrappers delegate to the folded methods', () => {
|
||||
// The index answers with an empty page (see the mock above), so what this
|
||||
// measures is the wrapper's own delegation.
|
||||
await svc.pois('cafe', bbox, 'de');
|
||||
expect(spies.searchOverpassPois).toHaveBeenCalledWith('cafe', bbox, 'de');
|
||||
// The caller's per-category budget rides along, so a corridor search that
|
||||
// falls through to Overpass gets the allowance it asked the index for.
|
||||
expect(spies.searchOverpassPois).toHaveBeenCalledWith('cafe', bbox, 'de', 60);
|
||||
} finally {
|
||||
Object.values(spies).forEach((s) => s.mockRestore());
|
||||
}
|
||||
|
||||
@@ -587,10 +587,12 @@ describe('toPlaceRecord', () => {
|
||||
describe('POI_CATEGORY_TO_TREK', () => {
|
||||
it('covers every pill the planner offers', () => {
|
||||
// The keys are the contract with the client's POI_CATEGORIES. A missing one
|
||||
// silently drops that pill back to Overpass, which is the slow path.
|
||||
// silently drops that pill back to Overpass, which is the slow path — and
|
||||
// for the four a drive needs that is the worst path of all, because the
|
||||
// corridor search asks for them sixteen boxes at a time.
|
||||
expect(Object.keys(POI_CATEGORY_TO_TREK).sort()).toEqual([
|
||||
'activity', 'bar', 'cafe', 'hotel', 'museum',
|
||||
'nature', 'restaurant', 'shopping', 'sights', 'supermarket',
|
||||
'activity', 'bar', 'cafe', 'campsite', 'charging', 'fuel', 'hotel', 'museum',
|
||||
'nature', 'rest_area', 'restaurant', 'shopping', 'sights', 'supermarket',
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user