Route Menu and Marker Hover Card#176
Conversation
Align the results header with the mock by using a compact app label plus Save/Export actions while preserving the existing sidebar toggle and pending-edit cancel behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Refine the sidebar title section, edit action, and footer copy styling to better match the hifi mock while keeping the route list behavior unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Use route palette colors for map pins and switch marker visuals from plain dots to pin-style markers so map markers align with route styling. Co-authored-by: Cursor <cursoragent@cursor.com>
Match the edit-mode layout with a wider results panel, save-edits CTA styling, and centered map status banner while keeping existing route behavior intact. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Add cyan timeline markers with dotted connectors in expanded routes. Remove duplicate sequence badge from EditableStopItem. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract recipientSummary for tests, hide locked contact row when empty, and use middle-dot formatting on hi-fi address cards. Co-authored-by: Cursor <cursoragent@cursor.com>
Map recipient_phone CSV alias, prefer form delivery quantity for capacityUsed, and add unit tests for recipient fields and capacity fallback. Co-authored-by: Cursor <cursoragent@cursor.com>
Add optional delivery window fields on Stop, format recipient and delivery lines on EditableStopItem, and map form delivery times into routes. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Match hi-fi expanded stop cards with icon rows for recipient and delivery, route-tinted package pill, and rounded card layout. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Add note doc icon row, hi-fi read-only note panel, and teal save control in edit mode for expanded stop cards. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Wire hi-fi results Export button to route JSON downloads, with a warning when edit mode or pending pin moves are active. Rebased on step3-edit-look-3-main-v7; keep existing routeColors palette. Co-authored-by: Cursor <cursoragent@cursor.com>
Rebased on step4-export-popup: per-route export/duplicate/delete menu, marker hover tooltips, and existing routeColors/Map directions unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
6b18478 to
cfc658d
Compare
kirillakovalenko
left a comment
There was a problem hiding this comment.
Code Review — PR #176: Route Menu and Marker Hover Card
Author: Kesar Singh Sidhu | Files: 22 | +1,713 / -266
Overview
This PR adds three user-facing features to the /results page:
Export modal — select and download optimized routes as JSON files, with an edit-mode guard
Per-route ⋯ menu — export, duplicate, and delete individual routes from sidebar cards
Marker hover card — floating tooltip on map pins showing stop address, contact, time, and notes
Supporting changes include a recipientSummary utility (with tests), a hasOptimizeResults sessionStorage check to enable the sidebar Results nav link, deliveryWindowStart/End fields flowing through from the edit form, and a full visual redesign of EditableStopItem and Sidebar.
Bugs
-
Wrong icons for Duplicate and Delete in RouteCardMenu
All three menu items render — including "Duplicate Route" and "Delete Route". The delete button applies text-red-700 styling but still shows a download arrow, which is misleading. Dedicated copy and trash icons are needed.
→ RouteCardMenu.tsx:1382–1403 -
"Save" pin-move button is always visible in the header
The Cancel button is correctly gated on pendingPinMove != null, but the Save button is always rendered:
{pendingPinMove != null && Cancel}
Save // always visible
This "Save" appears even when there's nothing to save and alongside the sidebar's "Save edits"/"Edit" toggle, creating two competing save affordances.
→ page.tsx:2166–2173
-
StopHoverCard ignores deliveryWindowStart/deliveryWindowEnd
EditableStopItem has a formatDeliveryWindow function that prefers the raw window strings when both are set. StopHoverCard uses formatTimeWindow which only reads stop.timeWindow. The two display components will show different time values for the same stop.
→ StopHoverCard.tsx:1896–1903 vs EditableStopItem.tsx:281–286 -
--edit-stone-700 CSS variable may be undefined
ExportRoutesModal references var(--edit-stone-700) for the "Export (N)" button border, but only --edit-teal-400 and --edit-foreground are added to edit.module.css. If this token isn't defined elsewhere in the cascade, the border will be invisible.
→ ExportRoutesModal.tsx:886
Code Quality
5. useIsClient duplicated across two files
ExportEditWarningModal.tsx and ExportRoutesModal.tsx each define an identical useIsClient hook. This should live in a shared utility (e.g., app/edit/hooks/useIsClient.ts).
-
Missing keyboard navigation in RouteCardMenu
The menu handles Escape to close but has no Up/Down arrow key navigation between items. WAI-ARIA menu/menuitem pattern requires arrow key support for screen reader compatibility.
→ RouteCardMenu.tsx:1316–1332 -
recipientSummary has an unreachable fallback
The "—" return at the end of recipientSummary is never reachable because the function is only called after hasRecipientContact() returns true. Harmless, but misleading.
→ recipientSummary.ts:210 -
role="status" misuse on the warning banner
In ExportEditWarningModal, the amber warning box has role="status". This role is for polite live regions that announce updates to screen readers. A static warning inside an already-announced modal dialog should use role="alert" or no role.
→ ExportEditWarningModal.tsx:635
Performance / Architecture
9. useContainerPixelPosition creates a new OverlayView per hover
Each time lat or lng changes (i.e., whenever a different stop is hovered), the useEffect tears down and rebuilds a google.maps.OverlayView. A single persistent overlay that updates its projection on bounds_changed / idle would avoid the extra DOM churn per hover.
→ MapStopHoverOverlay.tsx:1207–1231
- Module-level mutable markerScaledSize
let markerScaledSize: google.maps.Size | undefined is shared mutable state at module level. It works correctly as a lazy singleton but it's unusual for a React module. A simple useMemo inside the component or a getter that doesn't leak module-level state would be cleaner.
→ Map.tsx:920–929
Minor / Nits
11. Dead CSS transition on isSidebarOpen
isSidebarOpen is now hardcoded to true, but the sidebar wrapper retains transition-[width] duration-300 ease-in-out. The transition will never fire. If the toggle is intentionally removed, the transition class can go too.
→ page.tsx:2192
- "Done editing" doesn't auto-open export after clearing edit mode
When a user triggers export while in edit mode, clicks "Done editing", and leaves the warning modal — they must click Export again. This is acknowledged in the PR description, but could be smoothed by auto-opening the export modal after handleDoneEditingForExport.
Validation Gap
The PR checklist leaves typecheck, test, and build unchecked for a ~1,700-line addition. Given the new types (HoveredStopInfo, deliveryWindowStart/End) and the component refactors, a typecheck pass is recommended before merge.
Summary
The feature work is solid — the export flow, route menu, and hover card are all well-structured and the new utilities are properly unit tested. The main items to address before merging are the wrong icons on Duplicate/Delete menu items (#1), the always-visible Save pin button (#2), the time display inconsistency between StopHoverCard and EditableStopItem (#3), and the potentially undefined --edit-stone-700 variable (#4). The duplicate useIsClient hook and missing arrow key navigation are clean-up candidates worth addressing as well.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
/resultsin development when no optimize session exists.Motivation
Changes
Frontend (
app/ui)Export (Step 4)
ExportRoutesModal— select routes, Export (N) / Export All, per-route color accent in list.ExportEditWarningModal— blocks export while editing or when a pin drag is unsaved; Done editing exits edit mode.downloadRouteJson.ts— one.jsonfile per route with staggered browser downloads.page.tsx— Export in header; routes modal or warning depending on state.Per-route menu (Step 5)
RouteCardMenu— ⋯ menu on each route card (larger hit target): Export Route, Duplicate Route, Delete Route.Sidebar.tsx— menu next to expand chevron; does not toggle card expand.page.tsx— single-route export (same edit warning rules), duplicate route (copy below original), delete route.Marker hover (Step 5)
StopHoverCard— compact tooltip: route/stop badge, address, contact, time, optional note.MapStopHoverOverlay— positions card above pin (follows pan/zoom).Map.tsx— hover on Advanced Markers and fallbackMarker; hides while dragging.Dev / supporting
mock_route.json+mockRouteLoader.ts— two sample routes for local/results.routeColors.ts— accent colors for export route list.types.ts—HoveredStopInfo,phoneNumberon stops.edit.module.css—--edit-teal-400,--edit-foregroundfor export UI tokens.Backend
No backend changes.
Validation
Frontend
npm --prefix app/ui run lintnpm --prefix app/ui run format:checknpm --prefix app/ui run typechecknpm --prefix app/ui run testnpm --prefix app/ui run build/results(dev) — mock routes, export modal, route ⋯ menu, marker hover cardBackend
Skipped — frontend-only PR.
Risk
vehicleId/ stop ids; not a re-optimize — copy is for UI/testing until solver integration.mainwith export + Step 5 only; hi-fi Results chrome from older stacked branches is not included until thosemainPRs land.Rollout and Recovery
High-Signal PR Checklist