Repository navigation
feat: Keep platform-driven CSS transition values alive across React Native writes - #10092
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds pre-draw tracking for CSS platform transitions. Running transitions retain their animator, writer, and start value. Reconciliation restores overwritten properties before drawing and updates transition cleanup paths. ChangesCSS transition repair
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CSSPlatformTransitionsManager
participant CSSPlatformTransitionReconciler
participant ViewTreeObserver
participant View
CSSPlatformTransitionsManager->>CSSPlatformTransitionReconciler: track(target view)
CSSPlatformTransitionReconciler->>ViewTreeObserver: register pre-draw listener
ViewTreeObserver->>CSSPlatformTransitionReconciler: invoke before draw
CSSPlatformTransitionReconciler->>CSSPlatformTransitionsManager: repair active transitions
CSSPlatformTransitionsManager->>View: restore current or start value
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 |
ccf2a26 to
d42e9aa
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/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt`:
- Around line 39-43: Update CSSPlatformTransitionsManager.currentValue() to
return animator.animatedValue whenever it is available, including after the
animator stops but before cleanup; only fall back to startValue while the
animated value remains uninitialized, preserving the initial start-delay
behavior.
🪄 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: f586a3de-a898-46d7-b740-1a418c54572f
📒 Files selected for processing (2)
packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionReconciler.ktpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt
102d063 to
43471e0
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/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt`:
- Around line 132-134: The replacement logic around interrupted animator
handling must preserve the interrupted transition’s value: use its current
animated value while running, its startValue during the start delay, and
writer.get(view) only after completion; update currentValue() to fall back to
startValue when animatedValue is null. Add an instrumentation test covering the
clobber-and-replace sequence.
🪄 Autofix
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: 80f763d9-998a-4d90-8ae5-d05138c2170e
📒 Files selected for processing (1)
packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt
049ea54 to
cb76bbb
Compare
cb76bbb to
7353143
Compare
A platform transition animates the same View property React Native writes, so any commit touching the view overwrites it, and on Android Fabric re-applies the whole props blob, so any prop changing is enough. Re-asserts the animator's own value just before the frame is drawn. Every writer - a Fabric mount, the synchronous props path, the animation backend - runs on the UI thread earlier in the frame, so this covers all of them without knowing which one ran, and without depending on machinery that a feature flag can disable. The value comes from the animator rather than from resampling the curve, so it is exactly what is on screen; resampling would have to predict how long the commit takes. Most frames nothing was overwritten and the pass is a read and a compare.
ObjectAnimator only initialises its animated value on the first frame, and start() skips that when a start delay is set, so animatedValue reads back as a plain 0 for the whole delay. The repair pass took that at face value and drove the property to 0, which left a delayed opacity transition fully invisible until the animator produced its first frame. Carrying the start value alongside the animator gives the pass something true to hold until then. The listener now retires itself once nothing is left to hold, rather than the manager dropping it whenever the registry looks empty. Cancelling an animator runs its end callback synchronously, part way through installing the replacement, so the registry is briefly empty while a transition is very much still running. Deciding at pre-draw instead also lets each window stop on its own rather than waiting for the last animation anywhere to finish.
The listener can remove itself, so tracking windows needs a set rather than a map from observer to listener, and the separate retire step goes away. The repair pass was looking the writer up by property name on every frame even though it is known when the animation starts, so keep it with the animator and drop the lookup.
Returns early on the common path so the retire branch reads top to bottom. A window torn down mid-animation never draws again, so its listener never runs to retire itself; purging dead observers when a transition starts keeps the set from holding them.
097b5dd to
d7470a8
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/java/com/swmansion/reanimated/css/CSSPlatformTransitionReconciler.kt`:
- Around line 17-24: Update the active-transition tracking around
CSSPlatformTransitionReconciler.track so views re-register when attached to a
different window: add an OnAttachStateChangeListener that calls track(view) from
onViewAttachedToWindow, and retain the existing reconciliation behavior for
active transitions. Remove the listener when the final transition ends or is
removed, and add coverage for detaching and reattaching a view during a
transition.
🪄 Autofix
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: c3de5512-e693-424a-8656-e3970d6e9133
📒 Files selected for processing (2)
packages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionReconciler.ktpackages/react-native-reanimated/android/src/main/java/com/swmansion/reanimated/css/CSSPlatformTransitionsManager.kt
A platform transition animates the same
Viewproperty React Native writes, so any commit touching that view overwrites the running animation. On Android Fabric re-applies the whole props blob, so it is enough for any prop on the view to change.This PR adds a pre-draw listener that re-applies styles from running animators to ensure that the animation takes effect and is not overridden by RN.
Stacked on #10090.