Adding Save Edits Button on Mobile

[KesarSidhu]

Summary

• Adds a Save edits button to the mobile results navbar (top-right) while edit mode is active.
• Mirrors desktop behavior: clicking Save edits commits any pending pin drag and exits edit mode via handleEditModeChange(false).
• Cancel still appears when a pin drag is pending and discards the move without exiting edit mode.

Motivation

• Desktop has a clear Edit → Save edits flow in the sidebar; mobile only had an Edit toggle in the bottom sheet and a contextual pin-only Save in the navbar that was disabled until a drag occurred.
• Users entering edit mode on mobile had no obvious way to save and exit after dragging a marker, matching the confusion fixed on desktop in saving-marker-location.
• The hi-fi mockup places Save in the top-right of the mobile header — this aligns mobile with that design and desktop parity.

Changes

Frontend (app/ui)

• MobileResultsNavbar.tsx
  • Show Save edits in the top-right when isEditMode is true.
  • Show Cancel next to it when a pin drag is pending.
  • Remove the old always-visible pin-only Save button and saveDisabled prop.
• page.tsx
  • Wire isEditMode and onSaveEdits={() => handleEditModeChange(false)} to the mobile navbar.

Backend / mobile app / infra

• No backend, mobile app, or infrastructure changes in this PR.

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/ui run test:e2e
• 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 checks

1. Mobile viewport → /results → Edit (bottom sheet) → Save edits appears top-right.
2. Drag a marker → Cancel appears next to Save edits.
3. Save edits → marker stays at new location, edit mode exits.
4. Drag a marker → Cancel → marker snaps back, stays in edit mode.
5. Desktop behavior unchanged (sidebar Save edits still works).

Risk

• Low: Mobile navbar UI only; reuses existing handleEditModeChange commit-on-exit logic from saving-marker-location.
• Bottom sheet Edit button still enters edit mode; Save edits is now the primary exit path in the header (intentional, matches desktop).

Rollout and Recovery

• Frontend-only deploy of app/ui; no migrations or feature flags.
• Suggested base branch: saving-marker-location (this branch adds one commit on top).
• Roll back by reverting this PR; mobile will lose the header Save edits button.

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.

[markboenigk]

Review summary

Nice cleanup on the mobile Save/Cancel flow, and the handleEditModeChange fix (committing a pending pin drag instead of silently discarding it on exit) is a good catch that helps desktop too. One correctness issue in Map.tsx I'd want fixed before merge, plus a few smaller notes.

🔴 Blocking: duplicate/ghost polylines on the map

In RoutePolylinesOverlay (Map.tsx), the polyline-drawing effect ([map, routesPathKey]) and the pin-sync effect ([map, routes, pendingPinMove]) both write into polylinesByVehicleRef.current. On mount (and on most route changes) they run in the same commit, before any DirectionsService fetch can resolve:

1. Effect 1 clears all polylines and kicks off the sequential Directions fetch.
2. Effect 2 runs right after, sees no polyline yet for each route, and draws a straight-line fallback for every route.
3. As Effect 1's fetches resolve one at a time, it overwrites polylinesByVehicleRef.current[vehicleId] with the road-following polyline — without calling .setMap(null) on the fallback it's replacing.

The fallback stays attached to the map, orphaned from the ref, forever. Net effect: every route gets a permanent ghost straight line under/crossing the real route, on basically every load with 1+ routes.

Suggested fix — dispose the previous entry before every write in drawFallback and the cache-hit/road-result branches:

const prev = polylinesByVehicleRef.current[route.vehicleId];
if (prev) prev.setMap(null);
polylinesByVehicleRef.current[route.vehicleId] = roadPoly;

🟡 Worth a look

• requestDrivingDirections() wrapper: DirectionsService.route() already supports a Promise-returning form (no callback) with reject-on-non-OK semantics per Google's docs, so the original await directionsService.route(...) was likely already correct. Was this rewrite fixing an observed bug, or just defensive? If the latter, the original one-liner is simpler.
• Sequential Directions requests: the "one at a time" loop avoids rate limits per the comment, but scales draw time linearly with route count and widens the ghost-polyline window above. A small concurrency cap (2–3 in flight) might get the same rate-limit protection with less UX cost.
• page.tsx handleEditModeChange: the explicit setPendingPinMove(null) after savePendingPinMove() is redundant — that function already nulls it (or no-ops).

Test coverage

test:e2e is unchecked — given the ghost-polyline bug above is exactly the kind of async/rendering issue e2e would catch, worth running before merge.

Nit

The PR description only documents the mobile navbar change, but the diff also includes a substantial Map.tsx rewrite and a client-validation-store change from later commits — might be worth updating the description so it reflects the full scope.