HI-FI: Add Recipient Name And Phone Inputs To Edit Form

[KesarSidhu]

Summary

Edit-page address cards support recipient name and phone on desktop and mobile, with locked stops showing contact info only when at least one field is set.

Motivation

Drivers need recipient contact on optimized routes. The edit form must capture name and phone before optimization, and locked cards should not show an empty “Recipient —” row when both fields are blank.

Changes

• app/ui/src/app/edit/utils/recipientSummary.ts — shared formatter for name, phone, both (·), or neither; hasRecipientContact for gating.
• app/ui/src/app/edit/components/AddressCard.tsx — recipient name/phone inputs in edit state; locked/summary views use recipientSummary only when hasRecipientContact(a) is true (desktop, mobile summary, mobile expanded locked).
• app/ui/src/tests/recipientSummary.test.ts — tests for all four recipientSummary branches plus hasRecipientContact.

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

Risk

Low — edit UI only. Recipient fields use existing phoneNumber on AddressCard (wired to results in a separate PR). CSV import still leaves recipient fields empty (follow-up if bulk import is needed).

Rollout and Recovery

Ship with the edit page. Revert this PR if recipient layout or summary formatting needs adjustment.

[markboenigk]

Overall the implementation is clean — recipientSummary handles all four combinations correctly, type="tel" + autoComplete attributes are right on both inputs, and the mobile 2-column grid is consistent with other field pairs.

Two things to address before merge:

1. Recipient row always renders in locked state even when empty

recipientSummary returns "—" when both fields are blank, and the locked-state markup renders unconditionally. Every existing locked card — including stops with no recipient data — will now show an extra row displaying —. On mobile this is a visible labeled "Recipient / —" block for every stop. Consider gating the row:

{(a.recipientName || a.recipientPhone) && (
  <div className={ADDRESS_LOCKED_SURFACE_MD}>…</div>
)}

Same guard applies to the mobile locked block.

2. No test cases for recipientSummary's four branches

The function has clear logic (both, name-only, phone-only, neither) and is trivially testable. The test files are already being touched in this PR — adding four cases to cover each branch would be a low-friction add.

Minor: useCSVUpload.ts hardcodes empty strings for both fields with no CSV column mapping, so bulk-uploaded stops will always have blank recipient info. Looks like a deliberate scope cut — just worth a follow-up ticket if CSV import of recipient data is ever needed.

[kirillakovalenko]

Code Review: PR #162 — HI-FI: Add Recipient Name And Phone Inputs To Edit Form

Author: Kesar Singh Sidhu | Changes: +475 / -210 across 10 files | State: OPEN

---

Overview

This PR does three distinct things bundled together:

1. Extracts recipient name/phone display logic into a shared recipientSummary utility and updates AddressCard locked/summary views to use it.
2. Makes SidebarResultsButton context-aware (active/inactive/disabled states based on route and pathname).
3. Redesigns the /results page — new Sidebar layout with route cards, a stop connector timeline, branding footer, colored SVG map markers, and a unified nav header.

---

Correctness Issues

Bug: "Save" button always visible in results header

page.tsx:809-815 — The old code gated Save inside {pendingPinMove != null && (...)}. The new code puts Save unconditionally in the header alongside Cancel (which is still gated). Users will see a non-functional Save button at all times, even when no pin is being dragged.

Potential visual bug: Advanced marker anchor offset

Map.tsx:249 — The comment says "shift so the SVG tip sits on the stop" but translate(-50%, -50%) centers the 40px-tall pin element at the map point. This places the tip ~20px below the intended stop. For the pin tip to land on the coordinate, the transform should be translate(-50%, -100%).

---

Incomplete / Placeholder Code

• Export button has no onClick handler (page.tsx:817-821). It will silently do nothing. Either wire it up or keep it visually disabled with disabled/aria-disabled.
• … three-dot element in Sidebar (Sidebar.tsx:583) is a <span> with no interactivity. Users will treat it as a context menu trigger. Should be removed or wired up.
• Sidebar toggle removed — isSidebarOpen is now the constant true (page.tsx:738), removing the hide/show affordance entirely. If this is intentional, the transition-[width] CSS can also be removed as dead code.

---

Design/Style Inconsistencies

