diff --git a/.maestro/tests/material_top_bar_example.yaml b/.maestro/tests/material_top_bar_example.yaml index fd2fca7c..2f441a6a 100644 --- a/.maestro/tests/material_top_bar_example.yaml +++ b/.maestro/tests/material_top_bar_example.yaml @@ -18,6 +18,34 @@ appId: com.pagerviewexample - assertVisible: text: 'Tab1' +- tapOn: + id: 'material-top-bar-scroll-list-button' + +- extendedWaitUntil: + visible: + id: 'material-top-bar-list-item-30' + timeout: 5000 + +- tapOn: + id: 'material-top-bar-open-detail-button' + +- extendedWaitUntil: + visible: + id: 'material-top-bar-detail-screen' + timeout: 5000 + +- pressKey: Back + +- extendedWaitUntil: + visible: + id: 'material-top-bar-tab-1' + timeout: 5000 + +# The existing ComposeView and AndroidView host must survive the native-stack +# cover/reveal cycle, including the FlatList's native scroll position. +- assertVisible: + id: 'material-top-bar-list-item-30' + - tapOn: 'Tab2' - extendedWaitUntil: diff --git a/android/build.gradle b/android/build.gradle index 16e92aef..96ca1c5f 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -238,6 +238,7 @@ repositories { def kotlin_version = getExtOrDefault('kotlinVersion') def compose_version = getExtOrDefault('composeVersion') +def lifecycle_version = getExtOrDefault('lifecycleVersion') dependencies { //noinspection GradleDynamicVersion @@ -246,6 +247,11 @@ dependencies { implementation "androidx.compose.foundation:foundation:$compose_version" implementation "androidx.compose.runtime:runtime:$compose_version" implementation "androidx.compose.ui:ui:$compose_version" + // 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:$lifecycle_version" + testImplementation "androidx.arch.core:core-testing:2.2.0" + testImplementation "junit:junit:4.13.2" } if (isNewArchitectureEnabled()) { diff --git a/android/gradle.properties b/android/gradle.properties index 3020b8b8..7b041548 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,6 @@ PagerView_kotlinVersion=2.0.21 PagerView_composeVersion=1.7.8 +PagerView_lifecycleVersion=2.6.1 PagerView_minSdkVersion=24 PagerView_targetSdkVersion=35 PagerView_compileSdkVersion=35 diff --git a/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt b/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt index 8695b93c..5eaa6069 100644 --- a/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt +++ b/android/src/main/java/com/reactnativepagerview/ComposePagerView.kt @@ -24,6 +24,12 @@ import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.ViewCompositionStrategy import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import androidx.lifecycle.findViewTreeLifecycleOwner +import androidx.lifecycle.setViewTreeLifecycleOwner import com.facebook.react.bridge.ReactContext import com.facebook.react.uimanager.UIManagerHelper import com.reactnativepagerview.event.PageScrollEvent @@ -37,7 +43,19 @@ import kotlin.math.sign @OptIn(ExperimentalFoundationApi::class) class ComposePagerView(context: Context) : FrameLayout(context) { private val reactContext = context as ReactContext - private val composeView = ComposeView(context) + // react-native-screens fully removes and re-adds this screen's Fragment + // (rather than merely hiding it) while it's covered by another screen + // (see #1103), destroying that Fragment's view-tree Lifecycle. Compose's + // default composition-disposal strategies key off that ambient Lifecycle, + // so a ComposeView left to use them gets torn down on every cover/reveal + // cycle - and rebuilding the composition from scratch each time also + // discarded every page's native view host, resetting their state (e.g. a + // FlatList's scroll position, see #1104). Giving the ComposeView its own + // Lifecycle - one we control instead of the ambient, repeatedly-destroyed + // one - lets the composition (and each page's host) survive the cycle + // untouched. It only reaches DESTROYED for real in dispose() below. + private val composeLifecycleOwner = ComposeViewLifecycleOwner() + private var composeView: ComposeView? = null private val pages = mutableStateListOf() private val scrollEnabledState = mutableStateOf(true) private val orientationState = mutableStateOf(Orientation.Horizontal) @@ -57,45 +75,68 @@ class ComposePagerView(context: Context) : FrameLayout(context) { private val scrollCommandState = mutableStateOf(null) private var lastEmittedScrollState: String? = null private var lastEmittedPageSelected: Int? = null - private var didSetContent = false init { id = View.generateViewId() layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) isSaveEnabled = false - - composeView.layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) - composeView.isSaveEnabled = false - composeView.setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnDetachedFromWindow) applyOverScrollMode() touchSlop = ViewConfiguration.get(context).scaledTouchSlop } + private fun createComposeView(): ComposeView { + return ComposeView(context).apply { + layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) + isSaveEnabled = false + layoutDirection = androidLayoutDirection() + overScrollMode = androidOverScrollMode() + // Bind our own Lifecycle before the composition is created, so Compose + // resolves it instead of walking up to the ambient (and repeatedly + // destroyed) Fragment view-tree Lifecycle - see the field comment above. + setViewTreeLifecycleOwner(composeLifecycleOwner) + setViewCompositionStrategy( + ViewCompositionStrategy.DisposeOnLifecycleDestroyed(composeLifecycleOwner) + ) + } + } + override fun onAttachedToWindow() { super.onAttachedToWindow() - if (composeView.parent == null) { - super.addView(composeView) - post { - measureAndLayoutComposeView() - } - } - if (!didSetContent) { - didSetContent = true - composeView.setContent { + // Read the ambient owner from our parent before installing the stable + // owner on the ComposeView below. This keeps the composition alive across + // Fragment view-tree replacement while still following pause/stop events + // from the currently active host. + composeLifecycleOwner.attach(findViewTreeLifecycleOwner()?.lifecycle) + if (composeView == null) { + val view = createComposeView() + composeView = view + super.addView(view) + view.setContent { PagerContent() } } + post { + measureAndLayoutComposeView() + } } 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(). We pause it to CREATED instead, so lifecycle- + // aware content within pages (video players, nested Compose effects, + // screen-view tracking) stops doing work while covered - this doesn't + // dispose the composition, since DisposeOnLifecycleDestroyed only acts + // on ON_DESTROY. + composeLifecycleOwner.detach() super.onDetachedFromWindow() } + fun dispose() { + composeLifecycleOwner.destroy() + } + override fun dispatchTouchEvent(event: MotionEvent): Boolean { when (event.actionMasked) { MotionEvent.ACTION_DOWN -> { @@ -149,7 +190,7 @@ class ComposePagerView(context: Context) : FrameLayout(context) { val width = width.takeIf { it > 0 } ?: measuredWidth val height = height.takeIf { it > 0 } ?: measuredHeight if (measureComposeView(width, height)) { - composeView.layout(0, 0, width, height) + composeView?.layout(0, 0, width, height) } } @@ -157,11 +198,12 @@ class ComposePagerView(context: Context) : FrameLayout(context) { width: Int = measuredWidth, height: Int = measuredHeight ): Boolean { - if (composeView.parent !== this || width <= 0 || height <= 0) { + val view = composeView + if (view == null || view.parent !== this || width <= 0 || height <= 0) { return false } - composeView.measure( + view.measure( MeasureSpec.makeMeasureSpec(width, MeasureSpec.EXACTLY), MeasureSpec.makeMeasureSpec(height, MeasureSpec.EXACTLY) ) @@ -238,13 +280,16 @@ class ComposePagerView(context: Context) : FrameLayout(context) { fun setLayoutDirection(value: String) { layoutDirectionState.value = if (value == "rtl") LayoutDirection.Rtl else LayoutDirection.Ltr - val androidLayoutDirection = if (layoutDirectionState.value == LayoutDirection.Rtl) { + layoutDirection = androidLayoutDirection() + composeView?.layoutDirection = androidLayoutDirection() + } + + private fun androidLayoutDirection(): Int { + return if (layoutDirectionState.value == LayoutDirection.Rtl) { View.LAYOUT_DIRECTION_RTL } else { View.LAYOUT_DIRECTION_LTR } - layoutDirection = androidLayoutDirection - composeView.layoutDirection = androidLayoutDirection } fun setOffscreenPageLimit(value: Int) { @@ -265,13 +310,17 @@ class ComposePagerView(context: Context) : FrameLayout(context) { } private fun applyOverScrollMode() { - val androidOverScrollMode = when (overScrollModeState.value) { + val androidOverScrollMode = androidOverScrollMode() + overScrollMode = androidOverScrollMode + composeView?.overScrollMode = androidOverScrollMode + } + + private fun androidOverScrollMode(): Int { + return when (overScrollModeState.value) { OverScrollMode.Never -> View.OVER_SCROLL_NEVER OverScrollMode.Always -> View.OVER_SCROLL_ALWAYS OverScrollMode.Auto -> View.OVER_SCROLL_IF_CONTENT_SCROLLS } - overScrollMode = androidOverScrollMode - composeView.overScrollMode = androidOverScrollMode } private fun setSameOrientationChildGestureActive(value: Boolean) { @@ -606,3 +655,89 @@ class ComposePagerView(context: Context) : FrameLayout(context) { } } } + +// A stable Lifecycle that follows the currently attached host through +// CREATED/STARTED/RESUMED, but only moves to DESTROYED when destroy() is +// called explicitly. This lets it survive react-native-screens replacing the +// ambient Fragment view-tree owner (see the field comment above). +internal class ComposeViewLifecycleOwner : LifecycleOwner, LifecycleEventObserver { + private val registry = LifecycleRegistry(this).apply { + currentState = Lifecycle.State.CREATED + } + private var hostLifecycle: Lifecycle? = null + private var isAttached = false + private var isDestroyed = false + + override val lifecycle: Lifecycle + get() = registry + + fun attach(lifecycle: Lifecycle?) { + if (isDestroyed) { + return + } + + isAttached = true + setHostLifecycle(lifecycle) + updateState() + } + + fun detach() { + if (isDestroyed) { + return + } + + isAttached = false + setHostLifecycle(null) + updateState() + } + + fun destroy() { + if (isDestroyed) { + return + } + + isDestroyed = true + isAttached = false + setHostLifecycle(null) + registry.currentState = Lifecycle.State.DESTROYED + } + + override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { + if (event == Lifecycle.Event.ON_DESTROY && source.lifecycle === hostLifecycle) { + setHostLifecycle(null) + } + updateState() + } + + private fun setHostLifecycle(lifecycle: Lifecycle?) { + if (hostLifecycle === lifecycle) { + return + } + + hostLifecycle?.removeObserver(this) + hostLifecycle = lifecycle + lifecycle?.addObserver(this) + } + + private fun updateState() { + if (isDestroyed) { + return + } + + registry.currentState = if (!isAttached) { + Lifecycle.State.CREATED + } else { + when (hostLifecycle?.currentState) { + Lifecycle.State.RESUMED -> Lifecycle.State.RESUMED + Lifecycle.State.STARTED -> Lifecycle.State.STARTED + // INITIALIZED is expected briefly when react-native-screens installs + // a replacement Fragment view tree. DESTROYED belongs to the old + // tree. Neither should destroy or block the retained composition. + Lifecycle.State.INITIALIZED, + Lifecycle.State.CREATED, + Lifecycle.State.DESTROYED, + null -> Lifecycle.State.CREATED + } + } + } +} diff --git a/android/src/main/java/com/reactnativepagerview/PagerViewViewManager.kt b/android/src/main/java/com/reactnativepagerview/PagerViewViewManager.kt index 490e3128..d41052a7 100644 --- a/android/src/main/java/com/reactnativepagerview/PagerViewViewManager.kt +++ b/android/src/main/java/com/reactnativepagerview/PagerViewViewManager.kt @@ -69,6 +69,11 @@ class PagerViewViewManager : ViewGroupManager(), RNCViewPagerM return true } + override fun onDropViewInstance(view: ComposePagerView) { + view.dispose() + super.onDropViewInstance(view) + } + @ReactProp(name = "scrollEnabled", defaultBoolean = true) override fun setScrollEnabled(view: ComposePagerView?, value: Boolean) { if (view != null) { diff --git a/android/src/test/java/com/reactnativepagerview/ComposeViewLifecycleOwnerTest.kt b/android/src/test/java/com/reactnativepagerview/ComposeViewLifecycleOwnerTest.kt new file mode 100644 index 00000000..932410a1 --- /dev/null +++ b/android/src/test/java/com/reactnativepagerview/ComposeViewLifecycleOwnerTest.kt @@ -0,0 +1,86 @@ +package com.reactnativepagerview + +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import org.junit.Assert.assertEquals +import org.junit.Rule +import org.junit.Test + +class ComposeViewLifecycleOwnerTest { + @get:Rule + val instantTaskExecutorRule = InstantTaskExecutorRule() + + @Test + fun `follows the attached host lifecycle without inheriting destruction`() { + val owner = ComposeViewLifecycleOwner() + val host = TestLifecycleOwner() + + host.moveTo(Lifecycle.State.RESUMED) + owner.attach(host.lifecycle) + assertEquals(Lifecycle.State.RESUMED, owner.lifecycle.currentState) + + host.moveTo(Lifecycle.State.STARTED) + assertEquals(Lifecycle.State.STARTED, owner.lifecycle.currentState) + + host.moveTo(Lifecycle.State.CREATED) + assertEquals(Lifecycle.State.CREATED, owner.lifecycle.currentState) + + host.moveTo(Lifecycle.State.RESUMED) + assertEquals(Lifecycle.State.RESUMED, owner.lifecycle.currentState) + + host.moveTo(Lifecycle.State.DESTROYED) + assertEquals(Lifecycle.State.CREATED, owner.lifecycle.currentState) + } + + @Test + fun `survives host replacement and follows the replacement owner`() { + val owner = ComposeViewLifecycleOwner() + val oldHost = TestLifecycleOwner() + val replacementHost = TestLifecycleOwner() + + oldHost.moveTo(Lifecycle.State.RESUMED) + owner.attach(oldHost.lifecycle) + oldHost.moveTo(Lifecycle.State.DESTROYED) + + owner.detach() + owner.attach(replacementHost.lifecycle) + assertEquals(Lifecycle.State.CREATED, owner.lifecycle.currentState) + + replacementHost.moveTo(Lifecycle.State.STARTED) + assertEquals(Lifecycle.State.STARTED, owner.lifecycle.currentState) + + replacementHost.moveTo(Lifecycle.State.RESUMED) + assertEquals(Lifecycle.State.RESUMED, owner.lifecycle.currentState) + } + + @Test + fun `detaches from the host and only destroys on explicit disposal`() { + val owner = ComposeViewLifecycleOwner() + val host = TestLifecycleOwner() + + host.moveTo(Lifecycle.State.RESUMED) + owner.attach(host.lifecycle) + owner.detach() + assertEquals(Lifecycle.State.CREATED, owner.lifecycle.currentState) + + host.moveTo(Lifecycle.State.CREATED) + host.moveTo(Lifecycle.State.RESUMED) + assertEquals(Lifecycle.State.CREATED, owner.lifecycle.currentState) + + owner.destroy() + assertEquals(Lifecycle.State.DESTROYED, owner.lifecycle.currentState) + } + + private class TestLifecycleOwner : LifecycleOwner { + private val registry = LifecycleRegistry(this) + + override val lifecycle: Lifecycle + get() = registry + + fun moveTo(state: Lifecycle.State) { + registry.currentState = state + } + } +} diff --git a/example/src/MaterialTopTabExample.tsx b/example/src/MaterialTopTabExample.tsx index 20318a07..d6cb7b25 100644 --- a/example/src/MaterialTopTabExample.tsx +++ b/example/src/MaterialTopTabExample.tsx @@ -2,15 +2,37 @@ import React, { useState } from 'react'; import { createNativeStackNavigator } from '@react-navigation/native-stack'; import { createMaterialTopTabNavigator } from '@react-navigation/material-top-tabs'; -import { View, Text, Button } from 'react-native'; +import { View, Text, Button, FlatList, StyleSheet } from 'react-native'; + +const listItems = Array.from({ length: 50 }, (_, index) => `List item ${index}`); + +function Tab1(props: { onOpenDetail: () => void }) { + const listRef = React.useRef>(null); -function Tab1() { return ( - + Tab 1 +