chore(mobile): combine stop-detail + nearby fixes for QA (#165 #166 #174 #176) - #179
Merged
Conversation
Rotating the stop-detail screen fired two avoidable requests plus a reload
flicker: the header re-fetched `stops/{id}/route_type/{rt}` and the departures
poll restarted, flashing the loading skeleton.
Two minimal, idiomatic guards:
- `loadHeader()` is now idempotent — it returns early when the header is
already `Loaded`, so a recreated/re-entered ViewModel never re-hits the stop
header endpoint. The genuine first load still runs (header is `Loading`), and
`retryHeader()` resets to `Loading` first so retry-after-error still works.
- A `Result.Loading` emission no longer wipes an already-`Loaded` departures
list. On rotation `repeatOnLifecycle(RESUMED)` re-subscribes the poll and the
repository's first re-emission is `Loading`; we keep the last-good rows on
screen and let the next `Success` tick replace them in place. The genuine
first load (no rows yet) still shows the skeleton.
The 30s cadence, pull-to-refresh, and background-pause behaviour are unchanged.
Co-Authored-By: ai-tiro <ai-tiro@jfx.ac>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bringing the app to the foreground on a stop-detail screen intermittently fired TWO departures requests ~66 ms apart instead of one (issue #162). Root cause: startObserving() used a plain observeJob?.cancel() and then immediately launched the new collection. cancel() only *requests* cooperative cancellation, so the previous observe coroutine could still be parked inside the suspending fetch and fire a departures request at the same moment the fresh collection subscribed — an intermittent race with a small window. Fix: the new job now cancelAndJoin()s the prior job before subscribing to observeDepartures, so the old subscription is fully torn down first and exactly one fetch starts per resume. The prior Job is captured before reassigning observeJob, so the join never targets the coroutine it runs in (no self-join deadlock). The genuine first start has no prior job and subscribes immediately; the 30s polling cadence, background-pause behaviour, and the previous stacked fix's Loading-retention / idempotent-header contracts are all preserved. Adds a single-flight unit test asserting a pause->resume cycle leaves exactly one live collector and applies a single tick once. Co-Authored-By: ai-tiro <ai-tiro@jfx.ac> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tappable Different modes at one station share identical PTV coordinates — the Richmond train stop and the V/Line "Richmond Railway Station" are both stop_id 1162 at the same lat/lng. Rendered as-is their circles stacked on one pixel, so only the top stop was visible and the tap hit-test (nearest by true coordinate) could never resolve to the other. spreadColocatedStops() buckets stops by coordinate and fans any shared-point group evenly around a small (~20 m) circle, ordered by route type then id so a stop's nudged position is deterministic. It's the single source of display positions for both rendering and the tap hit-test, so a fanned-out dot is selectable exactly where it's drawn. Lone stops keep their exact coordinate. Chose the pixel-offset approach over re-enabling MapLibre clustering, which would reintroduce the #124 zoom-extreme bug it was removed to fix. Fixes #172 Co-Authored-By: ai-tiro <ai-tiro@jfx.ac> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_id)
PTV's /stops/{id}/route_type/{type} endpoint ignores the route_type path
param for the `routes` array and returns every route serving the physical
stop_id. At co-located stops sharing a stop_id (Richmond, stop_id 1162: the
metro platforms and the V/Line platform are one stop) this mixed modes, so
the Nearby sheet's Routes chips on the V/Line pin showed metro train lines
(Alamein, Belgrave, ...). Departures are fetched per (stopId, routeType) and
were already correct, which is why only the Routes chips were wrong.
Filter servingRoutes to the requested route_type in StopResponseDto.toDomain
so every getStopDetail consumer gets routes consistent with the mode it asked
for. Both current consumers (Nearby sheet, stop-detail departure-grouping
lookup) want the mode-consistent list.
Co-Authored-By: ai-tiro <ai-tiro@jfx.ac>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Uploaded via |
|
Debug APK: app-debug-bac23b725b91f3de8790bb613ea615eadceb15dd.apk (built from Requires GitHub login. Artifact expires after 3 days. |
itsjfx
marked this pull request as ready for review
June 21, 2026 10:37
This was referenced Jun 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Combines four open mobile PRs into one branch off
masterfor a single QA pass + manual review before merge:131cbd110c46a6b1c183dbac23b7stop_id)Why
PRs #165 and #166 were stacked on
fix/160-nearby-permission, which was already merged tomaster(via #164) — so the stack's base branch is gone and the PRs needed re-basing ontomasteranyway. Rather than re-target each stacked/independent PR individually, this rolls the mobile-only work into one branch so it can be QA'd together and manually verified before landing.Explicitly excluded
perf(proxy): trim routes from stops/location response— backend payload change, out of scope for this mobile batch.feat(release): signed release APK pipeline— CI/CD release change, out of scope.QA (AOSP emulator, real PTV data via proxy)
Unit tests green:
:feature:stop-detail:testDebugUnitTest,:feature:nearby:testDebugUnitTest.:app:assembleDebugclean.Notes / testing concerns
Co-Authored-By: ai-tiro ai-tiro@jfx.ac