Cancel predictive back gesture when stack changed (experimental animation API) #806
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt (1)
404-406: LGTM: Clean utility function, but remove extra blank lineThe
isSeekableextension function is well-implemented and improves code readability. However, there's an extra blank line before the function that should be removed to maintain consistent spacing with other utility functions in this file.private fun TransitionState<*>.isIdle(): Boolean = when (this) { is MutableTransitionState -> isIdle is SeekableTransitionState -> false else -> false } - private fun TransitionState<*>.isSeekable(): Boolean = this is SeekableTransitionStateextensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (1)
216-237: Enhance test coverage with additional assertions.While the test verifies the basic cancellation behavior, consider adding these assertions for more comprehensive coverage:
- Verify the final stack state after cancellation
- Assert the existence of the first item
- Verify the animation progress is reset
@Test fun GIVEN_gesture_started_WHEN_stack_popped_THEN_gesture_cancelled() { var stack by mutableStateOf(stack("1", "2")) val animation = DefaultStackAnimation(animator = fade(), onBack = { stack = stack.dropLast() },) composeRule.setContent { animation(stack, Modifier) { Text(text = it.configuration) } } backDispatcher.startPredictiveBack(BackEvent(progress = 0F)) composeRule.waitForIdle() backDispatcher.progressPredictiveBack(BackEvent(progress = 0.5F)) composeRule.waitForIdle() stack = stack.dropLast() composeRule.mainClock.autoAdvance = false composeRule.mainClock.advanceTimeByFrame() + assertEquals(stack("1"), stack) + composeRule.onNodeWithText("1").assertExists() + composeRule.onNodeWithText("1").assertTestTagToRootDoesNotExist { it.startsWith(TEST_TAG_PREFIX) } composeRule.onNodeWithText("2").assertTestTagToRootDoesNotExist { it.startsWith(TEST_TAG_PREFIX) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt(2 hunks)extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt(3 hunks)
🔇 Additional comments (2)
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt (1)
72-72: LGTM: Enhanced animation state handling for predictive back gestures
The additional condition items.values.last().transitionState.isSeekable() ensures smooth animation transitions during predictive back gestures by allowing immediate item updates when the last transition state is seekable. This prevents potential animation glitches during stack changes.
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (1)
497-497: LGTM!
The addition of the optional animator parameter is clean and maintains backward compatibility. The parameter is correctly utilized in the selector lambda.
Also applies to: 509-509
arkivanov
deleted the
cancel-predictive-back-animation-when-stack-changed
branch
Summary by CodeRabbit
New Features
Bug Fixes
Documentation