Added isPredictiveBack flag to stackAnimation function with selector #826
Conversation
WalkthroughThe pull request introduces significant modifications 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: 1
🧹 Outside diff range and nitpick comments (3)
extensions-compose-experimental/api/extensions-compose-experimental.klib.api (1)
70-70: Update documentation for the modifiedstackAnimationfunctionThe
stackAnimationfunction now includes theisPredictiveBackparameter in its selector lambda, changing from aFunction3to aFunction4. Please update the API documentation to reflect this change and guide users on how to use the new parameter effectively.extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt (1)
32-44: Enhance explanation of theisPredictiveBackparameterThe
selectorfunction now includes theisPredictiveBackflag as a parameter. Consider providing additional context or examples in the documentation to clarify how this flag influences the animation selection, which will aid developers in understanding and utilizing this feature.extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (1)
509-509: Consider enhancing test coverage for isPredictiveBack parameterWhile the selector implementation is updated correctly, consider adding specific test cases that verify the behavior when
isPredictiveBackis true vs false.Example test case:
@Test fun WHEN_isPredictiveBack_true_THEN_selector_receives_correct_value() { var receivedIsPredictiveBack: Boolean? = null DefaultStackAnimation( selector = { _, _, _, isPredictiveBack -> receivedIsPredictiveBack = isPredictiveBack null } ) // Test assertions }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
extensions-compose-experimental/api/android/extensions-compose-experimental.api(1 hunks)extensions-compose-experimental/api/extensions-compose-experimental.klib.api(1 hunks)extensions-compose-experimental/api/jvm/extensions-compose-experimental.api(1 hunks)extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt(3 hunks)extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt(2 hunks)extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt(1 hunks)extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimationTest.kt(2 hunks)extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt(1 hunks)extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimationDirectionsTest.kt(1 hunks)
🔇 Additional comments (9)
extensions-compose-experimental/api/jvm/extensions-compose-experimental.api (1)
86-88: Verify consistency of stackAnimation method signature changes
The stackAnimation methods have been updated to accept Function4 instead of Function3. Ensure that this change is consistently applied across all platforms and related APIs to maintain uniformity.
Run the following script to check for consistent updates:
✅ Verification successful
Based on the search results, I can provide the final response:
stackAnimation signatures are consistently updated in experimental modules but differ in stable modules
The changes to use Function4 are consistently applied in both Android and JVM platforms within the experimental modules:
extensions-compose-experimental/api/android/extensions-compose-experimental.apiextensions-compose-experimental/api/jvm/extensions-compose-experimental.api
However, the stable modules still use Function3:
extensions-compose/api/android/extensions-compose.apiextensions-compose/api/jvm/extensions-compose.api
This is expected behavior as experimental APIs can evolve independently from stable ones.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all occurrences of `stackAnimation` methods and verify they use `Function4`
rg 'fun stackAnimation' -A 5
Length of output: 7629
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt (1)
70-70: Ensure default selector lambda handles isPredictiveBack correctly
In the default stackAnimation function, the selector lambda has been updated to include the isPredictiveBack parameter. Confirm that this default implementation applies the animator correctly, regardless of the predictive back gesture state.
Run the following script to validate the default selector usage:
✅ Verification successful
Let me analyze the code further by checking the implementation of the default stackAnimation function.
Let me try a different approach to find the implementation.
Let me try one more search to find the implementation details.
Default selector implementation correctly handles isPredictiveBack parameter
The default implementation of stackAnimation with the animator parameter correctly handles the isPredictiveBack flag by consistently returning the same animator regardless of the predictive back state. This is the intended behavior as it ensures a consistent animation experience whether the transition is triggered by regular navigation or predictive back gesture.
Reasoning:
- The default selector
{ _, _, _, _ -> animator }intentionally ignores all parameters, includingisPredictiveBack - This ensures the same animation is applied consistently across all navigation scenarios
- The predictive back gesture behavior is controlled separately through the
predictiveBackParamsparameter, not through the animation selector
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of the default `stackAnimation` function and check the `selector` implementation
rg 'stackAnimation\(' -A 5
Length of output: 37500
Script:
#!/bin/bash
# Find the implementation of stackAnimation function
ast-grep --pattern 'fun stackAnimation($$$) {
$$$
}'
Length of output: 56
Script:
#!/bin/bash
# Find the implementation of stackAnimation function with the specific selector signature
rg -l "stackAnimation.*selector.*isPredictiveBack" --type kotlin
# If found, show the content of those files
rg "stackAnimation.*selector.*isPredictiveBack" -A 10 --type kotlin
Length of output: 136
Script:
#!/bin/bash
# Find the file containing the stackAnimation implementation
rg -l "DefaultStackAnimation" --type kotlin
# Show the content of the file containing DefaultStackAnimation
cat extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimation.kt
Length of output: 4008
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimationTest.kt (1)
80-80: Verify all test cases accommodate the updated selector signature
The selector lambda in this test now accepts the isPredictiveBack parameter. Ensure that all other tests involving selector have been updated accordingly and that they properly handle the new parameter to prevent test failures or incorrect behaviors.
Run the following script to find and review all selector usages in test files:
✅ Verification successful
Based on the search results, I can now provide a final response:
All test cases are properly updated with the new selector signature
Looking at all the selector usages across test files, we can confirm that:
- In
DefaultStackAnimationTest.kt, the selector is using all four parameters:{ _, _, _, _ -> null } - In
PredictiveBackGestureTest.kt, the selector is using all four parameters:{ _, _, _, _ -> animator } - In
StackAnimationDirectionsTest.kt, the selector is using all four parameters:{ child, _, _, _ -> ...}
The class definition in DefaultStackAnimation.kt confirms the correct signature with all parameters: child, otherChild, direction, and isPredictiveBack.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Locate all instances of `selector` in test source files to confirm updates
rg 'selector\s*=\s*' src/jvmTest/
Length of output: 88
Script:
#!/bin/bash
# Let's try to find the test files using fd and then search for selector patterns
fd -e kt "Test.kt$" | xargs rg "selector\s*=\s*" -A 2
Length of output: 3895
Script:
#!/bin/bash
# Let's also search for the DefaultStackAnimation class usage to understand the context better
rg "DefaultStackAnimation" --type kotlin
Length of output: 5747
Script:
#!/bin/bash
# Search for stack animation related test files
fd "Stack.*Test.kt$"
Length of output: 899
Script:
#!/bin/bash
# Let's look at the DefaultStackAnimation class definition to understand the selector parameter
rg "class DefaultStackAnimation" -A 10 --type kotlin
Length of output: 4172
Script:
#!/bin/bash
# Let's also check the implementation file for any recent changes
git diff HEAD~5 -- "**/DefaultStackAnimation.kt"
Length of output: 79
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/StackAnimationDirectionsTest.kt (1)
Line range hint 39-44: LGTM! Selector function signature updated correctly.
The selector function signature has been updated to accept the new parameter, maintaining consistency with the broader changes in the animation framework.
Let's verify the consistency of this change across the codebase:
✅ Verification successful
Let me analyze the selector signatures more thoroughly by checking the actual type definitions.
Based on the analysis of the codebase, I can now generate the final response:
Selector function signature is correctly updated in experimental module
Looking at the implementations across the codebase, the selector function in the experimental module consistently uses 4 parameters as seen in multiple files:
extensions-compose-experimental/.../StackAnimation.kt:selector = { _, _, _, _ -> animator }extensions-compose-experimental/.../DefaultStackAnimationTest.kt:selector = { _, _, _, _ -> null }extensions-compose-experimental/.../PredictiveBackGestureTest.kt:selector = { _, _, _, _ -> animator }
The updated selector signature in the test file matches the pattern used throughout the experimental module.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify selector function signatures in animation-related files
# Test: Search for selector function definitions. Expect: All selectors should have 4 parameters
# Search for selector function definitions in animation-related files
rg -A 2 'selector\s*=\s*\{.*\}'
Length of output: 2943
Script:
#!/bin/bash
# Search for selector type definitions in animation-related files
ast-grep --pattern 'typealias $_ = ($$$) -> $_'
# Also search for interface or class definitions that might define the selector
rg -A 5 'interface.*Animation|class.*Animation'
Length of output: 49643
extensions-compose-experimental/api/android/extensions-compose-experimental.api (1)
86-86: LGTM! API signatures updated correctly.
The stackAnimation function signatures have been properly updated to use Function4, maintaining API consistency.
Also applies to: 88-88
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/ChildStackTest.kt (1)
277-291: LGTM! Comprehensive test coverage for stack animations.
The test cases have been properly updated to cover all animation scenarios with the new selector signature, including:
- Basic animations (null, scale, fade, slide, combined)
- Predictive back animations with both parameter variants
Let's verify the test coverage:
✅ Verification successful
Test coverage for stack animations is comprehensive and well-structured
The verification confirms that the test cases cover all essential animation scenarios:
- Basic animations (null, scale, fade, slide)
- Combined animations (scale + fade + slide)
- Predictive back animations with different parameter variants
- Different selector signatures (both in experimental and stable versions)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage for stack animations
# Test: Search for all test cases using stackAnimation. Expect: Coverage for all animation types
# Search for test cases using stackAnimation
rg -p 'stackAnimation.*\{.*\}' --type kotlin
Length of output: 2346
extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt (3)
41-46: LGTM: Selector function signature enhancement
The addition of isPredictiveBack parameter enhances the selector's capability to handle predictive back navigation scenarios.
231-238: LGTM: AnimationItem constructor enhancement
The constructor changes properly support predictive back navigation while maintaining backward compatibility with the default false value.
278-288: LGTM: Proper predictive back animation handling
The implementation correctly sets up animation handling for predictive back gestures with appropriate animator initialization.
...vanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimationTest.kt
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
StackAnimation,StackAnimator, andChildPanelsLayout.ChildPanelsmethods to improve flexibility in panel configurations.Bug Fixes
Tests