Skip to content

HI-FI: Show Recipient And Delivery Text On Stop Cards#164

Open
KesarSidhu wants to merge 14 commits into
benevolentbandwidth:mainfrom
KesarSidhu:step3-edit-look-3-main-v5
Open

HI-FI: Show Recipient And Delivery Text On Stop Cards#164
KesarSidhu wants to merge 14 commits into
benevolentbandwidth:mainfrom
KesarSidhu:step3-edit-look-3-main-v5

Conversation

@KesarSidhu
Copy link
Copy Markdown
Contributor

@KesarSidhu KesarSidhu commented May 11, 2026

Summary

Expanded results stop cards show formatted recipient contact and delivery time text from mapped stop data, including optional delivery window ranges.

Motivation

Stop rows previously showed raw labels (“Name of addressed to”, single arrival time) and did not combine name and phone or render edit-form delivery windows. Drivers need a compact Recipient and Delivery line that matches the hi-fi expanded card layout.

Changes

  • app/ui/src/app/results/components/EditableStopItem.tsxformatContactLine (name · phoneNumber), formatDeliveryWindow (start–end or timeWindow fallback with By/From/At), Recipient and Delivery rows; package pill keeps “Packages” screen-reader label.
  • app/ui/src/app/results/types.ts — optional deliveryWindowStart and deliveryWindowEnd on Stop (recipient contact uses existing addresseeName and phoneNumber).
  • app/ui/src/app/edit/utils/vroomToRoutes.ts — map deliveryTimeStart / deliveryTimeEnd from address cards onto stop window fields after optimize.

Validation

Frontend

  • npm --prefix app/ui run lint
  • npm --prefix app/ui run typecheck
  • npm --prefix app/ui run format:check
  • 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 — results UI display only; falls back to arrival timeWindow when delivery window fields are unset.

Rollout and Recovery

Merge after step3-edit-look-3-main-v4 in the step3 stack. Revert if recipient or delivery formatting needs adjustment.

Kesar Sidhu and others added 12 commits May 24, 2026 18:31
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>
@KesarSidhu KesarSidhu force-pushed the step3-edit-look-3-main-v5 branch from d516709 to 29fa2d2 Compare May 25, 2026 02:20
Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown
Collaborator

@kirillakovalenko kirillakovalenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #164 — HI-FI: Show Recipient And Delivery Text On Stop Cards
Author: KesarSidhu | +597 / -221 across 15 files

Overview
This PR does more than its title suggests. Beyond formatting recipient/delivery text on stop cards, it also:

Redesigns the entire results sidebar (route cards, stop list layout, footer branding)
Adds dynamic active/disabled states to the SidebarResultsButton
Replaces Google Maps default pins with route-colored custom SVG pins
Adds the edit-page nav sidebar to the results page
Fixes capacityUsed to prefer the form's deliveryQuantity over the VROOM engine's load value
Bugs & Correctness Issues

  1. formatContactLine duplicates recipientSummary verbatim
    EditableStopItem.tsx:274-281 has identical logic to recipientSummary.ts. Import and reuse it — one function diverging from the other is a latent bug.

  2. "Save" button always rendered, not gated on pendingPinMove
    page.tsx:929-935 — the Save button is now always visible and calls savePendingPinMove unconditionally. Previously it was inside the pendingPinMove != null block. Clicking it with no pending move may silently no-op, but it's visually confusing and inconsistent with Cancel being conditionally rendered.

  3. Export button is a non-functional stub
    page.tsx:936-942 — no onClick handler. Should be left out or clearly marked as disabled until implemented.

  4. Sidebar toggle removed but isSidebarOpen conditional left behind
    page.tsx:858 — isSidebarOpen is hardcoded true and the toggle button is removed, but the width conditional isSidebarOpen ? "w-[28rem]" : "w-0" still references it. Either remove the variable entirely or restore the toggle.

Code Quality
Good:

hasOptimizeResults.ts is clean — proper SSR guard, safe JSON parse, focused responsibility.
recipientSummary.ts is well-extracted and tested thoroughly.
markerSvgDataUrl security comment is appropriate — calling out that fillColor is always a hex from the palette, never user input, is the right way to document this.
capacityUsed fix in vroomToRoutes.ts is correct and well-tested.
SidebarResultsButton correctly adds "use client" and uses aria-current="page" for the active state. Replacing the fake with a for the disabled state is a meaningful improvement.
Minor:

address?.deliveryQuantity && address.deliveryQuantity > 0 in vroomToRoutes.ts:214-216 — the first condition is redundant; address.deliveryQuantity > 0 already handles undefined/0. Not wrong, just noise.
Module-level let markerScaledSize in Map.tsx:348 persists across HMR reloads in dev. It's a valid lazy-init pattern for production but can cause stale-reference confusion when debugging.
Design Consistency
Stop sequence badges in Sidebar.tsx are hardcoded bg-cyan-500, while route cards use the accent color from routeColorHex. This looks intentional per the hi-fi design, but worth confirming — currently a stop from Route 2 (orange) has a cyan sequence badge.
The advanced marker anchor in Map.tsx:370 uses transform: translate(-50%, -50%). For a pin SVG whose tip is at the bottom center, this centers the element on the anchor point rather than aligning the pin tip to the location. This may cause markers to appear visually offset from their actual coordinate. Verify visually in the browser.
Test Coverage
Good additions:

recipientSummary.test.ts covers all four cases cleanly.
vroomToRoutes.test.ts covers capacityUsed fallback and recipient forwarding.
Missing:

No tests for hasOptimizeResults.ts.
formatTime12h and formatDeliveryWindow in EditableStopItem.tsx are untested — edge cases like midnight ("0:00"), noon ("12:00"), and already-formatted strings deserve coverage given the regex logic.
Validation Checklist
The PR has format:check, build, both mobile checks, and all backend checks unchecked. At minimum format:check and build should pass before merge.

Summary
The core feature (recipient/delivery formatting) is well-implemented and tested. The main issues to address before merge are: (1) deduplicate formatContactLine vs recipientSummary, (2) fix the Save button being always-visible, (3) remove or wire up the Export stub, and (4) verify the map pin anchor alignment visually.

Co-authored-by: Cursor <cursoragent@cursor.com>
@markboenigk
Copy link
Copy Markdown
Collaborator

Checked 2d5e2615 — Kirill's four items are all addressed. Good turnaround.

One correctness bug still standing:

Pin anchor regression (Map.tsx — AdvancedMarkers path)
wrapper.style.transform = "translate(-50%, -50%)" centers the element over the coordinate — the circle body lands on the stop, not the tip. This was fixed in #153's 6088dfe (translate(-50%, -100%)) but regressed here. The non-AdvancedMarkers path gets it right with anchor: new google.maps.Point(MARKER_ICON_WIDTH / 2, MARKER_ICON_HEIGHT).

Two minor things:

  • Cancel and Save still have identical styling — Save should read as the primary action
  • npm run format:check and npm run build still unchecked

Fix the pin anchor and confirm the build passes and this is good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants