Don't seek stack animation transition on back start when predictive back animatable is specified #797
Conversation
WalkthroughThe pull request involves significant refactoring of the Changes
Possibly related PRs
Poem
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: 2
📜 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 (1 hunks)
- extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (2)
417-444: Tests correctly verify non-animating transition whenpredictiveBackAnimatableis not nullThe test
GIVEN_predictive_animatable_not_null_and_two_children_in_stack_WHEN_gesture_progressed_THEN_animation_transition_not_animatingaccurately checks that the animation transition does not animate when a non-nullpredictiveBackAnimatableis provided, which aligns with the intended behavior.
447-474: Tests correctly verify animating transition whenpredictiveBackAnimatableis nullThe test
GIVEN_predictive_animatable_null_and_two_children_in_stack_WHEN_gesture_progressed_THEN_animation_transition_animatingappropriately verifies that the animation transition animates based on gesture progress whenpredictiveBackAnimatableis null, ensuring correct functionality.
...arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt
Show resolved
Hide resolved
...vanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt
Outdated
Show resolved
Hide resolved
…ack animatable is specified
arkivanov
force-pushed
the
dont-animate-transition-on-back-start
branch
from
72a0187 to
2051b46
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/TestUtils.kt (1)
17-25: LGTM: Well-implemented animation function with a minor suggestion.The
animateFloatfunction is well-implemented and aligns with the PR objective. It correctly maps theEnterExitStateto appropriate float values for animation purposes.Consider adding a brief comment explaining the purpose of this function and the meaning of the float values. This would enhance readability and maintainability. For example:
/** * Animates the transition between EnterExitState values. * @return A State<Float> representing the animation progress: * 0F for PreEnter and PostExit (not visible) * 1F for Visible (fully visible) */ @Composable internal fun Transition<EnterExitState>.animateFloat(): State<Float> = // ... (rest of the function remains the same)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt (1 hunks)
- extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/TestUtils.kt (1 hunks)
- extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt
- extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt
🧰 Additional context used
🔇 Additional comments (2)
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/TestUtils.kt (2)
3-9: LGTM: Import statements are correct and necessary.The new import statements are appropriately added to support the new
animateFloatfunction. They include the required classes and annotations from the androidx.compose packages.
1-25: Summary: Effective addition of animation utility function.The changes to this file are focused and well-implemented. The new
animateFloatfunction provides a useful utility for animating transitions betweenEnterExitStatevalues, which aligns with the PR's objective of improving stack animation transitions. The function is properly integrated into the existingTestUtils.ktfile without disrupting other utilities.These changes enhance the animation capabilities of the library, particularly for testing purposes, as indicated by the file's location in the test package. The implementation is clean, concise, and follows Compose best practices.
arkivanov
changed the title
Summary by CodeRabbit
New Features
Bug Fixes
Documentation