Repository navigation
feat: CSS animation callbacks on native - #10071
Conversation
|
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 (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughNative CSS animation callbacks now use event masks. ChangesNative CSS animation callbacks
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The PR adds native CSS animation callbacks without any actionable merge-blocking risk remaining; it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CSSManager
participant CSSCallbacksManager
participant cssCallbacksRegistry
participant CSSAnimationsManager
CSSManager->>CSSCallbacksManager: synchronize animation callbacks
CSSCallbacksManager->>cssCallbacksRegistry: register callback subscriber
CSSManager->>CSSAnimationsManager: apply animation event mask
cssCallbacksRegistry->>CSSCallbacksManager: deliver native CSS animation event
CSSCallbacksManager->>CSSManager: invoke mapped callback
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 |
32ec074 to
3f45c1f
Compare
0cb1ca9 to
787bbd4
Compare
787bbd4 to
179d304
Compare
179d304 to
251a897
Compare
251a897 to
e441039
Compare
e441039 to
efd411a
Compare
efd411a to
db9736f
Compare
db9736f to
5ff8b5e
Compare
5ff8b5e to
0b73ead
Compare
0b73ead to
1d5871c
Compare
3cff5e0 to
eff7536
Compare
4f67146 to
dd5be47
Compare
a4af454 to
f74856b
Compare
f74856b to
ad289d9
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.native.tsx (1)
146-180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender settled props when no settled style exists.
_syncStylePropsBackToReactcan populatestate.settledPropswithout populatingstate.settledStylewhen all settled keys come fromanimatedPropsor inline top-level props. The render condition checks onlystate.settledStyle, so those completed animated props are not rendered by React.Gate on either settled state field, and add the
styleoverride only whensettledStyleexists.Proposed fix
- if (FORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONS && this.state.settledStyle) { + if ( + FORCE_REACT_RENDER_FOR_SETTLED_ANIMATIONS && + (this.state.settledStyle || this.state.settledProps) + ) { + const settledStyle = this.state.settledStyle; return super.render({ nativeID, ...filteredProps, ...this.state.settledProps, - style: [...flattenArray(filteredProps.style), this.state.settledStyle], + ...(settledStyle + ? { style: [...flattenArray(filteredProps.style), settledStyle] } + : {}), ...jestProps, }); }Also applies to: 445-450
🤖 Prompt for 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. In `@packages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.native.tsx` around lines 146 - 180, Update the render condition that applies settled state to check whether either settledProps or settledStyle exists, so completed animated props render even without settled styles. Only add the style override when settledStyle is present, preserving the existing behavior for absent settled state.
🤖 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.
Outside diff comments:
In
`@packages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.native.tsx`:
- Around line 146-180: Update the render condition that applies settled state to
check whether either settledProps or settledStyle exists, so completed animated
props render even without settled styles. Only add the style override when
settledStyle is present, preserving the existing behavior for absent settled
state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8d1c823a-6c3d-4689-abbb-b427991b1c21
📒 Files selected for processing (7)
packages/react-native-reanimated/src/createAnimatedComponent/AnimatedComponent.native.tsxpackages/react-native-reanimated/src/css/component/AnimatedComponent.native.tsxpackages/react-native-reanimated/src/css/native/events/index.tspackages/react-native-reanimated/src/css/native/events/mask.tspackages/react-native-reanimated/src/css/native/managers/CSSCallbacksManager.tspackages/react-native-reanimated/src/css/native/managers/__tests__/CSSCallbacksManager.test.tspackages/react-native-reanimated/src/css/native/managers/__tests__/CSSManager.test.ts
ad289d9 to
fb7421c
Compare
fb7421c to
f40c75c
Compare
The mask helper is a lookup table, so asserting each entry restated it, and the tag routing and unregistration were each covered at two layers. Keep the behavioural assertion of each and drop the duplicate.
Naming the sentinel meant touching the existing comparisons in createAnimatedComponent, which this PR otherwise leaves alone.
Makes
onAnimationStart,onAnimationEnd,onAnimationIterationandonAnimationCancelfire on iOS and Android. They already work on web, and the shared types promised them everywhere, but native dropped them.Callbacks are passed inside
style, alongside the other CSS config props. Only the subscription mask crosses the JSI boundary, never the user function, and it is pushed to C++ only when the set of present callbacks changes.A cancel emitted while the view is unmounting is still dropped here; #10255 delivers it.