Uh oh!
There was an error while loading. Please reload this page.
fix(android): give ComposeView its own Lifecycle to fix #1103 and #1104 - #1105
Open
GaelCO wants to merge 3 commits into
Open
fix(android): give ComposeView its own Lifecycle to fix #1103 and #1104#1105GaelCO wants to merge 3 commits into
GaelCO wants to merge 3 commits into
Conversation
…and callstack#1104 react-native-screens fully removes and re-adds a covered screen's Fragment instead of just hiding it, destroying its view-tree Lifecycle. Compose's default disposal strategy keys off that ambient Lifecycle, so the pager's ComposeView was torn down and rebuilt on every cover/reveal cycle, causing a blank pager (callstack#1103) and loss of page state such as FlatList scroll position (callstack#1104). Giving the ComposeView its own self-owned Lifecycle lets the composition survive the cycle untouched.
There was a problem hiding this comment.
Pull request overview
This PR fixes Android regressions in the Compose-based pager when screens are covered/revealed (notably with react-native-screens) by decoupling the ComposeView’s composition disposal from the ambient Fragment view-tree Lifecycle, keeping the composition (and hosted native page views) alive across cover/reveal cycles.
Changes:
- Add explicit cleanup in
onDropViewInstanceto destroy a self-ownedLifecycleand dispose the Compose composition. - Rework
ComposePagerViewto create/bind aComposeViewwith a dedicatedLifecycleOwnerandDisposeOnLifecycleDestroyedstrategy. - Add an AndroidX Lifecycle runtime dependency required for view-tree lifecycle owner APIs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| android/src/main/java/com/reactnativepagerview/PagerViewViewManager.kt | Disposes pager resources when the RN view instance is dropped. |
| android/src/main/java/com/reactnativepagerview/ComposePagerView.kt | Introduces a self-owned lifecycle for the ComposeView and adjusts attach/detach behavior to preserve composition across cover/reveal. |
| android/build.gradle | Adds AndroidX Lifecycle runtime dependency for the new lifecycle owner usage. |
Suppressed comments (1)
android/src/main/java/com/reactnativepagerview/ComposePagerView.kt:652
- ComposeViewLifecycleOwner is hard-coded to start in Lifecycle.State.RESUMED and stay there until DESTROYED. That can keep lifecycle-aware work (e.g., coroutines/flows started with lifecycle-aware APIs) running even while the view is detached/covered, which is likely more work than necessary. Consider starting at CREATED and adding explicit attach/detach state transitions so the composition can pause while offscreen without being disposed.
// A Lifecycle that isn't derived from the ambient Fragment/Activity: it
// starts RESUMED and only moves to DESTROYED when destroy() is called
// explicitly, so it survives being covered/revealed by react-native-screens'
// Fragment remove+re-add (see the composeLifecycleOwner field comment on
// ComposePagerView above).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+116
to
+120
| override fun onDetachedFromWindow() { | ||
| updateSameOrientationAncestorsGestureState(false) | ||
| if (composeView.parent === this) { | ||
| super.removeView(composeView) | ||
| didSetContent = false | ||
| } | ||
| // composeView is intentionally left attached and alive here: its | ||
| // Lifecycle is self-owned (see composeLifecycleOwner) and only reaches | ||
| // DESTROYED in dispose(), so there is nothing to tear down on a mere |
Comment on lines
+249
to
+251
| // Needed for ComposeViewLifecycleOwner (see ComposePagerView.kt), which | ||
| // gives the ComposeView a self-owned Lifecycle instead of the ambient one. | ||
| implementation "androidx.lifecycle:lifecycle-runtime-ktx:2.6.1" |
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 freeto 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.
Summary
ComposeViewwas torn down and rebuilt on every cover/reveal cycle: this caused a blank pager after covering the screen ([Android] Tab content stays blank after navigating away and back over a PagerView (regression 8.0.5 → 9.0.2) #1103) and reset each page's native view state, e.g. FlatList scroll position ([Android] PagerView 9 remounts/recreates page after navigating back to Material Top Tabs #1104).ComposeViewits own self-ownedLifecycle(only destroyed for real inonDropViewInstance) lets the composition and each page's host survive the cover/reveal cycle untouched.Fixes#1103
Fixes#1104
Test plan
react-navigationmaterial-top-tabs stack that covering/revealing a screen containing the pager no longer blanks it out.