HI-FI: Left Navigation Strip on Results

[KesarSidhu]

Summary

• Adds the thin left navigation rail on the Results page with Manage and Results icon buttons, matching the edit-page mock for sizing, colors, and active/inactive states.
• Results shows as active on /results; Manage links to /edit. Results is enabled when optimizeResults exists in sessionStorage after optimization.

Motivation

• Results previously had no left nav strip, so the page did not match the hi-fi mock or the edit page layout.
• Users need consistent navigation between Manage and Results without losing context after running optimization.
• Rebased onto upstream/main and wired shared edit sidebar components so nav tokens stay aligned with the edit page.

Changes

Frontend

• app/ui/src/app/results/page.tsx — Replaced inline nav with shared EditSidebar, SidebarEditButton, and SidebarResultsButton; kept hi-fi top bar (Cancel / Save / Export).
• app/ui/src/app/edit/components/layout/sidebar/SidebarResultsButton.tsx — Active state on /results; links to /results when stored routes exist; disabled otherwise.
• app/ui/src/app/edit/utils/hasOptimizeResults.ts — Reads and validates optimizeResults from sessionStorage.
• app/ui/src/app/results/components/Sidebar.tsx — Minor formatting; left-strip route styling preserved from branch work.
• app/ui/src/app/results/utils/routeColors.ts — Minor formatting only.

Validation

Frontend

• npm --prefix app/ui run lint
• npm --prefix app/ui run format:check
• npm --prefix app/ui run typecheck
• npm --prefix app/ui run test
• npm --prefix app/ui run build
• npm --prefix app/mobile run lint
• npm --prefix app/mobile run typecheck

Backend

• cmake --preset dev
• .github/scripts/check-backend-static.sh build/dev
• cmake --build --preset dev --parallel
• ctest --preset dev --output-on-failure --no-tests=error -LE 'e2e|docker'
• docker compose -f deploy/compose/docker-compose.arm64.yml --env-file deploy/env/http-server.arm64.env config

Manual: opened /results with and without optimizeResults in sessionStorage — Manage navigates to /edit, Results is active on Results, disabled on edit when no stored routes.

Risk

• Low. UI-only change to Results navigation; no API or data model changes.
• SidebarResultsButton depends on sessionStorage; if optimize results are cleared before navigation, Results stays disabled until the user re-runs optimization.

Rollout and Recovery

• Safe to merge and deploy with the frontend; no feature flags or migrations.
• Rollback: revert this PR; Results falls back to main layout without the left nav rail.
• No database or backend recovery steps required.

High-Signal PR Checklist

• The summary states the user-visible or operational outcome, not just file names.
• The motivation explains why this change is needed now.
• The change list separates frontend, backend, infrastructure, and documentation work where applicable.
• Validation includes exact commands run, relevant output, and any checks intentionally skipped.
• Risks, migrations, feature flags, and rollback steps are called out when relevant.
• Screenshots or request/response examples are included for UI and API behavior changes.

[kirillakovalenko]

PR #151 — HI-FI: Left Navigation Strip on Results

Author: Kesar Singh Sidhu | +98 / -53 across 5 files

---

Overview

Adds a shared left navigation rail to the Results page by reusing EditSidebar, SidebarEditButton, and SidebarResultsButton from the edit page. Introduces a readHasOptimizeResults() utility to gate the Results link behind a sessionStorage check. The approach is sound and the code is generally clean, but there are a few correctness and style issues worth addressing before merge.

---

Bugs / Correctness Issues

1. "Save" button is always rendered — regression in results/page.tsx

In the original code, both Cancel and Save were inside the {pendingPinMove != null && ...} guard. In the new code, only Cancel is guarded — Save renders unconditionally:

{pendingPinMove != null && (
  <button onClick={cancelPendingPinMove}>Cancel</button>
)}
<button onClick={savePendingPinMove}>Save</button>  // ← always visible

Calling savePendingPinMove when pendingPinMove is null will likely no-op, but the button should not be visible at all outside of pin-move mode. This matches the old behavior and the mock.

2. Export button has no onClick — silently non-functional

<button
  type="button"
  className="... bg-[var(--edit-teal-500)] ..."
>
  Export
</button>

No handler is wired. If this is a known placeholder, it should at minimum be disabled so users don't click it and wonder why nothing happens. If export logic is out of scope for this PR, that should be called out explicitly.

3. Dead CSS transition on isSidebarOpen

The sidebar toggle was removed and isSidebarOpen is now hardcoded to true, but the width transition class is still conditioned on it:

const isSidebarOpen = true;
// ...
<div className={`... transition-[width] duration-300 ease-in-out ${isSidebarOpen ? "w-72" : "w-0"}`}>

The ternary will always resolve to "w-72". The transition CSS and ternary are dead code — simplify to just w-72.

---

Design / Consistency Issues

4. routeColors.ts — redundant comment added

The comment added to routeColors.ts:11 explains what % (modulo) does. This goes against the project convention of only commenting non-obvious behavior. Remove it.

5. Sidebar.tsx — border-l-4 conflicts with border border-zinc-200

className="rounded-xl border border-zinc-200 border-l-4 bg-zinc-50 shadow-sm overflow-hidden"

The previous inset box-shadow approach was visually correct and avoided any border override concerns. border-l-4 overrides border-left-width set by the border class; in Tailwind v3 this works due to stylesheet ordering, but the intent is slightly clearer with border-l-[4px] or by replacing border with border-t border-r border-b to make the sides explicit. The inset shadow approach was arguably safer and didn't require borderLeftColor on the style prop.

---

Code Quality

6. SidebarResultsButton — readHasOptimizeResults() is not reactive

