HI-FI: Route Colors

[KesarSidhu]

Summary

Per-route colors in the sidebar and road-following polylines on the map. Directions API supplies driving geometry and can update each route’s distanceMi; a coordinate-keyed cache limits repeat Directions calls.

Motivation

Routes were hard to distinguish when all map lines were the same color while the sidebar needed per-route accents. Reviewers also wanted driving paths and distance from Directions instead of straight lines only.

Changes

• routeColors.ts — Five-color palette; routeColorHex(routeIndex) for consistent route colors.
• Sidebar.tsx — Left border, swatch, and route title use routeColorHex(idx).
• Map.tsx — Directions polylines, cache (path + meters), onRouteDistanceUpdate, straight-line fallback when Directions fails or waypoints > 25.
• page.tsx — handleRouteDistanceUpdate updates distanceMi when the map reports a new value.

Validation

• npm --prefix app/ui run lint
• npm --prefix app/ui run build
• Manually verified /results: sidebar colors match map polylines.

Risk

Low — UI-only; existing directions/distance behavior preserved with caching and cleanup on unmount.

Rollout and Recovery

Ship with results page; revert PR if palette or map styling needs adjustment.

[markboenigk]

Hey @KesarSidhu — thanks for the PR! The color palette implementation is clean and the routeColorHex utility is a nice abstraction. A few things before this merges:

PR description doesn't match the diff
The description says "It does not change how routes are drawn with Directions, how distance is calculated" — but Map.tsx is now calling DirectionsService.route() per route and page.tsx gains handleRouteDistanceUpdate which rewrites distanceMi from the API response. Both route drawing and distance calculation are materially changed. Worth correcting so reviewers know what they're actually approving.

Scope
This bundles two independent features: sidebar colors (cosmetic) and road-following polylines + live distance updates (behavioral). Ideally these would be separate PRs so the Directions API change can get focused review. If splitting isn't practical at this point, at minimum the description should reflect both changes.

Directions API billing
DirectionsService.route() is called for every route on every render with no caching or deduplication guard. At scale this could add up — worth looking into caching the Directions result keyed on the route's waypoints (e.g. in a useRef map or a module-level cache). That way re-renders and repeated views of the same route don't trigger redundant API calls.

Minor: comments in routeColors.ts and Map.tsx
The inline comments explain what the code does rather than why, which the project avoids. These three can go:

• Top-of-file // Colors go in list order…
• // routeIndex (0 = first route…) on the function signature
• // uses the list in order… on the return line
• Map.tsx:57 billing comment (address this in the PR description instead)

The good stuff — cancelled flag pattern is correct, Promise.allSettled is the right choice, the 25-waypoint fallback is solid, and the bail-out in handleRouteDistanceUpdate (next.every(…) ? prev : next) is a nice touch.

[markboenigk]

Nice work overall — the cancellation pattern and state-update guards in particular are well thought out. A few things worth discussing before merge:

Map polylines don't pick up the per-route colors

The sidebar uses routeColorHex(idx) for borders, swatches, and text, but ROUTE_POLYLINE_OPTIONS still uses the single static #2563eb for every polyline on the map. A user sees route 1 as red in the sidebar but all lines are the same blue on the map. Is this intentional (follow-up PR)? If so, worth opening a linked issue so it doesn't get lost.

Re-render cascade causes polylines to be recreated twice on load

The flow is: Directions result arrives → onRouteDistanceUpdate → setRoutes(new distanceMi) → routes prop changes → effect re-runs → cache hit → onRouteDistanceUpdate (guard fires, no state change) → stops. Each re-run tears down and recreates all polylines. It terminates correctly and the cache prevents a second API call, but it's an extra render + setMap(null) cycle per route. A useRef flag tracking whether distances have been initialized would short-circuit the second run.

Module-level cache has no TTL

directionsCache survives SPA navigations. If a user runs a new optimization with overlapping stop coordinates (same location, different route assignment), they'll get cached geometry from the previous job. Probably fine for v1, but worth a comment or a future ticket.

Minor: new google.maps.DirectionsService() on every effect run

It's cheap, but moving it to a useRef would be a small cleanup. Also, if per-route colors come to the map later, ROUTE_POLYLINE_OPTIONS will need to be parameterized — a routePolylineOptions(color) helper now costs nothing and makes that change trivial.

[markboenigk]

The latest round addresses all the feedback — per-route colors land on the map now, effectKey cleanly breaks the distance-update cascade, DirectionsService is a useRef, and the remaining comments are appropriate WHY comments. Nice cleanup.

One thing is still missing before this is mergeable: onRouteDistanceUpdate is never actually connected to the app. RoutePolylinesOverlay accepts it as an optional prop and guards every call site correctly, but MapComponent doesn't declare the prop and line 273 never passes it through:

// Map.tsx:273 — callback never passed
<RoutePolylinesOverlay routes={routes} pendingPinMove={pendingPinMove} />

page.tsx also has no handleRouteDistanceUpdate. The Directions distances are computed and then thrown away — distanceMi in app state is never updated. Three things needed to close the loop:

1. Add onRouteDistanceUpdate to MapComponentProps and pass it to RoutePolylinesOverlay at line 273.
2. Add handleRouteDistanceUpdate in page.tsx, wrapped in useCallback, that calls setRoutes(...).
3. Pass it to <MapComponent>.

Related: once that's wired up, onRouteDistanceUpdate needs a stable identity (useCallback) in page.tsx — it's in the effect's dependency array, so a new function reference on every render would cause the polylines to tear down and rebuild on each setRoutes call, partially undoing what effectKey fixed.

Minor: two palette entries used as text color on a white background (#D57303 and #50881F) will likely fail WCAG AA contrast for text-sm. Either darken them for text-only usage or keep the route title text-zinc-800 and apply the accent to border/swatch only (which have no contrast requirement at that size).

[KesarSidhu]

@markboenigk