Dispose Screens only when transition ends (#436)

* Dispose Screens only when transition ends * Adding the new parameter to all transitions apis * fix lint * Disposing solution from #427 to fix animation not ended dispose bug * fix doc workflow * fix lint * update docs * update api dump * fix lint * addressing reviews
adrielcafe · Jun 4, 2024 · db387ca · db387ca
1 parent 486497c
commit db387ca
Show file tree

Hide file tree

Showing 10 changed files with 258 additions and 23 deletions.
diff --git a/.github/workflows/docs-publish.yaml b/.github/workflows/docs-publish.yaml
@@ -1,8 +1,6 @@
 name: Docs Publish
 on:
   push:
-    branches: [main, mkdocs]
-  pull_request:
     branches: [main]
 
 jobs:

diff --git a/docs/transitions-api.md b/docs/transitions-api.md
@@ -13,6 +13,25 @@ setContent {
 }
 ```
 
+!!! error
+    There is a know bug using any Transition APIs can leaky ScreenModels or ViewModels, this happens because Voyager by default
+    dispose Screens in the next Composition tick after a `pop` or `replace` is called, but the transition only finish later, so
+    the ScreenModel or ViewModel is re created or cleared to early. For this purpose since Voyager `1.1.0-beta02` we have introduce
+    a new API to fix this issue. For more details on the issue see [#106](https://github.com/adrielcafe/voyager/issues/106).
+
+    ```kotlin
+    Navigator(
+        screen = ...,
+        disposeBehavior = NavigatorDisposeBehavior(disposeSteps = false),
+    ) { navigator ->
+        SlideTransition(
+            navigator = navigator,
+            ...
+            disposeScreenAfterTransitionEnd = true
+        )
+    }
+    ```
+
 !!! warning
     Have encounter `Screen was used multiple times` crash? Provide a `uniqueScreenKey` for your Screens
 

diff --git a/samples/android/src/main/java/cafe/adriel/voyager/sample/screenTransition/SampleScreens.kt b/samples/android/src/main/java/cafe/adriel/voyager/sample/screenTransition/SampleScreens.kt
@@ -9,12 +9,15 @@ import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.material.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.text.font.FontWeight
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
+import cafe.adriel.voyager.core.model.ScreenModel
+import cafe.adriel.voyager.core.model.rememberScreenModel
 import cafe.adriel.voyager.core.screen.Screen
 import cafe.adriel.voyager.core.screen.ScreenKey
 import cafe.adriel.voyager.transitions.ScreenTransition
@@ -27,6 +30,18 @@ private val colors = listOf(
     Color.Black
 )
 
+class BaseSampleScreenModel(
+    val index: Int
+) : ScreenModel {
+
+    init {
+        println("Init BaseSampleScreenModel $index")
+    }
+    override fun onDispose() {
+        println("Disposing BaseSampleScreenModel $index")
+    }
+}
+
 abstract class BaseSampleScreen(
     private val transitionType: String
 ) : Screen {
@@ -37,10 +52,16 @@ abstract class BaseSampleScreen(
 
     @Composable
     override fun Content() {
+        val model = rememberScreenModel {
+            BaseSampleScreenModel(index)
+        }
+        val color = remember {
+            colors.getOrNull(index % colors.size) ?: colors.random()
+        }
         Column(
             modifier = Modifier
                 .fillMaxSize()
-                .background(colors[index % colors.size].copy(alpha = 0.3f))
+                .background(color.copy(alpha = 0.3f))
                 .padding(40.dp),
             verticalArrangement = Arrangement.Center,
             horizontalAlignment = Alignment.CenterHorizontally

diff --git a/...oid/src/main/java/cafe/adriel/voyager/sample/screenTransition/ScreenTransitionActivity.kt b/...oid/src/main/java/cafe/adriel/voyager/sample/screenTransition/ScreenTransitionActivity.kt
@@ -5,6 +5,7 @@ import androidx.activity.ComponentActivity
 import androidx.activity.compose.setContent
 import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
@@ -16,7 +17,9 @@ import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.unit.dp
 import cafe.adriel.voyager.navigator.Navigator
+import cafe.adriel.voyager.navigator.NavigatorDisposeBehavior
 import cafe.adriel.voyager.transitions.ScreenTransition
+import kotlin.random.Random
 
 class ScreenTransitionActivity : ComponentActivity() {
 
@@ -31,40 +34,67 @@ class ScreenTransitionActivity : ComponentActivity() {
     @Composable
     fun Content() {
         Navigator(
-            screen = NoCustomAnimationSampleScreen(0)
+            screen = NoCustomAnimationSampleScreen(0),
+            disposeBehavior = NavigatorDisposeBehavior(disposeSteps = false)
         ) { navigator ->
             Box(modifier = Modifier.fillMaxSize()) {
                 ScreenTransition(
                     navigator = navigator,
-                    defaultTransition = SlideTransition()
+                    defaultTransition = SlideTransition(),
+                    disposeScreenAfterTransitionEnd = true
                 )
 
-                Row(
+                Column(
                     modifier = Modifier
                         .align(Alignment.BottomCenter)
                         .fillMaxWidth()
-                        .padding(40.dp),
-                    horizontalArrangement = Arrangement.spacedBy(20.dp)
                 ) {
-                    Button(
-                        onClick = { navigator.push(FadeAnimationSampleScreen(navigator.items.size)) },
-                        modifier = Modifier.weight(1f)
+                    Row(
+                        modifier = Modifier
+                            .fillMaxWidth()
+                            .padding(10.dp),
+                        horizontalArrangement = Arrangement.spacedBy(20.dp)
                     ) {
-                        Text(text = "Fade")
-                    }
+                        Button(
+                            onClick = { navigator.push(FadeAnimationSampleScreen(navigator.items.size)) },
+                            modifier = Modifier.weight(1f)
+                        ) {
+                            Text(text = "Fade")
+                        }
 
-                    Button(
-                        onClick = { navigator.push(NoCustomAnimationSampleScreen(navigator.items.size)) },
-                        modifier = Modifier.weight(1f)
-                    ) {
-                        Text(text = "Default")
-                    }
+                        Button(
+                            onClick = { navigator.push(NoCustomAnimationSampleScreen(navigator.items.size)) },
+                            modifier = Modifier.weight(1f)
+                        ) {
+                            Text(text = "Default")
+                        }
 
-                    Button(
-                        onClick = { navigator.pop() },
-                        modifier = Modifier.weight(1f)
+                        Button(
+                            onClick = { navigator.pop() },
+                            modifier = Modifier.weight(1f)
+                        ) {
+                            Text(text = "Pop")
+                        }
+                    }
+                    Row(
+                        modifier = Modifier
+                            .fillMaxWidth()
+                            .padding(10.dp),
+                        horizontalArrangement = Arrangement.spacedBy(20.dp)
                     ) {
-                        Text(text = "Pop")
+                        Button(
+                            onClick = { navigator.replace(NoCustomAnimationSampleScreen(Random.nextInt())) },
+                            modifier = Modifier.weight(1f)
+                        ) {
+                            Text(text = "Replace")
+                        }
+
+                        Button(
+                            onClick = { navigator.replaceAll(NoCustomAnimationSampleScreen(Random.nextInt())) },
+                            modifier = Modifier.weight(1f)
+                        ) {
+                            Text(text = "ReplaceAll")
+                        }
                     }
                 }
             }

diff --git a/voyager-transitions/api/android/voyager-transitions.api b/voyager-transitions/api/android/voyager-transitions.api
@@ -8,33 +8,43 @@ public final class cafe/adriel/voyager/transitions/ComposableSingletons$Crossfad
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$FadeTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$FadeTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
+	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-2$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$ScaleTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$ScaleTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
+	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-2$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$ScreenTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$ScreenTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
 	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public static field lambda-3 Lkotlin/jvm/functions/Function4;
+	public static field lambda-4 Lkotlin/jvm/functions/Function4;
+	public static field lambda-5 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
 	public final fun getLambda-2$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
 	public final fun getLambda-3$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-4$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-5$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$SlideTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$SlideTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
+	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-2$voyager_transitions_release ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/CrossfadeTransitionKt {

diff --git a/voyager-transitions/api/desktop/voyager-transitions.api b/voyager-transitions/api/desktop/voyager-transitions.api
@@ -8,33 +8,43 @@ public final class cafe/adriel/voyager/transitions/ComposableSingletons$Crossfad
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$FadeTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$FadeTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
+	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-2$voyager_transitions ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$ScaleTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$ScaleTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
+	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-2$voyager_transitions ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$ScreenTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$ScreenTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
 	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public static field lambda-3 Lkotlin/jvm/functions/Function4;
+	public static field lambda-4 Lkotlin/jvm/functions/Function4;
+	public static field lambda-5 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions ()Lkotlin/jvm/functions/Function4;
 	public final fun getLambda-2$voyager_transitions ()Lkotlin/jvm/functions/Function4;
 	public final fun getLambda-3$voyager_transitions ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-4$voyager_transitions ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-5$voyager_transitions ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/ComposableSingletons$SlideTransitionKt {
 	public static final field INSTANCE Lcafe/adriel/voyager/transitions/ComposableSingletons$SlideTransitionKt;
 	public static field lambda-1 Lkotlin/jvm/functions/Function4;
+	public static field lambda-2 Lkotlin/jvm/functions/Function4;
 	public fun <init> ()V
 	public final fun getLambda-1$voyager_transitions ()Lkotlin/jvm/functions/Function4;
+	public final fun getLambda-2$voyager_transitions ()Lkotlin/jvm/functions/Function4;
 }
 
 public final class cafe/adriel/voyager/transitions/CrossfadeTransitionKt {

diff --git a/voyager-transitions/src/commonMain/kotlin/cafe/adriel/voyager/transitions/FadeTransition.kt b/voyager-transitions/src/commonMain/kotlin/cafe/adriel/voyager/transitions/FadeTransition.kt
@@ -8,6 +8,7 @@ import androidx.compose.animation.fadeOut
 import androidx.compose.animation.togetherWith
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import cafe.adriel.voyager.core.annotation.ExperimentalVoyagerApi
 import cafe.adriel.voyager.navigator.Navigator
 
 @Composable
@@ -16,10 +17,29 @@ public fun FadeTransition(
     modifier: Modifier = Modifier,
     animationSpec: FiniteAnimationSpec<Float> = spring(stiffness = Spring.StiffnessMediumLow),
     content: ScreenTransitionContent = { it.Content() }
+) {
+    FadeTransition(
+        navigator = navigator,
+        modifier = modifier,
+        disposeScreenAfterTransitionEnd = false,
+        animationSpec = animationSpec,
+        content = content
+    )
+}
+
+@ExperimentalVoyagerApi
+@Composable
+public fun FadeTransition(
+    navigator: Navigator,
+    modifier: Modifier = Modifier,
+    animationSpec: FiniteAnimationSpec<Float> = spring(stiffness = Spring.StiffnessMediumLow),
+    disposeScreenAfterTransitionEnd: Boolean = false,
+    content: ScreenTransitionContent = { it.Content() }
 ) {
     ScreenTransition(
         navigator = navigator,
         modifier = modifier,
+        disposeScreenAfterTransitionEnd = disposeScreenAfterTransitionEnd,
         content = content,
         transition = { fadeIn(animationSpec = animationSpec) togetherWith fadeOut(animationSpec = animationSpec) }
     )

diff --git a/voyager-transitions/src/commonMain/kotlin/cafe/adriel/voyager/transitions/ScaleTransition.kt b/voyager-transitions/src/commonMain/kotlin/cafe/adriel/voyager/transitions/ScaleTransition.kt
@@ -8,6 +8,7 @@ import androidx.compose.animation.scaleOut
 import androidx.compose.animation.togetherWith
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import cafe.adriel.voyager.core.annotation.ExperimentalVoyagerApi
 import cafe.adriel.voyager.core.stack.StackEvent
 import cafe.adriel.voyager.navigator.Navigator
 
@@ -20,10 +21,29 @@ public fun ScaleTransition(
     modifier: Modifier = Modifier,
     animationSpec: FiniteAnimationSpec<Float> = spring(stiffness = Spring.StiffnessMediumLow),
     content: ScreenTransitionContent = { it.Content() }
+) {
+    ScaleTransition(
+        navigator = navigator,
+        modifier = modifier,
+        animationSpec = animationSpec,
+        disposeScreenAfterTransitionEnd = false,
+        content = content
+    )
+}
+
+@ExperimentalVoyagerApi
+@Composable
+public fun ScaleTransition(
+    navigator: Navigator,
+    modifier: Modifier = Modifier,
+    animationSpec: FiniteAnimationSpec<Float> = spring(stiffness = Spring.StiffnessMediumLow),
+    disposeScreenAfterTransitionEnd: Boolean = false,
+    content: ScreenTransitionContent = { it.Content() }
 ) {
     ScreenTransition(
         navigator = navigator,
         modifier = modifier,
+        disposeScreenAfterTransitionEnd = disposeScreenAfterTransitionEnd,
         content = content,
         transition = {
             val (initialScale, targetScale) = when (navigator.lastEvent) {