Unify image loading on shared OkHttp and defer artwork presentation during scroll - #256
Merged
Merged
Conversation
…work presentation during scroll
- Replace coil-network-ktor3 with coil-network-okhttp and register the
fetcher explicitly: new SiloOkHttp foundation shares one ConnectionPool
between the Ktor API clients and Coil, with per-client tuned dispatchers
so image bursts and API fan-outs never queue behind each other. Loader
config is unified in buildSiloImageLoader so phone and TV cannot drift.
- Extend Home's existing artwork-presentation deferral to all artwork
scroll surfaces via DeferImagePresentationWhileScrolling: freshly decoded
images hold their thumbhash until the scroll settles (memory-cache hits
still present instantly), now covering horizontal rows, grids, detail
feeds, and vertical-scroll columns.
- Fix stale-capture load-more triggers in CatalogGrid and SearchResults:
keyless remember { derivedStateOf } froze first-composition
hasMore/isLoading values, breaking pagination in some entry states.
- Declare ktor-client-core explicitly in android-shared/androidApp (it was
only reaching the compile classpath transitively via the old Coil
artifact); regenerate dependency locks and verification metadata and
drop stale TV lock pins.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (41)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughAndroid image loading now uses Coil’s OkHttp integration with shared process-level OkHttp resources. Image-loader creation is centralized. Lazy lists and grids defer image presentation while scrolling across Android screens. Loading-state derivation updates when pagination inputs change. ChangesImage loading and networking
Scroll-aware image presentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The PR unifies image networking, defers artwork presentation during scrolling, and fixes pagination state handling; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 17.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 34 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
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
1 participant
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
One OkHttp foundation for image + API traffic. Replaces
coil-network-ktor3withcoil-network-okhttpand registers the fetcher explicitly instead of relying on service-loader auto-registration (which silently built its own default HTTP client). The newSiloOkHttpobject shares a singleConnectionPoolbetween the Ktor API clients and Coil so artwork rides the same warm TLS connections, while each client keeps its own tunedDispatcherso poster bursts and API fan-outs never queue behind each other (and long-lived websockets can't pin image slots). Loader configuration is unified inbuildSiloImageLoader(android-shared), so the phone and TV apps can no longer drift. The Media3 player transport deliberately keeps its own pool — documented inSiloOkHttp's KDoc.App-wide artwork presentation deferral. Home already held freshly decoded artwork behind its thumbhash placeholder until the vertical scroll settles; this generalizes that into
DeferImagePresentationWhileScrolling(OR-combining nested scroll axes) and wires it across all artwork surfaces: horizontal rows, browse/search/personal/collection grids, detail feeds, calendar shelves, downloads, inbox, reading hub, and the player next-up overlay. Requests and decodes keep running during the gesture; only first presentation waits, and memory-cache hits still present instantly so scroll-back is unchanged.Pagination trigger fixes.
CatalogGridandSearchResultscomputed their load-more signal inside a keylessremember { derivedStateOf }, permanently capturing first-compositionhasMore/isLoadingvalues — pagination could never fire (or kept re-firing at list end) depending on entry state. Now keyed on the flags.Dependency hygiene.
android-sharedandandroidAppimported Ktor client classes that only reached the compile classpath transitively through the old Coil artifact — now declared explicitly. Locks and verification metadata regenerated; stale TV lockfile pins removed.Testing
./gradlew :androidApp:assembleDebug :androidTvApp:assembleDebugand the full./gradlew testsuite pass.dumpsys gfxinfoshow zero slow bitmap uploads during scroll.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes