HI-FI: Base Route Card UI and Add B2 Footer Logo

[KesarSidhu]

Summary

• Reworks Results sidebar route cards to match the hi-fi mock (spacing, typography, colored icon tile, metrics row, and header controls).
• Replaces the footer b² text with the provided b2-logo.png asset via next/image.

Motivation

• Collapsed route cards on Results still used the earlier left-strip layout and did not match the latest hi-fi mock for route headers, stats, and vehicle row styling.
• The footer should use the official B² brand asset instead of styled text for consistent branding.

Changes

Frontend

• app/ui/src/app/results/components/Sidebar.tsx
  • Redesigned collapsed route cards: rounded-[24px], white card background, updated padding and typography.
  • Route icon in a colored h-11 w-11 tile with SVG path styling; driver name beside route title.
  • Metrics row (STOPS / DISTANCE / TIME) with updated labels and sizing.
  • Vehicle row with truck icon; ellipsis and chevron controls on the card header.
  • Edit button focus ring; edit-mode card border treatment preserved from step3-edit-look-1.
  • Footer: next/image logo (/b2-logo.png) instead of text “b²”.
• app/ui/public/b2-logo.png
  • Added provided B² logo asset for the Results sidebar footer.
• app/ui/src/app/results/page.tsx
  • Minor layout tweak (2 lines) aligned with sidebar updates.

Note: This PR builds on step3-edit-look-1 (edit-mode panel, map banner, wider sidebar, pins, nav). Merge or base on that branch first.

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: on /results, expand/collapse route cards — confirm icon tile, stats, vehicle row, and footer B² image; toggle edit mode — card borders and Save edits still behave as on step3-edit-look-1.

Risk

• Low. UI-only sidebar and static asset changes.
• b2-logo.png is ~130 KB; acceptable for footer but adds to deploy size.
• Depends on step3-edit-look-1 for edit-mode and page shell behavior.

Rollout and Recovery

• Safe to merge after step3-edit-look-1; no migrations or feature flags.
• Rollback: revert this PR to restore prior route-card layout and text footer.
• Remove app/ui/public/b2-logo.png only if reverting the logo change.

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#154 — HI-FI: Base Route Card UI and Add B2 Footer Logo

Author: KesarSidhu | Branch: step3-edit-look-2 → main | +371/-187, 6 files

---

Overview

This PR redesigns the Results sidebar route cards to match the hi-fi mock (colored icon tiles, metrics row, driver name, vehicle row, ellipsis/chevron controls), replaces the footer text with b2-logo.png via next/image, adds colored route pins to the map, and upgrades SidebarResultsButton to display active/inactive/disabled states based on route data and current pathname.

---

Correctness Issues

[Bug] "Save" button is always rendered regardless of pendingPinMove state (page.tsx:676-681)

The old code wrapped both Cancel and Save inside {pendingPinMove != null && (...)}. The refactor moved Save outside that conditional, making it always visible and always calling savePendingPinMove even when pendingPinMove === null.

// Current (broken):
{pendingPinMove != null && (<button onClick={cancelPendingPinMove}>Cancel</button>)}
<button onClick={savePendingPinMove}>Save</button>  // ← always rendered
<button>Export</button>
// Should be:
{pendingPinMove != null &amp;&amp; (
  &lt;&gt;
    &lt;button onClick={cancelPendingPinMove}&gt;Cancel&lt;/button&gt;
    &lt;button onClick={savePendingPinMove}&gt;Save&lt;/button&gt;
  &lt;/&gt;
)}

[Regression] Sidebar toggle removed (page.tsx:604)

isSidebarOpen is hardcoded to true, and the toggle button was removed. This silently kills the collapse feature. If the hi-fi spec calls for no toggle, add a comment; otherwise, this needs restoring.

---

Code Quality

Module-level mutable singleton in Map.tsx (Map.tsx:117)

let markerScaledSize: google.maps.Size | undefined;

This is a module-level mutable variable that is lazily initialized. While harmless in practice (same google.maps.Size value every time), it leaks state across renders. A useMemo or local variable inside the component would be cleaner, or at minimum a note that this is intentional.

Non-functional Export button (page.tsx:682-688)

The Export button has no onClick handler. This is fine for a hi-fi stub, but it should have a disabled state or a // TODO comment so it's not mistaken for a working control.

