skatespots/SkateSpots
1
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions 1

fix(directions): route map never rendered + dialog stacked under the spot sheet

Browse source 
RouteDialog was effectively broken — users got an empty dialog / 'no route'.

- Map init: the Leaflet map lives in Radix Dialog content, which mounts AFTER
  the first effect pass, so the mapDivRef guard was null, the init effect bailed,
  and the map was never created (no route line, ever). Hold the node in state
  with a callback ref and add it to the effect deps.
- Height: the global .leaflet-container{height:100%} override beats h-64; against
  a height-less parent it resolved to 0 (invisible map). Height now lives on the
  wrapper (relative h-64) and the map is h-full.
- Stacking: from the map, Directions rendered the dialog inside the spot Sheet
  (two nested Radix modals; the sheet covered the route). The map now owns the
  RouteDialog at page level and closes the sheet first.
- Routing transport: call OSRM directly (it sends CORS '*') with the cors proxy
  only as fallback; distinguish busy / no-route / unreachable errors and guard
  against NaN coordinates.
This commit is contained in:
SkateSpots 2026-09-27 17:07:44 +07:00
parent e5e2f7d7a1
commit 0b5230d4ab
4 changed files with 155 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
AGENTS.md
View file

@ -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 (`<div className="relative h-64">`) 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** `<Route element={<Layout/>}>`
with an `<Outlet/>`, so the chrome (header, nav, notifications, Sasquatch)

47
src/components/RouteDialog.tsx
View file

@ -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<HTMLDivElement>(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<HTMLDivElement | null>(null);
const mapRef = useRef<L.Map | null>(null);
const routeLayersRef = useRef<L.LayerGroup | null>(null);
const originMarkerRef = useRef<L.Marker | null>(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({
))}
</div>
{/* Map */}
<div className="relative">
{/* 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. */}
<div className="relative h-64">
<div
ref={mapDivRef}
className="h-64 w-full bg-muted"
ref={setMapDiv}
className="h-full w-full bg-muted"
aria-label="Route map"
/>

50
src/lib/routing.ts
View file

@ -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<OsrmResponse> {
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<RouteResult> {
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];

73
src/pages/MapPage.tsx
View file

@ -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
<Button
variant="outline"
className="h-12 w-full font-bold text-sm border-white/20 hover:bg-white/10 rounded-xl"
onClick={() => setOpen(true)}
onClick={() => {
if (!destination) return;
if (onDirections) onDirections({ ...destination, label: label || 'Spot' });
else setOpen(true);
}}
disabled={!destination}
>
<Navigation className="h-4 w-4 mr-2 text-[#ccff00]" />
Directions
</Button>
<RouteDialog
open={open}
onOpenChange={setOpen}
destination={destination}
destinationLabel={label || 'Spot'}
/>
{!onDirections && (
<RouteDialog
open={open}
onOpenChange={setOpen}
destination={destination}
destinationLabel={label || 'Spot'}
/>
)}
</>
);
}
// 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 (
<div className="relative">
<DirectionsButton lat={lat} lon={lon} label={label} />
<DirectionsButton lat={lat} lon={lon} label={label} onDirections={onDirections} />
</div>
);
})()}
@ -534,6 +564,9 @@ export default function MapPage() {
const [searchQuery, setSearchQuery] = useState('');
const [selectedCategories, setSelectedCategories] = useState<string[]>([]);
const [selectedSpot, setSelectedSpot] = useState<SkateSpot | null>(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"
>
<SheetTitle className="sr-only">{selectedSpot?.title ?? 'Spot details'}</SheetTitle>
{selectedSpot && <SpotBottomSheet spot={selectedSpot} onClose={() => setSelectedSpot(null)} />}
{selectedSpot && (
<SpotBottomSheet
spot={selectedSpot}
onClose={() => setSelectedSpot(null)}
onDirections={(target) => {
// Close the sheet first — the route dialog takes over.
setSelectedSpot(null);
setRouteTarget(target);
}}
/>
)}
</SheetContent>
</Sheet>
{/* In-app route — page-level so it never stacks inside the spot Sheet. */}
<RouteDialog
open={!!routeTarget}
onOpenChange={(open) => { if (!open) setRouteTarget(null); }}
destination={routeTarget ? { lat: routeTarget.lat, lng: routeTarget.lng } : null}
destinationLabel={routeTarget?.label ?? 'Spot'}
/>
</>
);
}