• Hardcoded cyan hex values (#7BCFC2, #6CCBBE) are mixed with var(--edit-teal-500) elsewhere in the same file. They appear to be slightly different shades (intentional?). Standardize to a single CSS variable.
• Stop sequence badges are now rendered as hardcoded bg-cyan-500 (Sidebar.tsx:657) instead of using the route accent color passed down. This means all stop numbers are cyan regardless of route color, breaking the per-route color coding established in the rest of the results UI.
• EditableStopItem receives accentColor (Sidebar.tsx:677) but the sequence badge was removed from that component. If accentColor is no longer used inside EditableStopItem, the prop is dead weight.

---

Code Quality

recipientSummary.ts / AddressCard.tsx — Clean extraction. The three locked-view call sites now use identical logic via the shared utility. Tests cover all four branches including the whitespace-only edge case. No issues.

hasOptimizeResults.ts — Well-written: SSR guard, safe JSON.parse, type annotation. Minor note: JSON.parse(...) as Route[] is a cast, not a runtime validation — a corrupt payload that is an array would pass, but returning true for a non-empty corrupt array is acceptable here since the results page will fail gracefully when it tries to render.

SidebarResultsButton.tsx — The three-state logic (active → inactive+routes → disabled) is clear. Replacing the disabled <Link href="#"> with a <span aria-disabled> is the correct accessibility fix.

markerSvgDataUrl — The inline comment noting fillColor is never user input is helpful given the direct SVG string interpolation. The module-level markerScaledSize singleton is fine but will persist across test runs if tests don't reload the module.

---

Validation Gaps

The PR checklist has several unchecked boxes:

• npm --prefix app/ui run build
• All mobile checks
• All backend checks

At minimum the UI build check should pass before merge to catch any import or type errors not caught by typecheck alone.

---

Summary

  |   -- | -- recipientSummary utility + tests | ✅ Good, merge-ready SidebarResultsButton active states | ✅ Good Advanced marker anchor offset | ⚠️ Likely visual bug Save button always visible | ❌ Bug — needs gating Export button placeholder | ⚠️ Ship disabled or wire up Stop badges use hardcoded cyan vs route accent | ⚠️ Breaks per-route color coding Build/mobile checks unchecked | ⚠️ Run before merge Code Review: PR #162 — HI-FI: Add Recipient Name And Phone Inputs To Edit Form Author: Kesar Singh Sidhu | Changes: +475 / -210 across 10 files | State: OPEN

Overview
This PR does three distinct things bundled together:

Extracts recipient name/phone display logic into a shared recipientSummary utility and updates AddressCard locked/summary views to use it.
Makes SidebarResultsButton context-aware (active/inactive/disabled states based on route and pathname).
Redesigns the /results page — new Sidebar layout with route cards, a stop connector timeline, branding footer, colored SVG map markers, and a unified nav header.
Correctness Issues
Bug: "Save" button always visible in results header

page.tsx:809-815 — The old code gated Save inside {pendingPinMove != null && (...)}. The new code puts Save unconditionally in the header alongside Cancel (which is still gated). Users will see a non-functional Save button at all times, even when no pin is being dragged.

Potential visual bug: Advanced marker anchor offset

Map.tsx:249 — The comment says "shift so the SVG tip sits on the stop" but translate(-50%, -50%) centers the 40px-tall pin element at the map point. This places the tip ~20px below the intended stop. For the pin tip to land on the coordinate, the transform should be translate(-50%, -100%).

Incomplete / Placeholder Code
Export button has no onClick handler (page.tsx:817-821). It will silently do nothing. Either wire it up or keep it visually disabled with disabled/aria-disabled.
… three-dot element in Sidebar (Sidebar.tsx:583) is a with no interactivity. Users will treat it as a context menu trigger. Should be removed or wired up.
Sidebar toggle removed — isSidebarOpen is now the constant true (page.tsx:738), removing the hide/show affordance entirely. If this is intentional, the transition-[width] CSS can also be removed as dead code.
Design/Style Inconsistencies
Hardcoded cyan hex values (#7BCFC2, #6CCBBE) are mixed with var(--edit-teal-500) elsewhere in the same file. They appear to be slightly different shades (intentional?). Standardize to a single CSS variable.
Stop sequence badges are now rendered as hardcoded bg-cyan-500 (Sidebar.tsx:657) instead of using the route accent color passed down. This means all stop numbers are cyan regardless of route color, breaking the per-route color coding established in the rest of the results UI.
EditableStopItem receives accentColor (Sidebar.tsx:677) but the sequence badge was removed from that component. If accentColor is no longer used inside EditableStopItem, the prop is dead weight.
Code Quality
recipientSummary.ts / AddressCard.tsx — Clean extraction. The three locked-view call sites now use identical logic via the shared utility. Tests cover all four branches including the whitespace-only edge case. No issues.

hasOptimizeResults.ts — Well-written: SSR guard, safe JSON.parse, type annotation. Minor note: JSON.parse(...) as Route[] is a cast, not a runtime validation — a corrupt payload that is an array would pass, but returning true for a non-empty corrupt array is acceptable here since the results page will fail gracefully when it tries to render.

SidebarResultsButton.tsx — The three-state logic (active → inactive+routes → disabled) is clear. Replacing the disabled with a is the correct accessibility fix.

markerSvgDataUrl — The inline comment noting fillColor is never user input is helpful given the direct SVG string interpolation. The module-level markerScaledSize singleton is fine but will persist across test runs if tests don't reload the module.

Validation Gaps
The PR checklist has several unchecked boxes:

npm --prefix app/ui run build
All mobile checks
All backend checks
At minimum the UI build check should pass before merge to catch any import or type errors not caught by typecheck alone.

Summary
recipientSummary utility + tests ✅ Good, merge-ready
SidebarResultsButton active states ✅ Good
Advanced marker anchor offset ⚠️ Likely visual bug
Save button always visible ❌ Bug — needs gating
Export button placeholder ⚠️ Ship disabled or wire up
Stop badges use hardcoded cyan vs route accent ⚠️ Breaks per-route color coding
Build/mobile checks unchecked ⚠️ Run before merge

[markboenigk]

Both concerns from my earlier comment are resolved: the empty locked row is now gated by hasRecipientContact and all four recipientSummary branches are tested. kirillakovalenko's issues are also addressed in the latest commit — Save button is correctly gated, pin anchor is translate(-50%, -100%), and badges use the route accent color. Code is ready. Blocked only by merge order — needs #160 and #161 to land first, then a rebase onto main. No major blockers after that.