Fixed stack animation not updating to new child component instance wh…

…en active configuration is unchanged
arkivanov · Oct 12, 2024 · 82aee7d · 82aee7d
1 parent 17852c5
commit 82aee7d
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 21 deletions.
diff --git a/...ivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt b/...ivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt
@@ -49,22 +49,31 @@ internal class DefaultStackAnimation<C : Any, T : Any>(
     ) {
         var currentStack by remember { mutableStateOf(stack) }
         var items by remember { mutableStateOf(getAnimationItems(newStack = currentStack)) }
+        var nextItems: Map<Any, AnimationItem<C, T>>? by remember { mutableStateOf(null) }
         val stackKeys = remember(stack) { stack.items.map { it.key } }
         val currentStackKeys = remember(currentStack) { currentStack.items.map { it.key } }
 
-        if (stackKeys != currentStackKeys) {
+        if (stack != currentStack) {
             val oldStack = currentStack
             currentStack = stack
 
             val updateItems =
                 when {
-                    stack.active.key == oldStack.active.key -> items.keys.singleOrNull() != stack.active.key
+                    stack.active.key == oldStack.active.key ->
+                        (items.keys.singleOrNull() != stack.active.key) ||
+                            (items.values.singleOrNull()?.child?.instance != stack.active.instance)
+
                     items.size == 1 -> items.keys.single() != stack.active.key
                     else -> items.keys.toList() != stackKeys
                 }
 
             if (updateItems) {
-                items = getAnimationItems(newStack = currentStack, oldStack = oldStack)
+                val newItems = getAnimationItems(newStack = currentStack, oldStack = oldStack)
+                if (items.size == 1) {
+                    items = newItems
+                } else {
+                    nextItems = newItems
+                }
             }
         }
 
@@ -82,12 +91,21 @@ internal class DefaultStackAnimation<C : Any, T : Any>(
                         },
                         content = content,
                     )
+
+                    if (item.direction.isExit) {
+                        DisposableEffect(Unit) {
+                            onDispose {
+                                nextItems?.also { items = it }
+                                nextItems = null
+                            }
+                        }
+                    }
                 }
             }
 
             // A workaround until https://issuetracker.google.com/issues/214231672.
             // Normally only the exiting child should be disabled.
-            if (disableInputDuringAnimation && (items.size > 1)) {
+            if (disableInputDuringAnimation && ((items.size > 1) || (nextItems != null))) {
                 Overlay(modifier = Modifier.matchParentSize())
             }
         }
