diff --git a/AGENTS.md b/AGENTS.md index fe8cf74..271f0b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -199,6 +199,27 @@ session with **no other memory of this project**. So: **< 2 min old**; the blocking "Finding your location…" overlay shows only when there is no seed (with a seed it refines silently via a corner chip). The map "Near Me" button also flips the sort to `proximity`. + - ⚠️ **`RouteDialog` had a total-failure bug (fixed 2026-09-27), don't regress:** + (1) its Leaflet map is inside Radix Dialog content, which mounts AFTER the + component's first effect pass, so a `mapDivRef.current` guard was null, + the init effect bailed, and **the map was never created** (no route line + ever appeared). The node is now held in **state** (`const [mapDiv,setMapDiv]`) + with a callback ref, and `mapDiv` is in the effect deps so it runs when the + node actually mounts. + (2) Even once created, the global `.leaflet-container { height: 100% }` + override in `index.css` **beats `h-64`** on the map element; against a + height-less parent it resolved to 0 (invisible map). Height now lives on the + wrapper (`
`) and the map is `h-full`. + (3) On the map, Directions used to render the dialog INSIDE the spot `Sheet` + (two stacked Radix modals; the sheet covered the route). The map now owns + the dialog at page level (`routeTarget` state) and closes the sheet first. + - OSRM is called **directly** (`router.project-osrm.org` sends + `Access-Control-Allow-Origin: *`) with the `proxy.shakespeare.diy` CORS proxy + only as a fallback — faster, and origin/destination never leave OSRM unless the + direct call is blocked. Failures are distinguished: `Routing service busy` + (429/TooManyRequests), `No route found` (NoRoute / invalid coords), else + unreachable. Verify route rendering with `scripts/`-style CDP repros, not just + the smoke test (which only checks mount). - **`Layout` is a router layout** — `AppRouter` wraps every route (except `/amber-callback` and the `*` 404) in **one** `}>` with an ``, so the chrome (header, nav, notifications, Sasquatch) diff --git a/src/components/RouteDialog.tsx b/src/components/RouteDialog.tsx index f2a0bfd..b47fe1c 100644 --- a/src/components/RouteDialog.tsx +++ b/src/components/RouteDialog.tsx @@ -20,6 +20,8 @@ import { formatDuration, TRAVEL_MODES, TRANSIT_ATTRIBUTION, + ROUTE_ERR_BUSY, + ROUTE_ERR_NO_ROUTE, type TravelMode, type RouteResult, } from '@/lib/routing'; @@ -89,7 +91,12 @@ export function RouteDialog({ destination, destinationLabel = 'Destination', }: RouteDialogProps) { - const mapDivRef = useRef(null); + // The map container is inside Radix Dialog content, which mounts AFTER this + // component's first effect pass. A plain useRef is therefore still null when + // the init effect runs, and — because the effect's deps never changed again — + // the map was NEVER created (empty dialog, no route line). Holding the node in + // state makes the effect re-run the moment it actually mounts. + const [mapDiv, setMapDiv] = useState(null); const mapRef = useRef(null); const routeLayersRef = useRef(null); const originMarkerRef = useRef(null); @@ -149,9 +156,9 @@ export function RouteDialog({ // ─── Initialize the Leaflet map once the dialog content is mounted ───────── useEffect(() => { - if (!open || !mapDivRef.current || mapRef.current || !destination) return; + if (!open || !mapDiv || mapRef.current || !destination) return; - const map = L.map(mapDivRef.current, { + const map = L.map(mapDiv, { center: [destination.lat, destination.lng], zoom: 14, scrollWheelZoom: true, @@ -188,7 +195,7 @@ export function RouteDialog({ originMarkerRef.current = null; destMarkerRef.current = null; }; - }, [open, destination]); + }, [open, mapDiv, destination]); // ─── Draw / update the origin marker when we have the user's location ────── useEffect(() => { @@ -268,15 +275,20 @@ export function RouteDialog({ } catch (err) { if (cancelled) return; if (err instanceof DOMException && err.name === 'AbortError') return; - setError( - mode === 'transit' - ? err instanceof Error && err.message === 'No transit route found' + const msg = err instanceof Error ? err.message : ''; + if (mode === 'transit') { + setError( + msg === 'No transit route found' ? 'No public-transport route found here — this area may not publish open timetables.' - : "Couldn't reach the transit service. Check your connection and try again." - : err instanceof Error && err.message === 'No route found' - ? 'No route found for this mode. Try another mode.' - : "Couldn't reach the routing service. Check your connection and try again.", - ); + : "Couldn't reach the transit service. Check your connection and try again.", + ); + } else if (msg === ROUTE_ERR_BUSY) { + setError('The routing service is busy right now — try again in a moment.'); + } else if (msg === ROUTE_ERR_NO_ROUTE) { + setError('No route for this mode — the destination may be off the road network. Try another mode.'); + } else { + setError("Couldn't reach the routing service. Check your connection and try again."); + } } finally { if (!cancelled) setRouting(false); } @@ -324,11 +336,14 @@ export function RouteDialog({ ))}
- {/* Map */} -
+ {/* Map. The height lives on the WRAPPER: the global + `.leaflet-container { height: 100% }` override beats a `h-64` on the + map element itself, and against a height-less parent that resolved to + 0 — a fully invisible map with no route line. */} +
diff --git a/src/lib/routing.ts b/src/lib/routing.ts index 34bc9ca..b27c461 100644 --- a/src/lib/routing.ts +++ b/src/lib/routing.ts @@ -98,9 +98,44 @@ interface OsrmResponse { }>; } +/** Error message constants the UI branches on (kept in sync with RouteDialog). */ +export const ROUTE_ERR_NO_ROUTE = 'No route found'; +export const ROUTE_ERR_BUSY = 'Routing service busy'; + +function isPoint(p: { lat?: number; lng?: number } | null | undefined): boolean { + return !!p && Number.isFinite(p.lat) && Number.isFinite(p.lng); +} + +/** + * Call OSRM and parse the JSON. + * + * OSRM sends `Access-Control-Allow-Origin: *`, so we call it DIRECTLY first — + * one less third-party hop (faster, and the origin/destination never pass + * through a third party). Only if the direct call fails do we retry through the + * CORS proxy (some networks/WebViews block the direct request). + */ +async function fetchOsrm(url: string, signal?: AbortSignal): Promise { + try { + const res = await fetch(url, { signal }); + if (res.ok) return (await res.json()) as OsrmResponse; + if (res.status === 429) throw new Error(ROUTE_ERR_BUSY); + // Any other non-OK (400/404/500…) is worth one proxy retry. + } catch (e) { + if ((e as Error)?.name === 'AbortError') throw e; + if ((e as Error)?.message === ROUTE_ERR_BUSY) throw e; + // fall through to the proxy + } + + const proxied = `${CORS_PROXY}${encodeURIComponent(url)}`; + const res = await fetch(proxied, { signal }); + if (res.status === 429) throw new Error(ROUTE_ERR_BUSY); + if (!res.ok) throw new Error(`Routing service returned ${res.status}`); + return (await res.json()) as OsrmResponse; +} + /** * Fetch a route between two points for a given travel mode using OSRM. - * Throws on network error or when OSRM returns no route. + * Throws `ROUTE_ERR_BUSY`, `ROUTE_ERR_NO_ROUTE`, or a network error. */ export async function fetchRoute( from: { lat: number; lng: number }, @@ -110,19 +145,16 @@ export async function fetchRoute( ): Promise { const { profile } = travelModeMeta(mode); if (!profile) throw new Error('Not a road routing mode'); // transit uses fetchTransitRoute + // Never send NaN/undefined to OSRM — that surfaces as a confusing "no route". + if (!isPoint(from) || !isPoint(to)) throw new Error(ROUTE_ERR_NO_ROUTE); // OSRM wants lon,lat;lon,lat const coords = `${from.lng},${from.lat};${to.lng},${to.lat}`; const target = `${OSRM_BASE}/${profile}/${coords}?overview=full&geometries=geojson`; - const url = `${CORS_PROXY}${encodeURIComponent(target)}`; - const res = await fetch(url, { signal }); - if (!res.ok) { - throw new Error(`Routing service returned ${res.status}`); - } - - const data = (await res.json()) as OsrmResponse; + const data = await fetchOsrm(target, signal); + if (data.code === 'TooManyRequests') throw new Error(ROUTE_ERR_BUSY); if (data.code !== 'Ok' || !data.routes || data.routes.length === 0) { - throw new Error('No route found'); + throw new Error(ROUTE_ERR_NO_ROUTE); } const route = data.routes[0]; diff --git a/src/pages/MapPage.tsx b/src/pages/MapPage.tsx index a24fb76..05d52fe 100644 --- a/src/pages/MapPage.tsx +++ b/src/pages/MapPage.tsx @@ -347,7 +347,23 @@ function StashDrawer() { // In-app directions — opens the same RouteDialog used on the detail pages. // Deliberately NO external map handoff: routing stays inside SkateSpots. -function DirectionsButton({ lat, lon, label }: { lat: string; lon: string; label: string }) { +function DirectionsButton({ + lat, + lon, + label, + onDirections, +}: { + lat: string; + lon: string; + label: string; + /** + * When provided, the PARENT owns the RouteDialog. Rendering a Radix Dialog + * inside the spot's Radix Sheet stacked two modals (the sheet covered the + * route), so the map passes this and renders the dialog itself; the sheet + * then closes and the route gets the whole screen. + */ + onDirections?: (target: { lat: number; lng: number; label: string }) => void; +}) { const [open, setOpen] = useState(false); const destination = useMemo(() => { const la = parseFloat(lat); @@ -360,24 +376,38 @@ function DirectionsButton({ lat, lon, label }: { lat: string; lon: string; label - + {!onDirections && ( + + )} ); } // Bottom sheet shown when a map marker is tapped -function SpotBottomSheet({ spot, onClose }: { spot: SkateSpot; onClose: () => void }) { +function SpotBottomSheet({ + spot, + onClose, + onDirections, +}: { + spot: SkateSpot; + onClose: () => void; + onDirections: (target: { lat: number; lng: number; label: string }) => void; +}) { const navigate = useNavigate(); const author = useAuthor(spot.pubkey); const metadata = author.data?.metadata; @@ -480,7 +510,7 @@ function SpotBottomSheet({ spot, onClose }: { spot: SkateSpot; onClose: () => vo const label = spot.title; return (
- +
); })()} @@ -534,6 +564,9 @@ export default function MapPage() { const [searchQuery, setSearchQuery] = useState(''); const [selectedCategories, setSelectedCategories] = useState([]); const [selectedSpot, setSelectedSpot] = useState(null); + // Directions target lives at page level (NOT inside the spot Sheet) so the + // RouteDialog is never nested inside another modal. + const [routeTarget, setRouteTarget] = useState<{ lat: number; lng: number; label: string } | null>(null); const [weatherEnabled, setWeatherEnabled] = useState(false); const [weatherFilter, setWeatherFilter] = useState<'all' | 'good' | 'perfect'>('all'); // Smart WoT default: @@ -1914,9 +1947,27 @@ export default function MapPage() { className="h-auto max-h-[55vh] rounded-t-2xl px-0 pb-0 bg-black/95 border-t border-white/10" > {selectedSpot?.title ?? 'Spot details'} - {selectedSpot && setSelectedSpot(null)} />} + {selectedSpot && ( + setSelectedSpot(null)} + onDirections={(target) => { + // Close the sheet first — the route dialog takes over. + setSelectedSpot(null); + setRouteTarget(target); + }} + /> + )} + + {/* In-app route — page-level so it never stacks inside the spot Sheet. */} + { if (!open) setRouteTarget(null); }} + destination={routeTarget ? { lat: routeTarget.lat, lng: routeTarget.lng } : null} + destinationLabel={routeTarget?.label ?? 'Spot'} + /> ); }