Repository navigation
feat: Resolve CSS transition timelines for Android and bridge them over JNI - #10060
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Android C++ CSS transition management with easing conversion, scalar transition application, reversal handling, and removal. Wires transition callbacks through ChangesAndroid CSS transitions
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CSSPlatformTransitions
participant NativeProxy
participant CSSPlatformTransitionsManager
participant AndroidAnimator
CSSPlatformTransitions->>NativeProxy: cssAnimateTransition with values and easing
NativeProxy->>CSSPlatformTransitionsManager: animateTransition
CSSPlatformTransitionsManager->>AndroidAnimator: start transition
AndroidAnimator-->>CSSPlatformTransitionsManager: completion callback
CSSPlatformTransitionsManager->>CSSPlatformTransitionsManager: retain or remove completed state
CSSPlatformTransitions->>NativeProxy: cssRemoveTransition
NativeProxy->>CSSPlatformTransitionsManager: removeTransition
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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 |
fc321a9 to
a1a4c84
Compare
5045268 to
7eee6dd
Compare
94063c7 to
1357669
Compare
1357669 to
1e0895d
Compare
1e0895d to
db219a6
Compare
8681967 to
2cd0923
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.cpp`:
- Around line 94-103: In the transition loop around animate_, remove or cancel
the existing platform transition for the current key before returning false when
animate_ fails. Ensure both the prior animator and its active_ entry are cleaned
up so loop fallback cannot conflict with the old Android animator or stale
reversal state.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a6563d7c-8083-404d-a0ba-7b6bf6789f16
📒 Files selected for processing (5)
packages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.cpppackages/react-native-reanimated/android/src/main/cpp/reanimated/android/CSS/CSSPlatformTransitions.hpackages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.cpppackages/react-native-reanimated/android/src/main/cpp/reanimated/android/NativeProxy.hpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/NativeProxy.kt
942016e to
35ec2e7
Compare
35ec2e7 to
43c2fd7
Compare
a06bc94 to
d87aa6d
Compare
d87aa6d to
010000d
Compare
Owns the per-property state CSS reversing needs: it detects a reversal against the in-flight transition's start value, reverse-shortens the duration through the easing curve, and hands the resolved timeline to the Kotlin animator. Elapsed time crosses instead of an absolute start. Reverse-shortening backdates the start and transition-delay pushes it forward, and computing it here keeps Kotlin off a second clock that would drift from the loop's timestamp. Easings cross as the same normalized point arrays the loop uses, so nothing reimplements the easing spec on the other side of the boundary.
Hands the animator the resolved start timestamp instead of an elapsed offset, so it can subtract against the clock once it is actually on the UI thread. Apple already takes the same value as CALayer.beginTime.
Nothing needs setTransitionAlpha any more, so the manager no longer requires API 29 and can be constructed unconditionally. That makes the field non-null and removes the null handling that went with it.
A variadic generic lambda forwards to the shared object in three lines, so the bindShared template it replaced can go. Ownership is unchanged: the captured shared_ptr still keeps the object alive for as long as the callbacks do. Also drops comments that named the file they mirror or restated a field list.
010000d to
8fe070b
Compare
Owns the per-property state CSS reversing needs: it detects a reversal against the in-flight transition's start value, reverse-shortens the duration through the easing curve, and hands the resolved timeline to the Kotlin animator from #10090.
Elapsed time crosses the boundary instead of an absolute start. Reverse-shortening backdates the start and
transition-delaypushes it forward, and computing it here keeps Kotlin off a second clock that would drift from the loop's timestamp.Easings cross as the same normalized point arrays the loop uses, so nothing reimplements the easing spec on the other side of the boundary.
Nothing routes to this yet; it is inert until the next PR flips the gate.
Stacked on #10090.