@@ -129,7 +147,7 @@ internal class DefaultStackAnimation<C : Any, T : Any>(
 
     private fun getAnimationItems(newStack: ChildStack<C, T>, oldStack: ChildStack<C, T>? = null): Map<Any, AnimationItem<C, T>> =
         when {
-            oldStack == null ->
+            (oldStack == null) || (newStack.active.key == oldStack.active.key) ->
                 keyedItemsOf(
                     AnimationItem(
                         child = newStack.active,

diff --git a/...st/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt b/...st/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt
@@ -1,5 +1,6 @@
 package com.arkivanov.decompose.extensions.compose.experimental.stack
 
+import androidx.compose.animation.AnimatedVisibilityScope
 import androidx.compose.foundation.clickable
 import androidx.compose.foundation.text.BasicText
 import androidx.compose.runtime.Composable
@@ -29,12 +30,13 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.Parameterized
+import kotlin.test.assertSame
 
 @OptIn(ExperimentalDecomposeApi::class)
 @Suppress("TestFunctionName")
 @RunWith(Parameterized::class)
 class ChildStackTest(
-    private val animation: StackAnimation<Config, Config>?,
+    private val animation: StackAnimation<Config, Any>?,
 ) {
 
     @get:Rule
@@ -92,6 +94,18 @@ class ChildStackTest(
         composeRule.onNodeWithText(text = "ChildA1", substring = true).assertExists()
     }
 
+    @Test
+    fun GIVEN_child_displayed_WHEN_new_child_instance_with_the_same_key_THEN_new_child_instance_displayed() {
+        val state = mutableStateOf(routerState(Child.Created(configuration = Config.A, instance = Any(), key = "A")))
+        var lastInstance: Any? = null
+        setContent(state) { lastInstance = it.instance }
+
+        val instance2 = Any()
+        state.setValueOnIdle(routerState(Child.Created(configuration = Config.A, instance = instance2, key = "A")))
+
+        assertSame(instance2, lastInstance)
+    }
+
     @Test
     fun GIVEN_child_B_displayed_and_child_A_in_back_stack_WHEN_pop_child_B_THEN_state_restored_for_child_A() {
         val state = mutableStateOf(routerState(Config.A))
@@ -186,11 +200,14 @@ class ChildStackTest(
         composeRule.onNodeWithText(text = "ChildA2", substring = true).assertDoesNotExist()
     }
 
-    private fun setContent(state: State<ChildStack<Config, Config>>) {
+    private fun setContent(
+        state: State<ChildStack<Config, Any>>,
+        content: @Composable AnimatedVisibilityScope.(Child.Created<Config, Any>) -> Unit = {
+            Child(name = it.key.toString())
+        },
+    ) {
         composeRule.setContent {
-            ChildStack(stack = state.value, animation = animation) { child ->
-                Child(name = child.key.toString())
-            }
+            ChildStack(stack = state.value, animation = animation, content = content)
         }
 
         composeRule.runOnIdle {}
@@ -211,6 +228,12 @@ class ChildStackTest(
             backStack = stack.dropLast(1).map { it.toChild() },
         )
 
+    private fun routerState(vararg stack: Child.Created<Config, Any>): ChildStack<Config, Any> =
+        ChildStack(
+            active = stack.last(),
+            backStack = stack.dropLast(1),
+        )
+
     private fun Pair<Any, Config>.toChild(): Child.Created<Config, Config> =
         Child.Created(configuration = second, instance = second, key = first)
 

diff --git a/...tlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt b/...tlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt
@@ -2,6 +2,7 @@ package com.arkivanov.decompose.extensions.compose.stack.animation
 
 import androidx.compose.foundation.layout.Box
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.key
 import androidx.compose.runtime.mutableStateOf
@@ -29,11 +30,18 @@ internal abstract class AbstractStackAnimation<C : Any, T : Any>(
     override operator fun invoke(stack: ChildStack<C, T>, modifier: Modifier, content: @Composable (child: Child.Created<C, T>) -> Unit) {
         var currentStack by remember { mutableStateOf(stack) }
         var items by remember { mutableStateOf(getAnimationItems(newStack = currentStack, oldStack = null)) }
+        var nextItems: Map<Any, AnimationItem<C, T>>? by remember { mutableStateOf(null) }
 
-        if (stack.active.key != currentStack.active.key) {
+        if (stack.active.key != currentStack.active.key || stack.active.instance != currentStack.active.instance) {
             val oldStack = currentStack
             currentStack = stack
-            items = getAnimationItems(newStack = currentStack, oldStack = oldStack)
+
+            val newItems = getAnimationItems(newStack = currentStack, oldStack = oldStack)
+            if (items.size == 1) {
+                items = newItems
+            } else {
+                nextItems = newItems
+            }
         }
 
         Box(modifier = modifier) {
@@ -50,20 +58,29 @@ internal abstract class AbstractStackAnimation<C : Any, T : Any>(
                         },
                         content = content,
                     )
+
+                    if (item.direction.isExit) {
+                        DisposableEffect(Unit) {
+                            onDispose {
+                                nextItems?.also { items = it }
+                                nextItems = null
+                            }
+                        }
+                    }
                 }
             }
 
             // A workaround until https://issuetracker.google.com/issues/214231672.
             // Normally only the exiting child should be disabled.
-            if (disableInputDuringAnimation && (items.size > 1)) {
+            if (disableInputDuringAnimation && ((items.size > 1) || (nextItems != null))) {
                 InputConsumingOverlay(modifier = Modifier.matchParentSize())
             }
         }
     }
 
     private fun getAnimationItems(newStack: ChildStack<C, T>, oldStack: ChildStack<C, T>?): Map<Any, AnimationItem<C, T>> =
         when {
-            oldStack == null ->
+            (oldStack == null) || (newStack.active.key == oldStack.active.key) ->
                 listOf(AnimationItem(child = newStack.active, direction = Direction.ENTER_FRONT, isInitial = true))
 
             (newStack.size < oldStack.size) && (newStack.active.key in oldStack.backStack) ->

diff --git a/...mpose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt b/...mpose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt
@@ -14,7 +14,6 @@ import androidx.compose.ui.test.junit4.createComposeRule
 import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performClick
 import com.arkivanov.decompose.Child
-import com.arkivanov.decompose.ExperimentalDecomposeApi
 import com.arkivanov.decompose.FaultyDecomposeApi
 import com.arkivanov.decompose.extensions.compose.stack.animation.StackAnimation
 import com.arkivanov.decompose.extensions.compose.stack.animation.fade
@@ -27,11 +26,12 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.Parameterized
+import kotlin.test.assertSame
 
 @Suppress("TestFunctionName")
 @RunWith(Parameterized::class)
 class ChildrenTest(
-    private val animation: StackAnimation<Config, Config>?,
+    private val animation: StackAnimation<Config, Any>?,
 ) {
 
     @get:Rule
@@ -89,6 +89,18 @@ class ChildrenTest(
         composeRule.onNodeWithText(text = "ChildA1", substring = true).assertExists()
     }
 
+    @Test
+    fun GIVEN_child_displayed_WHEN_new_child_instance_with_the_same_key_THEN_new_child_instance_displayed() {
+        val state = mutableStateOf(routerState(Child.Created(configuration = Config.A, instance = Any(), key = "A")))
+        var lastInstance: Any? = null
+        setContent(state) { lastInstance = it.instance }
+
+        val instance2 = Any()
+        state.setValueOnIdle(routerState(Child.Created(configuration = Config.A, instance = instance2, key = "A")))
+
+        assertSame(instance2, lastInstance)
+    }
+
     @Test
     fun GIVEN_child_B_displayed_and_child_A_in_back_stack_WHEN_pop_child_B_THEN_state_restored_for_child_A() {
         val state = mutableStateOf(routerState(Config.A))
@@ -183,11 +195,14 @@ class ChildrenTest(
         composeRule.onNodeWithText(text = "ChildA2", substring = true).assertDoesNotExist()
     }
 
-    private fun setContent(state: State<ChildStack<Config, Config>>) {
+    private fun setContent(
+        state: State<ChildStack<Config, Any>>,
+        content: @Composable (Child.Created<Config, Any>) -> Unit = {
+            Child(name = it.key.toString())
+        },
+    ) {
         composeRule.setContent {
-            Children(stack = state.value, animation = animation) { child ->
-                Child(name = child.key.toString())
-            }
+            Children(stack = state.value, animation = animation, content = content)
         }
 
         composeRule.runOnIdle {}
@@ -208,6 +223,12 @@ class ChildrenTest(
             backStack = stack.dropLast(1).map { it.toChild() },
         )
 
+    private fun routerState(vararg stack: Child.Created<Config, Any>): ChildStack<Config, Any> =
+        ChildStack(
+            active = stack.last(),
+            backStack = stack.dropLast(1),
+        )
+
     private fun Pair<Any, Config>.toChild(): Child.Created<Config, Config> =
         Child.Created(configuration = second, instance = second, key = first)
 
@@ -233,7 +254,7 @@ class ChildrenTest(
             getParameters().map { arrayOf(it) }
 
         @OptIn(FaultyDecomposeApi::class)
-        private fun getParameters(): List<StackAnimation<Config, Config>?> =
+        private fun getParameters(): List<StackAnimation<Any, Any>?> =
             listOf(
                 null,
                 stackAnimation { _, _, _ -> null },