HI-FI: Routes Panel Header and Footer Text#150
Conversation
|
A few things to sort out before this can merge: Blocker — rebase needed This branch and #149 both target Blocker — unconditional The TODO that was removed from Carried over from #149 (still unresolved)
Minors
The Edit → Done button swap is cleaner UX, and pulling in the shared |
dd7b559 to
e3534be
Compare
markboenigk
left a comment
There was a problem hiding this comment.
Good follow-up — all four blockers from the first round are resolved: rebased onto main, /results link guarded by readHasOptimizeResults(), Export disabled, Save/Cancel conditional. The shared sidebar nav components and Edit → Done button swap are the right calls.
Blocker — Directions cache silently removed
The old Map.tsx had a directionsCache Map (capped at 100 entries) that prevented repeat DirectionsService calls for routes that hadn't changed between renders. This PR removes it with no explanation. Directions API is billed per request separately from map tile loads — without the cache, every effect run calls the API for every route from scratch. The comment added at line 68 addresses key scoping, not request volume, so it doesn't cover this. Either restore the cache or explicitly justify why it's no longer needed.
Should fix — catch {} swallows the error
The fallback behaviour is correct but dropping the console.warn makes quota exhaustion, bad API keys, and network failures invisible in the console. Keep the log.
Carry-over from #149 — Save and Cancel still visually identical
Will resolve itself when this rebases after #149's Save button fix lands.
Nit — routeColors.ts comments restate the code
Both inline comments were flagged in the first round and are still here — drop them.
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>
Show Save/Cancel only when a pin move is pending, disable Export until export modal ships, and restore a screen-reader Results heading. Co-authored-by: Cursor <cursoragent@cursor.com>
Replay the full pre-rebase results stack (route colors, map polylines, top bar, left strip, routes panel) on current upstream/main in one commit to avoid conflicts with directions work already merged upstream. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid setState in useEffect (react-hooks/set-state-in-effect) by reading sessionStorage when rendering SidebarResultsButton. Co-authored-by: Cursor <cursoragent@cursor.com>
Use SIDEBAR_NAV_ITEM_INACTIVE (exported from formStyles.v2) and restore optional phoneNumber on Stop for vroomToRoutes. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
cc600ac to
4831a73
Compare
kirillakovalenko
left a comment
There was a problem hiding this comment.
- Edit/Done button missing state semantics — the button toggles a mode but has no
aria-pressedattribute. Screen readers won't know the current state.
// Suggestion:
<button aria-pressed={isEditMode} ...>
{isEditMode ? "Done" : "Edit"}
</button>
-
Class name concatenation in
Sidebar.tsx— The pattern${SIDEBAR_ROOT} ${className}works but is fragile. If this project already usesclsxor acnutility, prefer that for safety.
Sidebar.tsx:10 -
readHasOptimizeResults()called unconditionally — it's invoked even on the early-returnisResultsPagebranch wherehasStoredRoutesis never used. Tiny, but worth reordering the reads. -
Hardcoded emoji in JSX (❤️) — fine for a branding footer, but worth confirming it's intentional and won't cause issues in PDF export or other output paths.
| export default function SidebarResultsButton() { | ||
| const pathname = usePathname(); | ||
| const isResultsPage = pathname === "/results"; | ||
| const hasStoredRoutes = readHasOptimizeResults(); |
There was a problem hiding this comment.
readHasOptimizeResults is not reactive — it reads sessionStorage at render time with no subscription. If the user runs optimization again in the same session (updating sessionStorage), the Results nav won't update until the component re-renders for another reason. Consider a useState + storage event listener, or rely on a shared React state/context instead.
| "use client"; | ||
|
|
||
| import { useCallback, useEffect, useState } from "react"; | ||
| import styles from "../edit/edit.module.css"; |
There was a problem hiding this comment.
Cross-page CSS module coupling — results/page.tsx imports ../edit/edit.module.css and applies styles.root. This tightly couples two pages' stylesheets. If the intent is just to inherit CSS variables (like --edit-teal-500), consider either moving those variables to a shared CSS file, or applying them via a wrapper class in results/globals.css.
results/page.tsx:6
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
Refines the Results routes panel and edit/results navigation to match the hi-fi mock: panel header (title, route count, Edit/Done), route cards, b² footer, and safe navigation to
/resultsonly when optimize data exists. Stacked on #149 (step2-topbar).Motivation
The routes sidebar still used the old toggle layout, and the edit page could send users to
/resultswith no session data (error modal). Aligning the panel with the mock—and stacking on the top bar PR—fixes that and avoids merge conflicts with #149.Changes
Frontend
app/ui/src/app/results/components/Sidebar.tsx: Optimized Routes title/count in the header row, Edit/Done pill, scroll area + b² footer using--edit-teal-500, route card container styling.app/ui/src/app/results/page.tsx: Shared edit sidebar nav (SidebarEditButton,SidebarResultsButton); inherits HI-FI: Match Results Top Bar #149 top bar (conditional Save/Cancel, disabled Export,sr-onlyResults heading).app/ui/src/app/edit/components/Sidebar/SidebarResultsButton.tsx: Links to/resultsonly whenoptimizeResultshas at least one route; active state on the Results page.app/ui/src/app/edit/utils/hasOptimizeResults.ts: Session guard helper.app/ui/src/app/results/components/Map.tsx,utils/routeColors.ts,EditableStopItem.tsx: Per-route colors and related map/sidebar polish from the rebased stack.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 buildManual: Run optimize from edit → Results loads; on edit, Results nav stays disabled until routes exist; panel Edit/Done and footer match mock; Save/Cancel only after dragging a stop (#149).
Risk
Low–medium UI changes on Results and the edit sidebar. The results link guard prevents empty-session navigation. Keep base as
step2-topbaruntil #149 merges, then retarget tomain.Rollout and Recovery
Ship after #149 merges (or with base
step2-topbar). Revert to restore the old sidebar toggle and disabled/unconditional results link behavior.