Add trip progress to iOS Live Activity - #124
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
smart-trip-web | b037f7e | Commit Preview URL Branch Preview URL |
Aug 07 2026, 08:11 PM |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 884a116535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| TripProgressTrack( | ||
| model: model, | ||
| completedColor: .white, | ||
| remainingColor: .white.opacity(0.28), | ||
| markerFill: .white, | ||
| labelColor: .white.opacity(0.72) | ||
| ) |
There was a problem hiding this comment.
Keep the lock-screen activity within the 160-point limit
Adding this track makes the lock-screen layout intrinsically taller than ActivityKit's 160-point maximum: the track contributes roughly 34 points plus a new 10-point VStack gap, taking the existing approximately 129-point banner to about 173 points before any larger Dynamic Type sizing. On the lock screen this causes iOS to compress or truncate the activity, potentially hiding the progress track or existing trip information; the layout needs to reclaim vertical space or use a more compact lock-screen variant.
Useful? React with 👍 / 👎.
| private func progressValues(at now: Date) -> Values { | ||
| guard let departure = model.departureDate, | ||
| let arrival = model.arrivalDate, | ||
| arrival > departure else { |
There was a problem hiding this comment.
Stop rendering journey progress for cancelled trips
When a cancellation update arrives, the departure and arrival dates remain valid, so this calculation ignores model.isCanceled and continues advancing the marker through the alarm, walking, and train stages beneath the terminal “Cancelled” headline. In particular, a cancelled trip left visible past its scheduled departure appears to begin and complete a train journey that will not occur; hide the track or return a terminal, non-progressing state when isCanceled is true.
Useful? React with 👍 / 👎.
JamieRuderman
left a comment
There was a problem hiding this comment.
Code Review
A single-file, +244/−47 SwiftUI change adding a TripProgressTrack to the lock-screen banner and expanded Dynamic Island. The progress math is clean, well-clamped, and derived entirely from timestamps already in the content state — no new push traffic, no new contract with the TS side. Main gaps are terminal-state handling and refresh cadence.
Must fix
1. progressValues ignores isCanceled / isEnded — ios/App/SmartTripWidget/TripActivityWidget.swift:899
A cancelled trip still renders a live, advancing train timeline underneath a headline that reads "Cancelled". Likewise isEnded (the pushed arrival flag) is ignored — if the server ends the activity before arrivalDate, the headline says "Arrived" while the bar sits at, say, 80%.
Both existing surfaces (isArrived(model), HeadlineCountdown) already branch on these; the track should too — either hide it entirely or pin fraction to 1.0 for the terminal states.
Suggestions
2. 5-second refresh cadence is far more than the visual needs — TripActivityWidget.swift:798
.periodic(from: .now, by: 5) over an ~80-minute activity is ~1000 redraws, and each one moves the marker by ~0.1% of the track — imperceptible. Live Activity view refreshes are budgeted and battery-relevant; 30s is still smoother than the eye can follow here. The only leg where a fast tick could matter is the walk (~15 min at 26% of the bar), and that's still sub-pixel per tick.
3. Stale doc comment on TripProgressTrack — TripActivityWidget.swift:784
Claims the marker "changes bell → walking person → train at each milestone" — it's a plain Circle(). The PR description correctly says "solid dot"; the comment describes a design that wasn't built.
4. Header comment typos — TripActivityWidget.swift:8
Duplicated article ("with a / a three-stage"), says "three-stage" for what the legend renders as four stages (Alarm / Walk / Train / Arrive), and the reflowed line break leaves a short orphan line.
5. Legend labels don't align with the milestone dots — TripActivityWidget.swift:806
Labels are laid out with Spacer(minLength:) (roughly even spacing), but the milestone dots sit at fixed 18% / 44% of the track. "Walk" and "Train" will not sit above the dots they name. Either position the labels off the same alarmShare / walkingShare constants, or drop the dots.
6. ForEach keyed on CGFloat values — TripActivityWidget.swift:838
ForEach([leaveX, boardingX], id: \.self) — if usableWidth ever collapses to 0 (container narrower than the 18pt marker), both resolve to markerSize / 2 and SwiftUI gets duplicate IDs. Key by index instead.
7. Formatting nit — TripActivityWidget.swift:50
StatusPill(model: model) / .padding(.trailing, 8) in the trailing region is now one expression split across two lines with the modifier at the same indent level, so it reads like two siblings. Fold onto one line.
Worth eyeballing before merge
The expanded Dynamic Island has a hard height ceiling (~160pt), and the bottom region gained a legend row plus an 18pt track inside a new VStack(spacing: 10). Compiling clean doesn't prove it isn't clipped — check the walkingState preview in the DI expanded canvas.
Also: the only CI check on this PR is the Cloudflare Workers build, which doesn't touch iOS. The xcodebuild run in the description is the sole build signal, and it's local.
What looks good
elapsedFractionclamps on both ends and guards non-positive durations; the finalmin(max(fraction, 0), 1)is belt-and-braces.- Removing the header bell on both surfaces rather than leaving it alongside the timeline — no duplicate signal.
- The
hasReminderJourneyfallback (reminder absent, or not before departure) degrades to a sane train-only track instead of a degenerate zero-width alarm stage. - Accessibility isn't an afterthought:
children: .ignoreplus a per-phase label and a percent value. - The new
walkingStatepreview covers the one stage that was previously unreachable in the canvas, and it's wired into all three preview blocks.
What changed
Why
The existing Live Activity showed countdowns and trip times but did not communicate overall trip progress. The timeline makes the transition from leave reminder to walking and then riding immediately scannable without requiring additional app wakes or frequent push updates.
Implementation
Progress is derived from the reminder, departure, and arrival timestamps already present in the ActivityKit content state. A SwiftUI
TimelineViewadvances the marker locally while the app is suspended.Validation
xcodebuild -workspace ios/App/App.xcworkspace -scheme SmartTripWidget -configuration Debug -sdk iphonesimulator -derivedDataPath /tmp/smart-trip-derived CODE_SIGNING_ALLOWED=NO buildgit diff --check