The function is called synchronously during render. If sessionStorage is updated (e.g., after a re-optimization in another part of the app) without a component re-render, the button state will be stale. For this project's architecture this is probably acceptable, but it is worth documenting or using a useState + useEffect pattern:

const [hasStoredRoutes, setHasStoredRoutes] = useState(false);
useEffect(() => { setHasStoredRoutes(readHasOptimizeResults()); }, []);

This also avoids any hydration mismatch risk since sessionStorage is unavailable server-side (even with the typeof window guard, the initial render value would differ between server and client).

7. hasOptimizeResults.ts — the SSR guard is the right call, but the cast is unsafe

const parsed = JSON.parse(stored) as Route[];

as Route[] only satisfies TypeScript, it doesn't validate the shape at runtime. If sessionStorage is corrupted or written by a different version of the app, parsed.length > 0 will still return true. A minimal guard like checking parsed[0]?.vehicleId or similar would be more robust.

---

Missing Validation

The PR checklist explicitly left unchecked:

• npm run test — no test coverage for readHasOptimizeResults, which has meaningful branching logic (SSR guard, parse failure, empty array).
• npm run build — should be run before merge; the component graph changes are non-trivial.

---

Summary

  |   -- | -- Severity | Save button always visible is a visible regression; Export button being non-functional is a UX issue. Risk | Otherwise low — UI-only, no API/data model changes. Blockers | Items 1 and 2 (Save button regression, Export non-functional or disabled). Suggested before merge | Fix Save button guard, disable/remove Export, simplify isSidebarOpen, remove redundant comment, run build + add one test for readHasOptimizeResults. PR #151 — HI-FI: Left Navigation Strip on Results Author: Kesar Singh Sidhu | +98 / -53 across 5 files

Overview
Adds a shared left navigation rail to the Results page by reusing EditSidebar, SidebarEditButton, and SidebarResultsButton from the edit page. Introduces a readHasOptimizeResults() utility to gate the Results link behind a sessionStorage check. The approach is sound and the code is generally clean, but there are a few correctness and style issues worth addressing before merge.

Bugs / Correctness Issues

1. "Save" button is always rendered — regression in results/page.tsx

In the original code, both Cancel and Save were inside the {pendingPinMove != null && ...} guard. In the new code, only Cancel is guarded — Save renders unconditionally:

{pendingPinMove != null && (
Cancel
)}
Save // ← always visible
Calling savePendingPinMove when pendingPinMove is null will likely no-op, but the button should not be visible at all outside of pin-move mode. This matches the old behavior and the mock.

2. Export button has no onClick — silently non-functional

<button
type="button"
className="... bg-[var(--edit-teal-500)] ..."

Export

No handler is wired. If this is a known placeholder, it should at minimum be disabled so users don't click it and wonder why nothing happens. If export logic is out of scope for this PR, that should be called out explicitly.

3. Dead CSS transition on isSidebarOpen

The sidebar toggle was removed and isSidebarOpen is now hardcoded to true, but the width transition class is still conditioned on it:

const isSidebarOpen = true;
// ...

The ternary will always resolve to "w-72". The transition CSS and ternary are dead code — simplify to just w-72.

Design / Consistency Issues
4. routeColors.ts — redundant comment added

The comment added to routeColors.ts:11 explains what % (modulo) does. This goes against the project convention of only commenting non-obvious behavior. Remove it.

5. Sidebar.tsx — border-l-4 conflicts with border border-zinc-200

className="rounded-xl border border-zinc-200 border-l-4 bg-zinc-50 shadow-sm overflow-hidden"
The previous inset box-shadow approach was visually correct and avoided any border override concerns. border-l-4 overrides border-left-width set by the border class; in Tailwind v3 this works due to stylesheet ordering, but the intent is slightly clearer with border-l-[4px] or by replacing border with border-t border-r border-b to make the sides explicit. The inset shadow approach was arguably safer and didn't require borderLeftColor on the style prop.

Code Quality
6. SidebarResultsButton — readHasOptimizeResults() is not reactive

The function is called synchronously during render. If sessionStorage is updated (e.g., after a re-optimization in another part of the app) without a component re-render, the button state will be stale. For this project's architecture this is probably acceptable, but it is worth documenting or using a useState + useEffect pattern:

const [hasStoredRoutes, setHasStoredRoutes] = useState(false);
useEffect(() => { setHasStoredRoutes(readHasOptimizeResults()); }, []);
This also avoids any hydration mismatch risk since sessionStorage is unavailable server-side (even with the typeof window guard, the initial render value would differ between server and client).

7. hasOptimizeResults.ts — the SSR guard is the right call, but the cast is unsafe

const parsed = JSON.parse(stored) as Route[];
as Route[] only satisfies TypeScript, it doesn't validate the shape at runtime. If sessionStorage is corrupted or written by a different version of the app, parsed.length > 0 will still return true. A minimal guard like checking parsed[0]?.vehicleId or similar would be more robust.

Missing Validation
The PR checklist explicitly left unchecked:

npm run test — no test coverage for readHasOptimizeResults, which has meaningful branching logic (SSR guard, parse failure, empty array).
npm run build — should be run before merge; the component graph changes are non-trivial.
Summary
Severity Save button always visible is a visible regression; Export button being non-functional is a UX issue.
Risk Otherwise low — UI-only, no API/data model changes.
Blockers Items 1 and 2 (Save button regression, Export non-functional or disabled).
Suggested before merge Fix Save button guard, disable/remove Export, simplify isSidebarOpen, remove redundant comment, run build + add one test for readHasOptimizeResults.

[KesarSidhu]

@markboenigk