fix(stop-detail): retain header across rotation to avoid refetch - #165
Closed
ai-tiro wants to merge 1 commit into
Closed
fix(stop-detail): retain header across rotation to avoid refetch#165ai-tiro wants to merge 1 commit into
ai-tiro wants to merge 1 commit into
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>
|
Debug APK: app-debug-c64a9cdb036ec32dbcd1b6ee8c960cc1c8ac7a39.apk (built from Requires GitHub login. Artifact expires after 3 days. |
This was referenced Jun 18, 2026
Collaborator
Author
|
Superseded by #179, which combined these commits and has merged to master. Closing as superseded. |
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
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.
Closes #161. Stacked on #164 (
fix/160-nearby-permission) — review/merge that first; this PR's base is that branch, notmaster.What I did
Two minimal guards in
StopDetailViewModel:loadHeader()is now idempotent. It returns early when the header is alreadyHeaderState.Loaded, so a ViewModel that re-entersinit(or is recreated by the per-NavKeyhiltViewModelkeying) never re-hitsstops/{id}/route_type/{rt}. The genuine first load still runs (header startsLoading), andretryHeader()resets the state toLoadingbefore calling, so retry-after-error still re-fetches.Result.Loadingpoll emission no longer wipes an already-Loadeddepartures list. On rotation,repeatOnLifecycle(RESUMED)re-subscribes the poll and the repository's first re-emission isLoading. Previously that flipped the list back to the loading skeleton — the visible reload flicker. Now the last-good rows stay on screen and the nextSuccesstick replaces them in place. The genuine first load (no rows yet) still shows the skeleton.Tests:
header is fetched exactly once on first load,restarting the departures poll does not refetch the header,loading emission after data keeps the last-good list on screen.loading emission flips departures back to Loadingtest (which asserted the now-removed flicker behaviour) intofirst loading emission shows the loading skeleton(first-load skeleton still works).What I discovered / diagnosis of WHY rotation refetched
The departures refetch is the clear, always-reproducible half: the poll is hot only while collected, and
repeatOnLifecycle(RESUMED)cancels-and-resubscribes on every Pause→Resume — and a rotation is a Pause→Stop→Resume. The fresh subscribe forces an upstream fetch and emitsLoading, which the ViewModel was mapping straight toDeparturesState.Loading, flashing the skeleton.The header refetch only occurs if the ViewModel is recreated across the config change (e.g. the assisted-inject-per-NavKey path with no SavedState retention, per the Nav3-alpha caveat in
CLAUDE.md). Rather than depend onSavedStateHandle— which the issue notes Nav3 alpha doesn't wire NavKey fields into — I made the fetch itself idempotent, which is correct whether the VM survives or is recreated with retained state, and is the smaller change.What I tried that didn't work / chose not to do
Considered retaining the header via
SavedStateHandle. Skipped it: Nav3 alpha doesn't populate the NavKey into saved state here, and the idempotent-loadHeaderguard covers the reported behaviour with far less surface area and no new serialization concerns.Justification / testing notes
Both changes are local to the ViewModel and preserve the documented contracts: the 30s polling cadence, pull-to-refresh (the spinner is its own feedback, list is untouched), background-pause, and the mid-poll error → next-success recovery path all behave as before. No one-way-door changes.
Test gap: there's no instrumented rotation test that actually rotates the device; the unit tests simulate the rotation effect (re-entering
startObservingand theLoadingre-emission) at the ViewModel seam. A faithfulandroidTestrotation test is heavier and was left out — the production fix is prioritised.:feature:stop-detail:compileDebugKotlinand:feature:stop-detail:testDebugUnitTestboth pass.🤖 Generated with Claude Code