feat(ui): improve delivery time handling

[Gill87]

Summary

• Vehicle add/edit departure time now defaults blank minutes to 00 after a valid hour is entered, allowing common on-the-hour departures to save without an extra manual minute entry.
• Confirmed address cards now distinguish one-sided delivery windows: start-only windows show From <time>, end-only windows show By <time>, full ranges and empty windows keep their prior display.

Motivation

• Issue feat: Add "00" in the minute slot when a number is entered in the hour slot (vehicle creation) #209 reports that users often type only an hour for vehicle departure time, then hit validation because the separate minutes field stays blank.
• Issue feat: Distinguish Start and End Delivery Times #211 reports that confirmed delivery cards displayed start-only and end-only windows as identical bare times, making the schedule ambiguous while scanning the route setup list.
• These UI improvements reduce form friction and make locked delivery details clearer before optimization.

Changes

• Frontend: added vehicle departure-time helper logic that fills blank minutes with 00 only for valid hours from 1 through 12.
• Frontend: tracks whether 00 was generated by the UI so user-entered minutes, including a deliberate 00, are not overwritten.
• Frontend: added a shared locked delivery-time formatter and reused it in both desktop and mobile locked address-card views.
• Frontend tests: added focused Vitest coverage for departure minute defaulting, generated-minute clearing, user-entered minute preservation, and locked delivery-window labels.
• Backend, infrastructure, mobile app, and documentation behavior are unchanged by this PR.

Validation

Frontend

• npm --prefix app/ui test -- deliveryHelpers.test.ts — passed: 1 test file, 10 tests.
• npm --prefix app/ui run typecheck — passed.
• npm --prefix app/ui test — passed: 24 test files, 122 tests.
• npm --prefix app/ui run lint — skipped locally; CI will run the full lint gate for the branch.
• npm --prefix app/ui run format:check — skipped locally; CI will run the formatting gate for the branch.
• npm --prefix app/ui run build — skipped locally; CI will run the production build gate for the branch.

Other Checks

• npm --prefix app/mobile run lint — skipped locally because the mobile app is outside this UI change.
• npm --prefix app/mobile run typecheck — skipped locally because the mobile app is outside this UI change.
• cmake --preset dev — skipped locally because backend code is outside this UI change.
• .github/scripts/check-backend-static.sh build/dev — skipped locally because backend code is outside this UI change.
• cmake --build --preset dev --parallel — skipped locally because backend code is outside this UI change.
• ctest --preset dev --output-on-failure --no-tests=error -LE 'e2e|docker' — skipped locally because backend code is outside this UI change.

Risk

• Low UI risk: both changes are limited to edit-page time handling and confirmed address-card display text.
• The main departure-time risk is accidentally overwriting custom minutes; generated-minute state and focused tests cover user-entered minute preservation.
• The main address-card risk is inconsistent desktop/mobile wording; both locked views call the same formatter and share unit coverage.
• Saved data, validation constraints, solver payloads, and backend behavior are not changed.

Rollout and Recovery

• Rollout follows the normal frontend deployment path for the Next.js UI.
• Recovery is a small revert of the departure-time helper, locked delivery-window formatter, and their focused tests if unexpected editing or display behavior appears.
• Data migration, feature flag setup, and backfill work are not required.

High-Signal PR Checklist

• The summary states the user-visible outcome.
• The motivation explains why the changes are needed.
• The change list separates frontend work from unaffected areas.
• Validation includes exact commands run and checks intentionally left to CI.
• Risks and rollback steps are called out.
• UI behavior is described in text and covered by focused unit tests.

[markboenigk]

Overall: Clean, well-scoped PR — the formatter extraction and the From/By label logic are solid, and the unit tests cover the intent clearly. Three things worth a look before merge:

---

VehicleDetailsOverlay.tsx ~line 474 — side effect in state updater (correctness)

minutesAutoFilledRef.current is mutated inside the setMinutes functional updater. React 18 Strict Mode calls state updaters twice with the same previous state, and because the first invocation mutates the ref, the second invocation reads a stale value and can produce a different result.

Concrete case: auto-filled 00 is in minutes (ref = true), user then changes the hour to an invalid value (e.g. "0").

• First run → returns "", sets ref = false
• Second run → reads ref = false, returns currentMinutes ("00") instead of ""

React keeps the second result, so the minutes field fails to clear. Easiest fix is to move the ref update out of the updater and into the onChange handler directly after setMinutes:

const nextMinutes = getMinutesAfterHourChange(val, minutes, minutesAutoFilledRef.current);
minutesAutoFilledRef.current = nextMinutes === "00" && (minutes === "" || minutesAutoFilledRef.current);
setMinutes(nextMinutes);

(Reading minutes from the closure is fine here since this is a synchronous event handler, not an async update.)

---

VehicleDetailsOverlay.tsx line 96 — utility placement

getMinutesAfterHourChange is a pure function with no React dependencies. Exporting it from a component file is a bit surprising — the natural home is deliveryHelpers.ts, where the other delivery/time helpers live and where vehicleDepartureTime.test.ts could import it without touching a component. Minor, but worth moving if you're already touching the file.

---

VehicleDetailsOverlay.tsx lines 89 & 102 — duplicated hour bound

isValidTime and getMinutesAfterHourChange each independently encode >= 1 && <= 12. Right now they agree, but they're easy to drift apart. A shared constant or a small isValidHour(h: number) predicate would keep them locked together.

[Gill87]

Hey Mark,
Thank you for your feedback! I went ahead and addressed your concerns, thus the PR should be ready to merge now.

[markboenigk]

All three points from the last round are fixed: the ref mutation is out of the setMinutes updater, getMinutesAfterHourChange/isValidDepartureHour live in deliveryHelpers.ts, and both call sites share the one hour-bound predicate now.

One new issue came up in the rework, plus a couple of minor things worth a look before merge:

---

VehicleDetailsOverlay.tsx lines 158-164 & 450-467 — stale focusMinutesAfterRenderRef can steal focus mid-edit

focusMinutesAfterRenderRef.current = true is set unconditionally in the hours onChange whenever the value reaches 2 digits, and is only consumed/reset by the new useEffect keyed on [hours, minutes]. React bails out of re-rendering (and skips effects) when setHours/setMinutes are called with a value equal to current state.

Concrete case: hour is "12", minutes auto-filled to "00". User re-selects and retypes "1", "2" (same value) — setHours/setMinutes are no-ops, so the effect never runs and the flag stays true. User then backspaces to "1" — a real change that per the design shouldn't move focus (val.length !== 2). This time the effect does run, sees the stale flag, and jumps focus + selects the minutes field while the user is still mid-edit in hours.

Probably easiest to gate the ref set on the actual "did this change" condition rather than just val.length === 2, or clear the flag explicitly whenever the effect's guard condition wasn't met on the previous render.

---

deliveryHelpers.ts lines 157-161 — isValidDepartureHour accepts partially-numeric strings

export function isValidDepartureHour(hour: string | number): boolean {
  const hourNumber = typeof hour === "number" ? hour : parseInt(hour.trim(), 10);
  return hourNumber >= 1 && hourNumber <= 12;
}

parseInt ignores trailing garbage, so isValidDepartureHour("9am") or "1x" returns true. It's masked today because the only caller pre-strips non-digits, but it's a shared, exported utility now — a future caller passing unsanitized input (CSV import, another form) would silently accept invalid hours. Worth a stricter check, e.g. /^\d{1,2}$/.test(hour.trim()) before parsing.

---

VehicleDetailsOverlay.tsx lines 491-493 vs 158-164 — duplicate .select() call

Calling minutesRef.current?.focus() in the effect synchronously fires the input's own onFocus handler, which already does if (minutesAutoFilledRef.current) e.target.select(). The effect then repeats the same check and calls .select() again right after. Harmless, but dead logic — the effect only needs the .focus() call; onFocus already owns the select-on-autofill behavior.

---

deliveryHelpers.ts line 172 / VehicleDetailsOverlay.tsx lines 460-462 — auto-fill condition duplicated across the seam

getMinutesAfterHourChange decides internally when to return "00", but the caller separately re-derives the same condition to update minutesAutoFilledRef:

minutesAutoFilledRef.current =
  nextMinutes === "00" && (minutes === "" || minutesAutoFilledRef.current);

If the helper's branching ever changes, this copy can quietly drift out of sync. Might be cleaner for the helper to return { minutes, autoFilled } so there's one owner of that decision.

---

Nothing else stood out — the AddressCard.tsx migration and the new test coverage look solid.

[Gill87]

Hey @markboenigk,
I went ahead and addressed the issues mentioned in your last comment, the PR should be good to merge.