diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index bd24c97cd..34918d0e8 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -9,6 +9,7 @@ import 'leaflet.markercluster/dist/MarkerCluster.Default.css' import { mapsApi } from '../../api/client' import { getCategoryIcon, CATEGORY_ICON_MAP } from '../shared/categoryIcons' import ReservationOverlay from './ReservationOverlay' +import { useTransportRoutes } from '../../hooks/useTransportRoutes' import type { Reservation } from '../../types' import { POI_CATEGORY_BY_KEY, type Poi } from './poiCategories' @@ -468,6 +469,8 @@ export const MapView = memo(function MapView({ // day route, so hiding the route hides them too (#1065). return reservations.filter((r: Reservation) => (r.type === 'transit' && showTransitRoutes) || set.has(r.id)) }, [reservations, visibleConnectionIds, showTransitRoutes]) + // Real road geometry for car/bus/taxi/bicycle bookings (straight line until it loads/if it fails). + const transportRoutes = useTransportRoutes(visibleReservations) // Dynamic padding: account for sidebars + bottom inspector + day detail panel const paddingOpts = useMemo((): L.FitBoundsOptions => { const isMobile = typeof window !== 'undefined' && window.innerWidth < 768 @@ -712,6 +715,7 @@ export const MapView = memo(function MapView({ showConnections showStats={showReservationStats} onEndpointClick={onReservationClick} + roadRoutes={transportRoutes} /> {poiMarkers} diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index f16b84962..cc79f6e78 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -11,6 +11,7 @@ import { CATEGORY_ICON_MAP } from '../shared/categoryIcons' import { isStandardFamily, supportsCustom3d, wantsTerrain, addCustom3dBuildings, addTerrainAndSky } from './mapboxSetup' import { attachLocationMarker, type LocationMarkerHandle } from './locationMarkerMapbox' import { ReservationMapboxOverlay } from './reservationsMapbox' +import { useTransportRoutes } from '../../hooks/useTransportRoutes' import { MAPBOX_DEFAULT_STYLE, styleForActiveProvider, basemapLanguage, type GlMapProvider } from './glProviders' import LocationButton from './LocationButton' import { useGeolocation } from '../../hooks/useGeolocation' @@ -903,6 +904,8 @@ export function MapViewGL({ // day route, so hiding the route hides them too (#1065). return reservations.filter(r => (r.type === 'transit' && showTransitRoutes) || set.has(r.id)) }, [reservations, visibleConnectionIds, showTransitRoutes]) + // Real road geometry for car/bus/taxi/bicycle bookings (straight line until it loads/if it fails). + const transportRoutes = useTransportRoutes(visibleReservations) useEffect(() => { const map = mapRef.current @@ -920,8 +923,8 @@ export function MapViewGL({ showStats: showReservationStats, showEndpointLabels, onEndpointClick: (id) => onReservationClickRef.current?.(id), - }) - }, [visibleReservations, showReservationStats, showEndpointLabels, mapReady, glProvider]) + }, transportRoutes) + }, [visibleReservations, transportRoutes, showReservationStats, showEndpointLabels, mapReady, glProvider]) // Fit bounds on fitKey change — matches the Leaflet BoundsController const paddingOpts = useMemo(() => { diff --git a/client/src/components/Map/ReservationOverlay.tsx b/client/src/components/Map/ReservationOverlay.tsx index fec3a3007..d1cdb2128 100644 --- a/client/src/components/Map/ReservationOverlay.tsx +++ b/client/src/components/Map/ReservationOverlay.tsx @@ -308,9 +308,12 @@ interface Props { showConnections: boolean showStats: boolean onEndpointClick?: (reservationId: number) => void + // Real road-network geometry for car/bus/taxi/bicycle bookings, keyed by + // reservation id. When present it is drawn instead of the straight arc. + roadRoutes?: Map } -export default function ReservationOverlay({ reservations, showConnections, showStats, onEndpointClick }: Props) { +export default function ReservationOverlay({ reservations, showConnections, showStats, onEndpointClick, roadRoutes }: Props) { useEndpointPane() const map = useMap() const [zoom, setZoom] = useState(() => map.getZoom()) @@ -390,35 +393,41 @@ export default function ReservationOverlay({ reservations, showConnections, show return ( <> - {visibleItems.map(item => item.transitSegs.length > 0 - ? item.transitSegs.map((seg, segIdx) => ( - - {!seg.walk && ( + {visibleItems.map(item => { + if (item.transitSegs.length > 0) { + return item.transitSegs.map((seg, segIdx) => ( + + {!seg.walk && ( + + )} - )} - - + + )) + } + // Prefer the real road route (car/bus/taxi/bicycle) over the straight arc. + const road = roadRoutes?.get(item.res.id) + const lines = road && road.length >= 2 ? [road] : item.arcs + return lines.map((seg, segIdx) => ( + )) - : item.arcs.map((seg, segIdx) => ( - - )))} + })} {visibleItems.flatMap(item => item.waypoints.map((wp, wi) => ( source, + addSource: vi.fn(), + addLayer: vi.fn(), + getLayer: () => undefined, + removeLayer: vi.fn(), + removeSource: vi.fn(), + on: vi.fn(), + off: vi.fn(), + getZoom: () => 12, + project: ([lng, lat]: [number, number]) => ({ x: lng * 1000, y: lat * 1000 }), + } +} + +const FakeMarker = vi.fn(function () { + const marker = { + setLngLat: () => marker, + addTo: () => marker, + remove: vi.fn(), + getElement: () => document.createElement('div'), + } + return marker +}) as unknown as new () => unknown + +function carBooking(): Reservation { + return { + id: 1, type: 'car', status: 'confirmed', + endpoints: [ + { role: 'from', sequence: 0, name: 'A', code: null, lat: 48.0, lng: 2.0, timezone: null, local_time: null, local_date: null }, + { role: 'to', sequence: 1, name: 'B', code: null, lat: 48.2, lng: 2.3, timezone: null, local_time: null, local_date: null }, + ], + } as unknown as Reservation +} + +const opts = { showConnections: true, showStats: false, showEndpointLabels: false } + +function lastFeatureCoords(map: ReturnType) { + const calls = map._source.setData.mock.calls + const data = calls[calls.length - 1]?.[0] as { features: { geometry: { coordinates: [number, number][] } }[] } + return data.features[0].geometry.coordinates +} + +describe('ReservationMapboxOverlay road routes (#1425)', () => { + it('draws the real road geometry when a road route is supplied', () => { + const map = fakeMap() + const overlay = new ReservationMapboxOverlay(map as never, opts, FakeMarker as never) + const road: [number, number][] = [[48.0, 2.0], [48.1, 2.15], [48.2, 2.3]] + overlay.update([carBooking()], opts, new Map([[1, road]])) + // GeoJSON is [lng, lat]; the routed 3-point line, not the straight 2-point arc. + expect(lastFeatureCoords(map)).toEqual([[2.0, 48.0], [2.15, 48.1], [2.3, 48.2]]) + }) + + it('falls back to the straight arc when no road route is supplied', () => { + const map = fakeMap() + const overlay = new ReservationMapboxOverlay(map as never, opts, FakeMarker as never) + overlay.update([carBooking()], opts) + expect(lastFeatureCoords(map)).toEqual([[2.0, 48.0], [2.3, 48.2]]) + }) + + it('sets no line features while connections are hidden', () => { + const map = fakeMap() + const overlay = new ReservationMapboxOverlay(map as never, opts, FakeMarker as never) + overlay.update([carBooking()], { ...opts, showConnections: false }, new Map([[1, [[48, 2], [48.2, 2.3]]]])) + const calls = map._source.setData.mock.calls + const data = calls[calls.length - 1]?.[0] as { features: unknown[] } + expect(data.features).toHaveLength(0) + }) +}) diff --git a/client/src/components/Map/reservationsMapbox.ts b/client/src/components/Map/reservationsMapbox.ts index 7b1c62ab7..5e87f79cd 100644 --- a/client/src/components/Map/reservationsMapbox.ts +++ b/client/src/components/Map/reservationsMapbox.ts @@ -203,6 +203,7 @@ type MarkerConstructor = new (options?: { element?: HTMLElement; anchor?: string export class ReservationMapboxOverlay { private map: mapboxgl.Map private items: TransportItem[] = [] + private roadRoutes: Map = new Map() private opts: ReservationOverlayOptions private MarkerCtor: MarkerConstructor private endpointMarkers: GlMarker[] = [] @@ -221,9 +222,10 @@ export class ReservationMapboxOverlay { map.on('render', this.updateStatsRotation) } - update(reservations: Reservation[], opts: ReservationOverlayOptions) { + update(reservations: Reservation[], opts: ReservationOverlayOptions, roadRoutes?: Map) { this.opts = opts this.items = buildItems(reservations) + this.roadRoutes = roadRoutes ?? new Map() this.render() } @@ -326,7 +328,10 @@ export class ReservationMapboxOverlay { }, })) } - return item.arcs.map(seg => ({ + // Prefer the real road route (car/bus/taxi/bicycle) over the straight arc. + const road = this.roadRoutes.get(item.res.id) + const lines = road && road.length >= 2 ? [road] : item.arcs + return lines.map(seg => ({ type: 'Feature' as const, properties: { resId: item.res.id, diff --git a/client/src/hooks/useTransportRoutes.ts b/client/src/hooks/useTransportRoutes.ts new file mode 100644 index 000000000..0516431e4 --- /dev/null +++ b/client/src/hooks/useTransportRoutes.ts @@ -0,0 +1,103 @@ +import { useEffect, useRef, useState } from 'react' +import type { Reservation, ReservationEndpoint } from '../types' +import { calculateRouteWithLegs } from '../components/Map/RouteCalculator' + +/** + * Real road-network geometry for road-based transport bookings (car, bus, taxi, + * bicycle), so their map lines follow actual streets instead of a straight + * as-the-crow-flies line — the same idea as the real transit paths (#1065), + * but routed on demand rather than stored on the reservation. + * + * Trains and "other transport" keep their straight line (rail/unknown modes + * aren't road-routable); flights/cruises/ferries use the great-circle arc. + * Any routing failure falls back to the straight line the overlay already draws. + */ + +const ROAD_PROFILE: Record = { + car: 'driving', + bus: 'driving', + taxi: 'driving', + bicycle: 'cycling', +} + +// Beyond this straight-line distance a car/taxi/bike (or even coach) booking is +// almost always a data quirk or an inter-continental hop the road router can't +// resolve — keep the straight line and don't hammer the public OSRM demo. +const MAX_ROUTE_KM = 2000 + +function haversineKm(a: { lat: number; lng: number }, b: { lat: number; lng: number }): number { + const R = 6371 + const dLat = (b.lat - a.lat) * Math.PI / 180 + const dLng = (b.lng - a.lng) * Math.PI / 180 + const la1 = a.lat * Math.PI / 180 + const la2 = b.lat * Math.PI / 180 + const h = Math.sin(dLat / 2) ** 2 + Math.cos(la1) * Math.cos(la2) * Math.sin(dLng / 2) ** 2 + return 2 * R * Math.asin(Math.sqrt(h)) +} + +function orderedWaypoints(r: Reservation): ReservationEndpoint[] { + return (r.endpoints || []) + .filter(e => e.role === 'from' || e.role === 'to' || e.role === 'stop') + .slice() + .sort((a, b) => (a.sequence ?? 0) - (b.sequence ?? 0)) +} + +/** + * Returns a map of reservation id → routed [lat, lng] polyline for the + * road-based bookings among `reservations`. Missing entries mean "not routed + * (yet / not routable)" — the caller should draw its straight line for those. + * Routing runs once per reservation waypoint-set and is cached across the app + * by RouteCalculator, so day switches and re-renders don't re-fetch. + */ +export function useTransportRoutes(reservations: Reservation[]): Map { + const [routes, setRoutes] = useState>(new Map()) + // id → waypoint signature already fetched/attempted, so an unchanged booking + // is never re-requested even as the reservations array identity churns. + const attemptedRef = useRef>(new Map()) + // Aborted only on unmount — a reservations change must not cancel an + // in-flight route we still want. + const abortRef = useRef(null) + if (!abortRef.current) abortRef.current = new AbortController() + useEffect(() => () => abortRef.current?.abort(), []) + + useEffect(() => { + const jobs: { id: number; profile: 'driving' | 'cycling'; points: { lat: number; lng: number }[] }[] = [] + for (const r of reservations) { + const profile = ROAD_PROFILE[r.type] + if (!profile) continue + const wps = orderedWaypoints(r) + if (wps.length < 2) continue + let dist = 0 + for (let i = 0; i < wps.length - 1; i++) dist += haversineKm(wps[i], wps[i + 1]) + if (dist > MAX_ROUTE_KM) continue + const key = `${profile}:${wps.map(w => `${w.lat},${w.lng}`).join('|')}` + if (attemptedRef.current.get(r.id) === key) continue + attemptedRef.current.set(r.id, key) + jobs.push({ id: r.id, profile, points: wps.map(w => ({ lat: w.lat, lng: w.lng })) }) + } + if (!jobs.length) return + + let cancelled = false + void (async () => { + // Sequential to stay gentle on the shared public router. + for (const job of jobs) { + try { + const result = await calculateRouteWithLegs(job.points, { signal: abortRef.current?.signal, profile: job.profile }) + if (cancelled) return + if (result.coordinates.length >= 2) { + setRoutes(prev => { + const next = new Map(prev) + next.set(job.id, result.coordinates) + return next + }) + } + } catch { + // Leave it unrouted — the overlay keeps the straight line. + } + } + })() + return () => { cancelled = true } + }, [reservations]) + + return routes +} diff --git a/client/tests/integration/hooks/useTransportRoutes.test.ts b/client/tests/integration/hooks/useTransportRoutes.test.ts new file mode 100644 index 000000000..787fcd8ae --- /dev/null +++ b/client/tests/integration/hooks/useTransportRoutes.test.ts @@ -0,0 +1,83 @@ +import { renderHook, waitFor } from '@testing-library/react' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { Reservation } from '../../../src/types' + +const { calculateRouteWithLegs } = vi.hoisted(() => ({ calculateRouteWithLegs: vi.fn() })) +vi.mock('../../../src/components/Map/RouteCalculator', () => ({ calculateRouteWithLegs })) + +import { useTransportRoutes } from '../../../src/hooks/useTransportRoutes' + +function booking(id: number, type: string, from: [number, number], to: [number, number]): Reservation { + return { + id, + type, + status: 'confirmed', + endpoints: [ + { role: 'from', sequence: 0, name: 'A', code: null, lat: from[0], lng: from[1], timezone: null, local_time: null, local_date: null }, + { role: 'to', sequence: 1, name: 'B', code: null, lat: to[0], lng: to[1], timezone: null, local_time: null, local_date: null }, + ], + } as unknown as Reservation +} + +const PARIS: [number, number] = [48.8566, 2.3522] +const VERSAILLES: [number, number] = [48.8049, 2.1204] + +beforeEach(() => { + calculateRouteWithLegs.mockReset() + calculateRouteWithLegs.mockResolvedValue({ coordinates: [PARIS, [48.83, 2.24], VERSAILLES], distance: 20000, duration: 1500, legs: [] }) +}) + +describe('useTransportRoutes (#1425 real road routes)', () => { + it('routes a car booking with the driving profile and returns its geometry', async () => { + const res = [booking(1, 'car', PARIS, VERSAILLES)] + const { result } = renderHook(() => useTransportRoutes(res)) + await waitFor(() => expect(result.current.get(1)).toBeTruthy()) + expect(result.current.get(1)).toHaveLength(3) + expect(calculateRouteWithLegs).toHaveBeenCalledWith(expect.any(Array), expect.objectContaining({ profile: 'driving' })) + }) + + it('routes a bicycle booking with the cycling profile', async () => { + const res = [booking(2, 'bicycle', PARIS, VERSAILLES)] + renderHook(() => useTransportRoutes(res)) + await waitFor(() => expect(calculateRouteWithLegs).toHaveBeenCalled()) + expect(calculateRouteWithLegs).toHaveBeenCalledWith(expect.any(Array), expect.objectContaining({ profile: 'cycling' })) + }) + + it('does not route non-road types (flight, train, transit)', async () => { + const res = [ + booking(3, 'flight', PARIS, VERSAILLES), + booking(4, 'train', PARIS, VERSAILLES), + booking(5, 'transit', PARIS, VERSAILLES), + ] + renderHook(() => useTransportRoutes(res)) + await new Promise(r => setTimeout(r, 20)) + expect(calculateRouteWithLegs).not.toHaveBeenCalled() + }) + + it('skips routing beyond the sanity distance cap (keeps the straight line)', async () => { + // Paris → Tokyo as a "car" booking: ~9700 km, well past the 2000 km cap. + const res = [booking(6, 'car', PARIS, [35.68, 139.69])] + renderHook(() => useTransportRoutes(res)) + await new Promise(r => setTimeout(r, 20)) + expect(calculateRouteWithLegs).not.toHaveBeenCalled() + }) + + it('falls back silently (no entry) when routing throws', async () => { + calculateRouteWithLegs.mockRejectedValueOnce(new Error('OSRM down')) + const res = [booking(7, 'taxi', PARIS, VERSAILLES)] + const { result } = renderHook(() => useTransportRoutes(res)) + await waitFor(() => expect(calculateRouteWithLegs).toHaveBeenCalled()) + await new Promise(r => setTimeout(r, 10)) + expect(result.current.get(7)).toBeUndefined() + }) + + it('does not re-request an unchanged booking when the array identity changes', async () => { + const res1 = [booking(8, 'car', PARIS, VERSAILLES)] + const { rerender } = renderHook(({ r }) => useTransportRoutes(r), { initialProps: { r: res1 } }) + await waitFor(() => expect(calculateRouteWithLegs).toHaveBeenCalledTimes(1)) + // New array, same booking coordinates → no second fetch. + rerender({ r: [booking(8, 'car', PARIS, VERSAILLES)] }) + await new Promise(r => setTimeout(r, 20)) + expect(calculateRouteWithLegs).toHaveBeenCalledTimes(1) + }) +})