Ellipsis … inside the expand button has no action (Sidebar.tsx:472)

The … is rendered as a <span> inside the collapse <button>. If it's meant to open a context menu (as hi-fi mocks typically show), it should be a separate interactive element. If it's purely decorative, aria-hidden should be added.

Edit-mode visual indicator removed from sidebar container (Sidebar.tsx:228)

The old <aside> toggled border-amber-500 in edit mode. The new <aside> always uses border-zinc-200. The individual cards now show border-[#6CCBBE], but the sidebar frame no longer has a visual affordance. This may be intentional per the new design, but it was an explicit design feature before.

---

New Utility — hasOptimizeResults.ts

The implementation is clean and correct for its purpose. One minor note:

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

return Array.isArray(parsed) && parsed.length > 0;

The as Route[] is a type assertion with no runtime validation of shape — only presence is checked. This is acceptable since this app wrote the data itself, but if the schema of Route evolves, stale sessionStorage data will silently pass this check. Consider a zod or lightweight schema check if the type has required fields that must be present.

---

Accessibility

• aria-hidden added to SVG icons — good.
• fill="currentColor" in SidebarResultsButton — good, inherits color from context.
• aria-disabled on a bare <span> (SidebarResultsButton.tsx:77): aria-disabled is only meaningful on interactive roles. A <span> with no role won't convey the disabled state to screen readers. Since this is purely non-interactive, removing aria-disabled entirely is fine.
• aria-current="page" on the active results nav item — good addition.

---

Branch Dependency Risk

The PR body states:

"Merge or base on that branch first." (referring to step3-edit-look-1)

But baseRefName is main. If step3-edit-look-1 has not yet been merged, this diff will include its changes too, and rebasing/merging order matters. Confirm that step3-edit-look-1 is merged before this, or rebase this branch on top of it and retarget the PR accordingly.

---

Validation Gaps

• npm run test and npm run build are unchecked. For a UI PR of this size (371 additions), a passing build check is the minimum bar — runtime errors in JSX won't be caught by lint/typecheck alone.
• No screenshots included despite the checklist item being unchecked.

---

Summary

Must fix	Always-visible Save button (correctness regression)
Should fix	Confirm or restore sidebar toggle intent; Export button needs disabled or handler
Minor	Ellipsis aria-hidden, aria-disabled on , module-level markerScaledSize
Process	Run build + tests; verify branch ordering with step3-edit-look-1; add screenshots

The UI refactor itself is well-structured and the new hasOptimizeResults utility is a clean addition. The Save button regression is the only correctness bug that must be addressed before merging.

PR#154 — HI-FI: Base Route Card UI and Add B2 Footer Logo Author: KesarSidhu | Branch: step3-edit-look-2 → main | +371/-187, 6 files

Overview
This PR redesigns the Results sidebar route cards to match the hi-fi mock (colored icon tiles, metrics row, driver name, vehicle row, ellipsis/chevron controls), replaces the footer text with b2-logo.png via next/image, adds colored route pins to the map, and upgrades SidebarResultsButton to display active/inactive/disabled states based on route data and current pathname.

Correctness Issues
[Bug] "Save" button is always rendered regardless of pendingPinMove state (page.tsx:676-681)

The old code wrapped both Cancel and Save inside {pendingPinMove != null && (...)}. The refactor moved Save outside that conditional, making it always visible and always calling savePendingPinMove even when pendingPinMove === null.

// Current (broken):
{pendingPinMove != null && (<button onClick={cancelPendingPinMove}>Cancel</button>)}
<button onClick={savePendingPinMove}>Save</button>  // ← always rendered
<button>Export</button>

// Should be:
{pendingPinMove != null && (
  <>
    <button onClick={cancelPendingPinMove}>Cancel</button>
    <button onClick={savePendingPinMove}>Save</button>
  </>
)}

[Regression] Sidebar toggle removed (page.tsx:604)

isSidebarOpen is hardcoded to true, and the toggle button was removed. This silently kills the collapse feature. If the hi-fi spec calls for no toggle, add a comment; otherwise, this needs restoring.

Code Quality
Module-level mutable singleton in Map.tsx (Map.tsx:117)

let markerScaledSize: google.maps.Size | undefined;
This is a module-level mutable variable that is lazily initialized. While harmless in practice (same google.maps.Size value every time), it leaks state across renders. A useMemo or local variable inside the component would be cleaner, or at minimum a note that this is intentional.

Non-functional Export button (page.tsx:682-688)

The Export button has no onClick handler. This is fine for a hi-fi stub, but it should have a disabled state or a // TODO comment so it's not mistaken for a working control.

Ellipsis … inside the expand button has no action (Sidebar.tsx:472)

The … is rendered as a inside the collapse . If it's meant to open a context menu (as hi-fi mocks typically show), it should be a separate interactive element. If it's purely decorative, aria-hidden should be added.

Edit-mode visual indicator removed from sidebar container (Sidebar.tsx:228)

The old

toggled border-amber-500 in edit mode. The new always uses border-zinc-200. The individual cards now show border-[#6CCBBE], but the sidebar frame no longer has a visual affordance. This may be intentional per the new design, but it was an explicit design feature before.

New Utility — hasOptimizeResults.ts
The implementation is clean and correct for its purpose. One minor note:

const parsed = JSON.parse(stored) as Route[];
return Array.isArray(parsed) && parsed.length > 0;
The as Route[] is a type assertion with no runtime validation of shape — only presence is checked. This is acceptable since this app wrote the data itself, but if the schema of Route evolves, stale sessionStorage data will silently pass this check. Consider a zod or lightweight schema check if the type has required fields that must be present.

Accessibility
aria-hidden added to SVG icons — good.
fill="currentColor" in SidebarResultsButton — good, inherits color from context.
aria-disabled on a bare (SidebarResultsButton.tsx:77): aria-disabled is only meaningful on interactive roles. A with no role won't convey the disabled state to screen readers. Since this is purely non-interactive, removing aria-disabled entirely is fine.
aria-current="page" on the active results nav item — good addition.
Branch Dependency Risk
The PR body states:

"Merge or base on that branch first." (referring to step3-edit-look-1)

But baseRefName is main. If step3-edit-look-1 has not yet been merged, this diff will include its changes too, and rebasing/merging order matters. Confirm that step3-edit-look-1 is merged before this, or rebase this branch on top of it and retarget the PR accordingly.

Validation Gaps
npm run test and npm run build are unchecked. For a UI PR of this size (371 additions), a passing build check is the minimum bar — runtime errors in JSX won't be caught by lint/typecheck alone.
No screenshots included despite the checklist item being unchecked.
Summary
Must fix Always-visible Save button (correctness regression)
Should fix Confirm or restore sidebar toggle intent; Export button needs disabled or handler
Minor Ellipsis aria-hidden, aria-disabled on , module-level markerScaledSize
Process Run build + tests; verify branch ordering with step3-edit-look-1; add screenshots
The UI refactor itself is well-structured and the new hasOptimizeResults utility is a clean addition. The Save button regression is the only correctness bug that must be addressed before merging.

[markboenigk]

Review

Checked the full diff plus the latest fix commit (0fd315db — "Address review fixes on edit look 2").

Three bugs fixed in the latest commit — good catch:

• isSidebarOpen restored as useState (was hardcoded true, making the sidebar permanently open)
• Hamburger button restored in the results header
• Save button moved back inside the pendingPinMove guard (was unconditionally visible)

All CI checks pass (backend, mobile, ui, pr-description). ✅

---

Three items still worth addressing:

1. Save and Cancel buttons are visually identical (page.tsx)
Both share the same class string. Save is the primary/destructive action on a pin drag — it should have a distinct visual weight (e.g. bg-[var(--edit-teal-500)] to match the Edit button style).

2. Export button has no onClick (page.tsx)
It's a live, clickable button that does nothing. Either wire it up or render it disabled:

<button type="button" disabled aria-disabled="true" className="... opacity-50 cursor-not-allowed">
  Export
</button>

3. Advanced marker anchor may be off (Map.tsx:139)
translate(-50%, -50%) places the element's center at the coordinate. For a teardrop pin you want the tip (bottom edge) at the stop, which needs translate(-50%, -100%). Worth a quick browser check against a known stop position.

---

Overall the card redesign and nav wiring look solid. Minor items above are easy fixes — happy to approve once they're addressed or confirmed intentional.