From fa095ea9bcea2accdcffbe614ab568b98c83e3ed Mon Sep 17 00:00:00 2001 From: Arkadii Ivanov Date: Mon, 16 Sep 2024 22:03:21 +0100 Subject: [PATCH] Fixed incorrect predictive back animation playing on hardware back button click with the new animation API --- .../stack/animation/DefaultStackAnimation.kt | 43 +++++--- .../animation/PredictiveBackGestureTest.kt | 100 ++++++++++++++++-- 2 files changed, 125 insertions(+), 18 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 1d4b696bb..1ade1239b 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 @@ -221,8 +221,24 @@ internal class DefaultStackAnimation( private val setItems: (Map>) -> Unit, ) : BackCallback() { private var animationHandler: AnimationHandler? = null + private var initialBackEvent: BackEvent? = null override fun onBackStarted(backEvent: BackEvent) { + initialBackEvent = backEvent + } + + override fun onBackProgressed(backEvent: BackEvent) { + startIfNeeded() + + scope.launch { + animationHandler?.progress(backEvent) + } + } + + private fun startIfNeeded() { + val backEvent = initialBackEvent ?: return + initialBackEvent = null + val animationHandler = AnimationHandler(animatable = predictiveBackParams.animatable(backEvent)) this.animationHandler = animationHandler val exitChild = stack.active @@ -252,25 +268,28 @@ internal class DefaultStackAnimation( } } - override fun onBackProgressed(backEvent: BackEvent) { - scope.launch { - animationHandler?.progress(backEvent) - } - } - override fun onBackCancelled() { + initialBackEvent = null + scope.launch { - animationHandler?.cancel() - animationHandler = null - setItems(getAnimationItems(newStack = stack)) + animationHandler?.also { handler -> + handler.cancel() + animationHandler = null + setItems(getAnimationItems(newStack = stack)) + } } } override fun onBack() { + initialBackEvent = null + scope.launch { - animationHandler?.finish() - animationHandler = null - setItems(getAnimationItems(newStack = stack.dropLast())) + animationHandler?.also { handler -> + handler.finish() + animationHandler = null + setItems(getAnimationItems(newStack = stack.dropLast())) + } + predictiveBackParams.onBack() } } diff --git a/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt b/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt index 3276498d1..aae5038d7 100644 --- a/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt +++ b/extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt @@ -23,6 +23,7 @@ import org.junit.Rule import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertTrue @Suppress("TestFunctionName") @OptIn(ExperimentalDecomposeApi::class) @@ -50,7 +51,7 @@ class PredictiveBackGestureTest { } @Test - fun WHEN_startPredictiveBack_THEN_gesture_started() { + fun WHEN_startPredictiveBack_THEN_active_child_shown_without_progress() { var stack by mutableStateOf(stack("1", "2")) val animation = DefaultStackAnimation(onBack = { stack = stack.dropLast() }) @@ -63,8 +64,9 @@ class PredictiveBackGestureTest { backDispatcher.startPredictiveBack(BackEvent(progress = 0F)) composeRule.waitForIdle() - composeRule.onNodeWithText("1").assertTestTagToRootExists(enterTestTag(progress = 0F)) - composeRule.onNodeWithText("2").assertTestTagToRootExists(exitTestTag(progress = 0F)) + composeRule.onNodeWithText("1").assertDoesNotExist() + composeRule.onNodeWithText("2").assertExists() + composeRule.onNodeWithText("2").assertTestTagToRootDoesNotExist { it.startsWith(TEST_TAG_PREFIX) } } @Test @@ -286,7 +288,50 @@ class PredictiveBackGestureTest { } @Test - fun GIVEN_three_children_in_stack_WHEN_predictive_back_finished_THEN_previous_child_not_animated_after_pop() { + fun GIVEN_three_children_in_stack_and_gesture_started_WHEN_predictive_back_finished_THEN_predictive_back_animatable_not_created() { + var stack by mutableStateOf(stack("1", "2", "3")) + val values = ArrayList() + var isAnimatableCreated = false + + val animation = + DefaultStackAnimation( + predictiveBackAnimatable = { initialBackEvent -> + isAnimatableCreated = true + TestAnimatable(initialBackEvent) + }, + onBack = { + values.clear() + stack = stack.dropLast() + }, + ) + + + composeRule.setContent { + animation(stack, Modifier) { + val float by transition.animateFloat { state -> + when (state) { + EnterExitState.PreEnter -> 0F + EnterExitState.Visible -> 1F + EnterExitState.PostExit -> 0F + } + } + + if (it.configuration == "2") { + values += float + } + } + } + + backDispatcher.startPredictiveBack(BackEvent(progress = 0F)) + composeRule.waitForIdle() + backDispatcher.back() + composeRule.waitForIdle() + + assertFalse(isAnimatableCreated) + } + + @Test + fun GIVEN_three_children_in_stack_and_gesture_started_WHEN_predictive_back_finished_THEN_previous_child_animated_after_pop() { var stack by mutableStateOf(stack("1", "2", "3")) val values = ArrayList() @@ -320,17 +365,60 @@ class PredictiveBackGestureTest { backDispatcher.back() composeRule.waitForIdle() + assertTrue(values.any { it < 1F }) + } + + @Test + fun GIVEN_three_children_in_stack_and_gesture_progressed_WHEN_predictive_back_finished_THEN_previous_child_not_animated_after_pop() { + var stack by mutableStateOf(stack("1", "2", "3")) + val values = ArrayList() + + val animation = + DefaultStackAnimation( + onBack = { + values.clear() + stack = stack.dropLast() + } + ) + + + composeRule.setContent { + animation(stack, Modifier) { + val float by transition.animateFloat { state -> + when (state) { + EnterExitState.PreEnter -> 0F + EnterExitState.Visible -> 1F + EnterExitState.PostExit -> 0F + } + } + + if (it.configuration == "2") { + values += float + } + } + } + + backDispatcher.startPredictiveBack(BackEvent(progress = 0F)) + composeRule.waitForIdle() + backDispatcher.progressPredictiveBack(BackEvent(progress = 0.5F)) + composeRule.waitForIdle() + backDispatcher.back() + composeRule.waitForIdle() + assertFalse(values.any { it < 1F }) } - private fun DefaultStackAnimation(onBack: () -> Unit): DefaultStackAnimation = + private fun DefaultStackAnimation( + predictiveBackAnimatable: (initialBackEvent: BackEvent) -> PredictiveBackAnimatable? = ::TestAnimatable, + onBack: () -> Unit, + ): DefaultStackAnimation = DefaultStackAnimation( disableInputDuringAnimation = false, predictiveBackParams = { PredictiveBackParams( backHandler = backDispatcher, onBack = onBack, - animatable = ::TestAnimatable, + animatable = predictiveBackAnimatable, ) }, selector = { _, _, _ -> null },