feat: implement route discovery, comparison, and map visualization with health insights.#8
Conversation
- Added HomeMap component to display a Mapbox map with geolocation features. - Updated Home page to render the HomeMap component. - Introduced PrivateLayout component for consistent layout structure. - Updated TypeScript configuration to include additional type definitions. - Added Mapbox GL and related dependencies to package.json and package-lock.json. - Created global.d.ts for Mapbox CSS module declaration.
…n points and define route data schema.
…nation selection with search and geolocation.
…th health insights.
📝 WalkthroughWalkthroughAdds a new RoutePage and associated UI/components to discover and compare up to three Mapbox route alternatives (walking/driving/cycling). Reads Changes
Sequence DiagramsequenceDiagram
participant User
participant HomeMap
participant Router
participant RoutePage
participant MapboxGeoAPI as Mapbox Geocoding API
participant MapboxDirAPI as Mapbox Directions API
participant RouteMap as RouteMapBackground
User->>HomeMap: select source & destination
User->>HomeMap: click "Find Cleanest Route"
HomeMap->>HomeMap: validate coords (non-null, distinct)
HomeMap->>Router: navigate /home/routes?from=lng,lat&to=lng,lat
Router->>RoutePage: load with query params
RoutePage->>RoutePage: parse `from`,`to`
RoutePage->>MapboxGeoAPI: reverse-geocode source coords
MapboxGeoAPI-->>RoutePage: return source address
RoutePage->>MapboxGeoAPI: reverse-geocode dest coords
MapboxGeoAPI-->>RoutePage: return dest address
RoutePage->>RoutePage: set mode / state
RoutePage->>MapboxDirAPI: fetch up to 3 route alternatives (mode)
MapboxDirAPI-->>RoutePage: return routes (distance,duration,geometry)
RoutePage->>RouteMap: provide routes, coords, selectedRouteIndex
RouteMap->>RouteMap: init/update map, sources, layers, markers, fit bounds
User->>RoutePage: change mode or select route
RoutePage->>MapboxDirAPI: fetch new alternatives (if needed)
MapboxDirAPI-->>RoutePage: return new routes
RouteMap->>RouteMap: update visualization/highlight
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ith proper routing
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@client/app/`(private)/home/routes/(from)/(to)/page.tsx:
- Line 39: The code falls back to an empty MAPBOX_TOKEN which causes silent auth
failures; update the initialization and/or the component that uses MAPBOX_TOKEN
to detect an empty value and surface a clear dev-time warning (e.g.,
console.warn or a visible dev banner) and optionally throw or render an error in
non-production to prevent silent API calls; locate the MAPBOX_TOKEN constant and
the component/function that invokes Mapbox APIs and add a runtime check that
logs the missing token with context (include MAPBOX_TOKEN in the message) and
aborts Mapbox calls when empty.
- Around line 78-93: Update reverseGeocode and fetchRoutes to explicitly check
response.ok before calling response.json(); if !response.ok, read the body (or
response.status/text), throw or return a clear error containing response.status
and any error text so callers don't silently proceed on 4xx/5xx
responses—specifically modify the async function reverseGeocode(lng, lat) and
the fetchRoutes(...) implementation to validate response.ok, log/throw a
descriptive error including response.status and response.statusText (and JSON
error body when available), and ensure the catch paths return the fallback
coordinate string only for network/runtime errors, not for HTTP error responses.
- Around line 96-136: In fetchRoutes, add a guard to check response.ok before
calling response.json so HTTP errors don't get parsed as success: after awaiting
fetch(url) inspect the response object (response.ok and response.status) and if
not ok, log the response status and text, setError with a descriptive message
and setRoutes([]), then return early; only call await response.json() and
proceed to use data when response.ok is true—update the error handling in the
fetchRoutes try/catch to include non-2xx responses similarly (use the existing
setError, setRoutes, and console.error calls).
In `@client/components/home/HomeMap.tsx`:
- Around line 123-145: handleFindRoute currently logs validation failures to the
console (using console.log) so users get no feedback; replace these console.log
calls inside handleFindRoute with an inline error state (e.g., add a React state
variable routeError via useState<string | null>) and setRouteError(...) for the
three validation cases (missing sourceLocation, missing destLocation, identical
coordinates), render routeError next to the find-route button similar to how
geoError is rendered, and clear routeError before calling
router.push(`/home/routes?from=${from}&to=${to}`); alternatively you may use the
existing toast system instead of inline state if preferred.
In `@client/components/routes/InsightToast.tsx`:
- Around line 5-22: InsightToast currently hardcodes the PM2.5 reduction and has
no dismissal behavior; update the InsightToast component to accept props (e.g.,
pm25Reduction: number and optional onClose: () => void or a controlled visible:
boolean) and use those props to render the percentage instead of the hardcoded
"30%"; add local state (useState) for visibility if you want an uncontrolled
dismissible toast, wire the close button's onClick to call onClose and/or set
the visibility state, and conditionally render/return null when not visible;
reference the InsightToast component and the close <button> that renders <X />
to locate where to add the onClick and where to replace the hardcoded text with
the pm25Reduction prop.
In `@client/components/routes/MapControls.tsx`:
- Around line 8-16: The three icon-only buttons in MapControls.tsx (the button
wrapping the Plus, the button wrapping the Minus, and the button wrapping the
LocateFixed icon) are missing accessible labels; add descriptive aria-label
attributes to each button (e.g., aria-label="Zoom in" for the Plus button,
aria-label="Zoom out" for the Minus button, aria-label="Locate me" for the
LocateFixed button) so screen readers can announce their purpose, and ensure any
existing onClick handlers or accessibility roles remain unchanged.
- Around line 5-19: MapControls currently renders three buttons with no click
handlers; update the MapControls component to accept callback props (e.g.,
onZoomIn, onZoomOut, onLocate) and wire those props to the buttons' onClick
handlers (reference the MapControls function and the three buttons rendering
Plus, Minus, and LocateFixed). Also add prop types or an interface for the
callbacks and default to no-op functions if needed; if you intend them as
placeholders instead, add aria-disabled and a visual disabled style to the
buttons and document that they are non-functional.
In `@client/components/routes/RouteComparisonPanel.tsx`:
- Around line 118-126: The AQI, pollution reduction and exposure warning values
in RouteComparisonPanel are hardcoded (the JSX span values 92/74/42, the "-34%
avg." string, and the "High PM2.5 Exposure Zone" text) and must be replaced with
real data or clearly marked as placeholders; update the RouteData type (or the
props passed into RouteComparisonPanel) to include fields like aqiScore,
pollutionReductionPct, and exposureWarning, then render those values (e.g., use
route.aqiScore instead of the static ternary, route.pollutionReductionPct
instead of "-34% avg.", and route.exposureWarning instead of the fixed string),
or alternatively change the UI text to indicate demo/placeholder values (e.g.,
append "(demo)" or "Placeholder" to each displayed value) so users aren’t
misled; modify the component rendering logic in RouteComparisonPanel to read
these new fields (or add the placeholder labels) and ensure any consuming code
that constructs RouteData supplies the real API values or explicit placeholders.
- Around line 99-107: The title assignment in RouteComparisonPanel currently
maps labels by array index (the JSX block using {index === 0 ? "Cleanest Path" :
index === 1 ? "Balanced" : "Fastest"}), which breaks when the API returns a
different order or a different number of routes; update the component to derive
labels from each route's metrics (e.g., emissions, duration, score) instead of
index: compute/compare route properties (for example lowest emissions →
"Cleanest Path", lowest duration → "Fastest", or a computed composite score →
"Balanced") before rendering and then use that computed label when rendering
each route card so it works for any number/order of routes.
In `@client/components/routes/RouteDiscoveryPanel.tsx`:
- Line 14: The onFindRoutes prop on RouteDiscoveryPanel is declared but not
used; either remove it from the component props interface or wire it to the UI
action (e.g., the "Change Route" button). Locate the RouteDiscoveryPanel
component and its props interface (the onFindRoutes definition) and either (A)
remove onFindRoutes from the interface and all places that pass it, or (B)
attach the handler to the Change Route button's onClick (or the appropriate
control) so the button calls props.onFindRoutes() (ensure proper null/undefined
guarding). Make the change consistently where the prop is referenced so the lint
warning is resolved.
- Around line 128-135: The "Avoid Busy Roads" control in RouteDiscoveryPanel is
currently a static visual; add interactive state and a callback: introduce a
boolean state (e.g., avoidBusyRoads via useState) in the RouteDiscoveryPanel
component, replace the static toggle div with a clickable element (button/div)
that calls a handler (e.g., toggleAvoidBusyRoads) to flip state and invoke a
prop callback (e.g., onAvoidBusyRoadsChange) when changed, update the toggle's
classes/position to reflect state (background color and knob alignment), and add
accessibility attributes (role="switch" or aria-pressed) so user interactions
actually toggle the feature.
In `@client/components/routes/RouteMapBackground.tsx`:
- Around line 56-83: The code currently creates exactly three route
sources/layers (ids like `route-${i}`) which can drop extra alternatives or
leave empty layers; update the logic in RouteMapBackground.tsx to base the loop
on the actual routes array length (e.g., routes.length) and create a
source+layer per route using the same `route-${index}` id pattern, and before
creating ensure you remove any stale layers/sources (check
map.getLayer/map.getSource and call map.removeLayer/map.removeSource) so fewer
returned routes don't leave empty artifacts; also ensure you handle adding only
when the source/layer doesn't already exist to avoid dup errors.
- Around line 130-154: The useEffect that updates route layer styles (watching
selectedRouteIndex, using mapRef and map.isStyleLoaded()) can drop updates if
the style isn't ready; change it to wait for style load like the other effect:
if map.isStyleLoaded() is false, register a one-time styledata/style.load
listener on map that applies the line-color/line-width/line-opacity updates for
layers `route-0`..`route-2` when the style becomes ready, and remove the
listener after firing; otherwise apply the paint property changes immediately.
Ensure the logic lives inside the same useEffect that depends on
selectedRouteIndex and references mapRef.current and selectedRouteIndex.
- Around line 92-127: The effect currently bails when map.isStyleLoaded() is
false, so if routes arrive before the Mapbox style finishes loading they are
never applied; fix by extracting the route update + fitBounds logic into a
function (e.g., updateRoutes) and in the useEffect call it immediately if
map.isStyleLoaded() is true, otherwise register a one-time or persistent map
event listener (map.on('load') or map.on('styledata') / map.once('load')) to
call updateRoutes once the style is ready, and remove the listener in the
cleanup; reference mapRef, routes, and the current useEffect to locate where to
implement the updateRoutes function and event listener registration/cleanup so
route data is applied after style load.
- Around line 156-174: The useEffect in RouteMapBackground removes all elements
with the ".mapboxgl-marker" selector which can delete markers from other
components; instead, track marker instances created here by storing them in refs
(e.g., a markersRef or sourceMarkerRef/destinationMarkerRef) and on updates
remove only those specific Marker objects using their .remove() method before
creating new mapboxgl.Marker instances for source and destination; update the
effect to read/write mapRef, source, destination and clean up markers in the ref
on unmount or when replacing them.
🧹 Nitpick comments (8)
client/app/layout.tsx (1)
6-6: Commented-out Toaster code left in — either restore or remove.The
sonnerpackage was added as a dependency (inpackage.json), yet theToasterimport and usage are commented out. If the Toaster is not needed for this PR, consider removing both the commented-out lines and thesonnerdependency to avoid shipping dead code/unused dependencies. If it's needed for a follow-up, a TODO comment explaining the intent would help.Also applies to: 54-54
client/components/home/HomeMap.tsx (1)
141-144: Encode query parameter values to prevent malformed URLs.While commas in
lng,latare technically URL-safe, it's a best practice to encode query parameter values, especially since coordinates can include negative numbers (the-is fine but future format changes could introduce unsafe characters).Proposed fix
const from = `${sourceLocation.lng},${sourceLocation.lat}`; const to = `${destLocation.lng},${destLocation.lat}`; - router.push(`/home/routes?from=${from}&to=${to}`); + router.push(`/home/routes?from=${encodeURIComponent(from)}&to=${encodeURIComponent(to)}`);client/components/routes/RouteDiscoveryPanel.tsx (1)
7-7:TravelModetype is duplicated acrossRouteDiscoveryPanel.tsxandRouteComparisonPanel.tsx.Extract shared types (
TravelMode,RouteData) into a common module (e.g.,types/routes.ts) to keep a single source of truth and avoid drift.client/components/routes/RouteMapBackground.tsx (1)
97-98: Variablesourceshadows the component prop of the same name.
const source = map.getSource(...)shadows thesourceprop (the origin coordinates). While it doesn't cause a bug here since the prop isn't used in this effect body, it hurts readability and could cause confusion during maintenance.Proposed fix
- const source = map.getSource(`route-${index}`) as mapboxgl.GeoJSONSource; - if (source) { - source.setData({ + const geoSource = map.getSource(`route-${index}`) as mapboxgl.GeoJSONSource; + if (geoSource) { + geoSource.setData({client/package.json (1)
21-21: Defernext-themesandsonnerdependencies until they are actually integrated.
next-themesandsonnerare added topackage.jsonbut not imported or used anywhere in the codebase.sonner'sToastercomponent is commented out inlayout.tsx, andnext-themeshas no setup. Remove these from the current PR and add them back when you're ready to wire them up, keeping the dependency footprint minimal.client/app/(private)/home/routes/(from)/(to)/page.tsx (3)
13-37:RouteDataandMapboxRouteare structurally identical — consolidate into one type.These two types have the exact same shape. Use a single type (e.g.,
RouteData) and dropMapboxRoute. Additionally,TravelModeappears to be redefined inRouteDiscoveryPanel.tsxandRouteComparisonPanel.tsxas well — consider extracting shared types into a commontypes/routes.tsfile to keep definitions in sync.♻️ Suggested consolidation
-type RouteData = { - distance: number; // in meters - duration: number; // in seconds - geometry: { - coordinates: [number, number][]; - type: string; - }; -}; - -type TravelMode = "walking" | "driving" | "cycling"; - -type MapboxRoute = { - distance: number; - duration: number; - geometry: { - coordinates: [number, number][]; - type: string; - }; -}; +// Consider moving these to a shared types file (e.g., `@/types/routes.ts`) +type Coordinates = { + lng: number; + lat: number; +}; + +type RouteData = { + distance: number; // in meters + duration: number; // in seconds + geometry: { + coordinates: [number, number][]; + type: string; + }; +}; + +type TravelMode = "walking" | "driving" | "cycling";Then on line 117, replace
MapboxRoutewithRouteData:- .map((route: MapboxRoute) => ({ + .map((route: RouteData) => ({
54-76:reverseGeocodeis missing from the effect's dependency array and fire-and-forget promises lack cleanup.
reverseGeocodeis defined in the component body and called inside this effect but not listed indeps— this will trigger anreact-hooks/exhaustive-depslint warning. More importantly, ifsearchParamschanges rapidly or the component unmounts, the in-flight geocode fetches will still callsetSourceAddress/setDestAddresson stale or unmounted state.Consider either:
- Moving
reverseGeocodeinside the effect (simplest), or- Wrapping it in
useCallbackand adding it to deps, with anAbortControllerfor cleanup.♻️ Option 1: Move into the effect
useEffect(() => { + const reverseGeocode = async (lng: number, lat: number): Promise<string> => { + try { + const response = await fetch( + `https://api.mapbox.com/geocoding/v5/mapbox.places/${lng},${lat}.json?access_token=${MAPBOX_TOKEN}` + ); + const data = await response.json(); + if (data.features && data.features.length > 0) { + return data.features[0].place_name; + } + return `${lat.toFixed(4)}, ${lng.toFixed(4)}`; + } catch { + return `${lat.toFixed(4)}, ${lng.toFixed(4)}`; + } + }; + const fromParam = searchParams.get("from"); const toParam = searchParams.get("to"); // ... rest of effect }, [searchParams]);
146-154: Consider wrapping handlers inuseCallbackfor stable references.
handleModeChangeandhandleRouteSelectare recreated on every render. If child components useReact.memo, these unstable references will defeat memoization. Wrapping them inuseCallbackis a low-effort improvement.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@client/app/`(private)/home/routes/(from)/(to)/page.tsx:
- Around line 179-180: InsightToast is rendered unconditionally and shows static
health data; change the JSX so InsightToast is only rendered when route data is
available and successful (e.g. when your route-fetch state indicates not
loading, no error, and routes.length > 0). Locate the parent component around
InsightToast/MapControls and wrap InsightToast with a guard that checks the
fetch state variables used in this file (for example isLoading, error, and
routes or routeSegments) so the toast is suppressed during loading, error, or
empty-route states.
🧹 Nitpick comments (1)
client/app/(private)/home/routes/(from)/(to)/page.tsx (1)
54-76:reverseGeocodeis called inside the effect but missing from its dependency array.This violates the
react-hooks/exhaustive-depsrule. Currently safe because the function only closes over the module-levelMAPBOX_TOKEN, but it's fragile—any future change that introduces component state intoreverseGeocodewill silently stale-close.Wrap it in
useCallback(with[]deps since it only uses a module constant) or extract it as a standalone function outside the component.♻️ Suggested fix: extract outside component
+// Move outside RouteContent — no component state needed +const reverseGeocode = async (lng: number, lat: number): Promise<string> => { + try { + const response = await fetch( + `https://api.mapbox.com/geocoding/v5/mapbox.places/${lng},${lat}.json?access_token=${MAPBOX_TOKEN}` + ); + if (!response.ok) { + throw new Error(`Geocoding request failed: ${response.status}`); + } + const data = await response.json(); + if (data.features && data.features.length > 0) { + return data.features[0].place_name; + } + return `${lat.toFixed(4)}, ${lng.toFixed(4)}`; + } catch (error) { + console.error("Geocoding failed:", error); + return `${lat.toFixed(4)}, ${lng.toFixed(4)}`; + } +}; + const RouteContent = () => { // ... - // Reverse geocode to get address from coordinates - const reverseGeocode = async (lng: number, lat: number): Promise<string> => { - ... - };Also applies to: 78-93
| const fetchRoutes = useCallback( | ||
| async (mode: TravelMode) => { | ||
| if (!source || !destination) return; | ||
|
|
||
| setIsLoading(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| const profile = `mapbox/${mode}`; | ||
| const coordinates = `${source.lng},${source.lat};${destination.lng},${destination.lat}`; | ||
|
|
||
| // Request multiple alternative routes | ||
| const url = `https://api.mapbox.com/directions/v5/${profile}/${coordinates}?alternatives=true&geometries=geojson&overview=full&steps=true&access_token=${MAPBOX_TOKEN}`; | ||
|
|
||
| const response = await fetch(url); | ||
| const data = await response.json(); | ||
|
|
||
| if (data.code === "Ok" && data.routes && data.routes.length > 0) { | ||
| // Take up to 3 routes | ||
| const fetchedRoutes = data.routes | ||
| .slice(0, 3) | ||
| .map((route: MapboxRoute) => ({ | ||
| distance: route.distance, | ||
| duration: route.duration, | ||
| geometry: route.geometry, | ||
| })); | ||
| setRoutes(fetchedRoutes); | ||
| } else { | ||
| setError("No routes found. Please try different locations."); | ||
| setRoutes([]); | ||
| } | ||
| } catch (err) { | ||
| console.error("Error fetching routes:", err); | ||
| setError("Failed to fetch routes. Please try again."); | ||
| setRoutes([]); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, | ||
| [source, destination] | ||
| ); |
There was a problem hiding this comment.
Stale route layers on the map when fewer routes are returned.
fetchRoutes may return 1–2 routes instead of 3 (Mapbox alternatives=true doesn't guarantee 3). However, RouteMapBackground initializes 3 map sources/layers and only updates sources matching routes.forEach((route, index) => ...). Previously drawn route data on layers beyond the new array length (e.g., route-2 when only 2 routes are returned) will remain visible on the map.
Consider clearing all route sources before applying new data, or reset unused sources to empty geometry in RouteMapBackground when routes.length < 3.
…n insights and map integration.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@client/app/`(private)/home/routes/(from)/(to)/page.tsx:
- Line 266: MapControls is rendered without its required callbacks so its
buttons are no-ops; wire the component's props (onZoomIn, onZoomOut, onLocate)
to the map reference operations used in this page (e.g., the shared mapRef or
lifted state controlling the map) by passing functions that call the appropriate
methods on mapRef (for example, a handler that reads mapRef.current and calls
the map's zoomIn/zoomOut or setZoom and a locate/fitToUserLocation routine), or
remove MapControls until you provide those handlers; update the MapControls JSX
to include onZoomIn={...}, onZoomOut={...}, and onLocate={...} referencing the
existing mapRef handlers (or the page's zoom/locate functions).
- Around line 261-265: The InsightToast is rendered with a fallback default (30)
when routes[selectedRouteIndex]?.pollutionReductionPct is undefined; update the
rendering in page.tsx to avoid showing a misleading value by only rendering
<InsightToast> when the selected route actually has a defined
pollutionReductionPct (e.g., check
routes[selectedRouteIndex]?.pollutionReductionPct != null) or pass an explicit
value (such as 0 or null) to the pm25Reduction prop to prevent the component's
internal default from firing; ensure you reference the selectedRouteIndex and
routes array used in the component so the conditional targets the correct route
object.
In `@client/components/routes/InsightToast.tsx`:
- Around line 34-39: The close <button> in InsightToast.tsx (the element using
onClick={handleClose} and rendering the <X /> icon) lacks an accessible label;
add an aria-label (e.g., aria-label="Close" or aria-label={t('Close')}) to the
button so screen readers can announce its purpose, and ensure it doesn't alter
existing click/hover behavior handled by handleClose.
In `@client/components/routes/RouteComparisonPanel.tsx`:
- Around line 104-110: The "Best for Health" badge is incorrectly hardcoded to
index === 0 causing mismatch with getRouteLabel's "Cleanest Path" logic; compute
the index of the route with the highest aqiScore (e.g., bestHealthIndex) once at
the top of RouteComparisonPanel (using the same criteria as getRouteLabel) and
replace the index === 0 check with index === bestHealthIndex so the badge is
shown on the actual cleanest route.
🧹 Nitpick comments (5)
client/app/(private)/home/routes/(from)/(to)/page.tsx (2)
13-31:RouteData,TravelMode, andCoordinatestypes are duplicated across multiple files.
RouteDatais defined here (Lines 19-29), inRouteComparisonPanel.tsx(Lines 7-17), and a subset inRouteMapBackground.tsx(Lines 13-20).TravelModeis duplicated inRouteDiscoveryPanel.tsxandRouteComparisonPanel.tsx. This will drift over time.Extract shared types into a single module (e.g.,
@/types/routes.ts) and import from there.
64-86:reverseGeocodeis missing from theuseEffectdependency array.
reverseGeocodeis defined inside the component and called within this effect, but is not listed as a dependency. While it currently works because the function only closes over the module-levelMAPBOX_TOKEN, the React exhaustive-deps rule would flag this — and future edits adding state dependencies could introduce stale-closure bugs.Wrap
reverseGeocodeinuseCallback(likefetchRoutes) or move it outside the component.client/components/routes/RouteMapBackground.tsx (2)
58-60: Emptymap.on("load")callback is dead code.This listener does nothing and can be removed.
Proposed fix
mapRef.current = map; - map.on("load", () => { - // Map loaded - }); - return () => {
72-79: Leftover prompt/design notes in comments.Lines 73-83 contain conversational "thinking aloud" comments (e.g., "The prompt asks to…", "Let's assume…"). These read as AI-generated implementation notes rather than documentation. Clean them up to concise comments or remove entirely.
client/components/routes/RouteComparisonPanel.tsx (1)
51-59:getRouteLabelis recomputed on every card render withMath.max(...allRoutes.map(...))spread.For the current max of 3 routes this is trivial, but it's called inside
.map()so it runs N×N. Consider computing labels once before the JSX.
| {!isLoading && !error && routes.length > 0 && ( | ||
| <InsightToast | ||
| pm25Reduction={routes[selectedRouteIndex]?.pollutionReductionPct} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
InsightToast shows a misleading default when pollutionReductionPct is undefined.
When the selected route has no pollutionReductionPct (only route index 0 gets one from the demo data), the prop falls through to InsightToast's default of 30, displaying a fabricated "reduces PM2.5 exposure by 30%" message. Either pass an explicit value or don't render the toast when the data is unavailable.
Proposed fix
- {!isLoading && !error && routes.length > 0 && (
+ {!isLoading && !error && routes.length > 0 && routes[selectedRouteIndex]?.pollutionReductionPct != null && (
<InsightToast
pm25Reduction={routes[selectedRouteIndex]?.pollutionReductionPct}
/>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {!isLoading && !error && routes.length > 0 && ( | |
| <InsightToast | |
| pm25Reduction={routes[selectedRouteIndex]?.pollutionReductionPct} | |
| /> | |
| )} | |
| {!isLoading && !error && routes.length > 0 && routes[selectedRouteIndex]?.pollutionReductionPct != null && ( | |
| <InsightToast | |
| pm25Reduction={routes[selectedRouteIndex]?.pollutionReductionPct} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In `@client/app/`(private)/home/routes/(from)/(to)/page.tsx around lines 261 -
265, The InsightToast is rendered with a fallback default (30) when
routes[selectedRouteIndex]?.pollutionReductionPct is undefined; update the
rendering in page.tsx to avoid showing a misleading value by only rendering
<InsightToast> when the selected route actually has a defined
pollutionReductionPct (e.g., check
routes[selectedRouteIndex]?.pollutionReductionPct != null) or pass an explicit
value (such as 0 or null) to the pm25Reduction prop to prevent the component's
internal default from firing; ensure you reference the selectedRouteIndex and
routes array used in the component so the conditional targets the correct route
object.
| pm25Reduction={routes[selectedRouteIndex]?.pollutionReductionPct} | ||
| /> | ||
| )} | ||
| <MapControls /> |
There was a problem hiding this comment.
MapControls rendered without callback props — buttons are non-functional.
MapControls accepts onZoomIn, onZoomOut, and onLocate callbacks, but none are passed here. The zoom/locate buttons will render but do nothing when clicked.
Wire the callbacks to mapRef operations (via a shared ref or lifted state), or remove the component until it's functional.
🤖 Prompt for AI Agents
In `@client/app/`(private)/home/routes/(from)/(to)/page.tsx at line 266,
MapControls is rendered without its required callbacks so its buttons are
no-ops; wire the component's props (onZoomIn, onZoomOut, onLocate) to the map
reference operations used in this page (e.g., the shared mapRef or lifted state
controlling the map) by passing functions that call the appropriate methods on
mapRef (for example, a handler that reads mapRef.current and calls the map's
zoomIn/zoomOut or setZoom and a locate/fitToUserLocation routine), or remove
MapControls until you provide those handlers; update the MapControls JSX to
include onZoomIn={...}, onZoomOut={...}, and onLocate={...} referencing the
existing mapRef handlers (or the page's zoom/locate functions).
| <button | ||
| onClick={handleClose} | ||
| className="text-white/40 transition-colors hover:text-white dark:text-slate-500 dark:hover:text-slate-300" | ||
| > | ||
| <X size={18} /> | ||
| </button> |
There was a problem hiding this comment.
Close button missing aria-label.
The close button only contains an X icon with no accessible label for screen readers.
Proposed fix
<button
onClick={handleClose}
+ aria-label="Dismiss health insight"
className="text-white/40 transition-colors hover:text-white dark:text-slate-500 dark:hover:text-slate-300"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={handleClose} | |
| className="text-white/40 transition-colors hover:text-white dark:text-slate-500 dark:hover:text-slate-300" | |
| > | |
| <X size={18} /> | |
| </button> | |
| <button | |
| onClick={handleClose} | |
| aria-label="Dismiss health insight" | |
| className="text-white/40 transition-colors hover:text-white dark:text-slate-500 dark:hover:text-slate-300" | |
| > | |
| <X size={18} /> | |
| </button> |
🤖 Prompt for AI Agents
In `@client/components/routes/InsightToast.tsx` around lines 34 - 39, The close
<button> in InsightToast.tsx (the element using onClick={handleClose} and
rendering the <X /> icon) lacks an accessible label; add an aria-label (e.g.,
aria-label="Close" or aria-label={t('Close')}) to the button so screen readers
can announce its purpose, and ensure it doesn't alter existing click/hover
behavior handled by handleClose.
| {index === 0 && ( | ||
| <div className="absolute top-0 right-0 p-2"> | ||
| <span className="rounded-full border border-[#2bee6c]/20 bg-[#2bee6c]/10 px-2 py-1 text-[10px] font-bold tracking-tight text-[#2bee6c] uppercase"> | ||
| Best for Health | ||
| </span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
"Best for Health" badge is hardcoded to the first route (index 0), contradicting getRouteLabel logic.
getRouteLabel determines "Cleanest Path" based on the highest aqiScore, but the "Best for Health" ribbon is always pinned to index === 0. If the API returns routes where the first route doesn't have the highest AQI, the badge and label will disagree.
Proposed fix: derive the badge from actual data
- {index === 0 && (
+ {route.aqiScore === Math.max(...routes.map((r) => r.aqiScore || 0)) && (route.aqiScore || 0) > 0 && (
<div className="absolute top-0 right-0 p-2">Or compute bestHealthIndex once above the JSX and compare against it.
🤖 Prompt for AI Agents
In `@client/components/routes/RouteComparisonPanel.tsx` around lines 104 - 110,
The "Best for Health" badge is incorrectly hardcoded to index === 0 causing
mismatch with getRouteLabel's "Cleanest Path" logic; compute the index of the
route with the highest aqiScore (e.g., bestHealthIndex) once at the top of
RouteComparisonPanel (using the same criteria as getRouteLabel) and replace the
index === 0 check with index === bestHealthIndex so the badge is shown on the
actual cleanest route.
2 participants
Summary by CodeRabbit
New Features
UI