From 82aee7db64f4fc899d5498262bfc2a8fe22411e3 Mon Sep 17 00:00:00 2001 From: Arkadii Ivanov Date: Sat, 12 Oct 2024 16:54:10 +0100 Subject: [PATCH] Fixed stack animation not updating to new child component instance when active configuration is unchanged --- .../stack/animation/DefaultStackAnimation.kt | 28 ++++++++++++--- .../experimental/stack/ChildStackTest.kt | 33 ++++++++++++++--- .../stack/animation/AbstractStackAnimation.kt | 25 ++++++++++--- .../extensions/compose/stack/ChildrenTest.kt | 35 +++++++++++++++---- 4 files changed, 100 insertions(+), 21 deletions(-) diff --git a/extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt b/extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt index 1ade1239b..d35716ab3 100644 --- a/extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt +++ b/extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt @@ -49,22 +49,31 @@ internal class DefaultStackAnimation( ) { var currentStack by remember { mutableStateOf(stack) } var items by remember { mutableStateOf(getAnimationItems(newStack = currentStack)) } + var nextItems: Map>? 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( }, 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( private fun getAnimationItems(newStack: ChildStack, oldStack: ChildStack? = null): Map> = when { - oldStack == null -> + (oldStack == null) || (newStack.active.key == oldStack.active.key) -> keyedItemsOf( AnimationItem( child = newStack.active, diff --git a/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt b/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt index 5b8554453..8e207e5fd 100644 --- a/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt +++ b/extensions-compose-experimental/src/jvmTest/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?, + private val animation: StackAnimation?, ) { @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>) { + private fun setContent( + state: State>, + content: @Composable AnimatedVisibilityScope.(Child.Created) -> 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): ChildStack = + ChildStack( + active = stack.last(), + backStack = stack.dropLast(1), + ) + private fun Pair.toChild(): Child.Created = Child.Created(configuration = second, instance = second, key = first) diff --git a/extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt b/extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt index 5b079530b..43ed2cfba 100644 --- a/extensions-compose/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/stack/animation/AbstractStackAnimation.kt +++ b/extensions-compose/src/commonMain/kotlin/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( override operator fun invoke(stack: ChildStack, modifier: Modifier, content: @Composable (child: Child.Created) -> Unit) { var currentStack by remember { mutableStateOf(stack) } var items by remember { mutableStateOf(getAnimationItems(newStack = currentStack, oldStack = null)) } + var nextItems: Map>? 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,12 +58,21 @@ internal abstract class AbstractStackAnimation( }, 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()) } } @@ -63,7 +80,7 @@ internal abstract class AbstractStackAnimation( private fun getAnimationItems(newStack: ChildStack, oldStack: ChildStack?): Map> = 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/extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt b/extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt index c39fa4ed1..c348d190c 100644 --- a/extensions-compose/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/stack/ChildrenTest.kt +++ b/extensions-compose/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?, + private val animation: StackAnimation?, ) { @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>) { + private fun setContent( + state: State>, + content: @Composable (Child.Created) -> 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): ChildStack = + ChildStack( + active = stack.last(), + backStack = stack.dropLast(1), + ) + private fun Pair.toChild(): Child.Created = 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?> = + private fun getParameters(): List?> = listOf( null, stackAnimation { _, _, _ -> null },