Fixed the same predictive back gesture animation repeating when quickly started again#988
Merged
Merged
Conversation
📝 WalkthroughWalkthroughReworks PredictiveBack lifecycle in DefaultStackAnimation by replacing nullable handlers with a sealed State machine (Idle/Started/Progress/Finishing), switching keyed item collections to Changes
Sequence DiagramsequenceDiagram
actor User
participant Client
participant PredictiveBackCallback
participant StateMachine
participant AnimationHandler
participant Stack
User->>Client: start back gesture
Client->>PredictiveBackCallback: onBackStarted()
PredictiveBackCallback->>StateMachine: transition -> Started
StateMachine->>AnimationHandler: create/start handler
AnimationHandler->>Stack: compute animation items
loop gesture moves
User->>Client: gesture progress
Client->>PredictiveBackCallback: onBackProgressed(progress)
PredictiveBackCallback->>StateMachine: (must be Progress) update progress
StateMachine->>AnimationHandler: update progress
end
alt user cancels
User->>Client: cancel gesture
Client->>PredictiveBackCallback: onBackCancelled()
PredictiveBackCallback->>StateMachine: transition -> Idle
StateMachine->>AnimationHandler: cancel handler
AnimationHandler->>Stack: restore current items
else user completes
User->>Client: complete gesture
Client->>PredictiveBackCallback: onBack()
PredictiveBackCallback->>StateMachine: if Progress -> Finishing -> Idle
StateMachine->>AnimationHandler: finish handler
AnimationHandler->>Stack: finalize items and invoke onBack()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arkivanov
force-pushed
the
fixed-back-gesture-repeat
branch
from
f2f44f7 to
be43189
Compare
arkivanov
force-pushed
the
fixed-back-gesture-repeat
branch
2 times, most recently
from
b10d1ac to
4f20870
Compare
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@extensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.kt`:
- Around line 298-327: The bug is that onBackCancelled() and onBack() allow a
second predictive gesture to interrupt or duplicate the finishing coroutine;
update both handlers to treat State.Finishing as sticky by ignoring terminal
events while finishing: in onBackCancelled() only act when currentState is
State.Progress (leave state as Finishing and do not reset to Idle if
currentState is Finishing), and in onBack() only start the finish flow when
currentState is State.Progress; if currentState is State.Finishing simply return
(do not call predictiveBackParams.onBack or mutate state); preserve the existing
scope.launch logic that cancels/finishes currentState.animationHandler and then
sets state = State.Idle and calls setItems(getAnimationItems(...)) so the
finishing coroutine still completes uninterrupted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3cb91604-bd9e-4ff2-a88f-f6b00e5de780
📒 Files selected for processing (3)
extensions-compose-experimental/api/extensions-compose-experimental.klib.apiextensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.ktextensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt
…ly started again If you start a new predictive back gesture before the current gesture is fully finished, the same animation is started again. Instead, it should do nothing and allow the current animation to finish.
arkivanov
force-pushed
the
fixed-back-gesture-repeat
branch
from
985ec4a to
efaa7bb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt (1)
538-576: Remove the unusedanimationCountfrom this test.
animationCountis declared at line 541 and incremented at line 546, but never asserted in this test — it looks like a leftover from copy‑pastingGIVEN_gesture_finishing_WHEN_new_gesture_started_THEN_new_animation_not_started. The scenario here only needs to verify the stack isn't popped, so the counter (and the relatedanimatableside effect) is dead code.♻️ Proposed cleanup
`@Test` fun GIVEN_gesture_finishing_WHEN_back_THEN_stack_not_popped() { var stack by mutableStateOf(stack("1", "2")) - var animationCount = 0 val animation = DefaultStackAnimation( predictiveBackAnimatable = { - animationCount++ - TestAnimatable( initialBackEvent = it, finish = { suspendCancellableCoroutine { // Simulate a long-running animation } }, ) }, onBack = { stack = stack.dropLast() }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt` around lines 538 - 576, Remove the unused animationCount and its side-effect in the test GIVEN_gesture_finishing_WHEN_back_THEN_stack_not_popped: delete the declaration var animationCount and the animationCount++ inside the predictiveBackAnimatable lambda passed to DefaultStackAnimation (which constructs the TestAnimatable), leaving the TestAnimatable/finish behavior and onBack logic intact so the test only verifies the stack isn't popped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@extensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt`:
- Around line 538-576: Remove the unused animationCount and its side-effect in
the test GIVEN_gesture_finishing_WHEN_back_THEN_stack_not_popped: delete the
declaration var animationCount and the animationCount++ inside the
predictiveBackAnimatable lambda passed to DefaultStackAnimation (which
constructs the TestAnimatable), leaving the TestAnimatable/finish behavior and
onBack logic intact so the test only verifies the stack isn't popped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8968cc9-352d-480a-9e87-5b0def0fd8b7
📒 Files selected for processing (3)
extensions-compose-experimental/api/extensions-compose-experimental.klib.apiextensions-compose-experimental/src/commonMain/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/DefaultStackAnimation.ktextensions-compose-experimental/src/jvmTest/kotlin/com/arkivanov/decompose/extensions/compose/experimental/stack/animation/PredictiveBackGestureTest.kt
✅ Files skipped from review due to trivial changes (1)
- extensions-compose-experimental/api/extensions-compose-experimental.klib.api
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If you start a new predictive back gesture before the current gesture is fully finished, the same animation is started again. Instead, it should do nothing and allow the current animation to finish.
Summary by CodeRabbit
Refactor
Tests