Conversation
WalkthroughThis PR upgrades React Native Reanimated from v3 to v4 and integrates React Native Worklets, replacing the deprecated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Failure
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Failure
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Failure
Rohit3523
had a problem deploying
to
upload_experimental_android
GitHub Actions
Failure
|
Android Build Available Rocket.Chat Experimental 4.66.0.107555 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTOBmiQlkEe-OeA3KvdurMOyXgF0YILJjAwuhDsPx5-LdCpHYt19pvHOiftS6iCycG0q6ZpQ_pgK-UBglwa |
…Chat/Rocket.Chat.ReactNative into test.poc-a11y-bottom-shet
…Chat/Rocket.Chat.ReactNative into test.poc-a11y-bottom-shet
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/containers/AudioPlayer/Seek.tsx`:
- Around line 76-80: The scale-up animation is currently run inside onUpdate
causing it to restart every frame; move the withTiming(1.3, { duration: 150 })
call into the onStart handler so it runs once when the gesture begins. Update
the gesture handlers around onStart, onUpdate, and onEnd: in onStart call
scale.value = withTiming(1.3, { duration: 150 }) (and set any initial
contextX.value if used), in onUpdate keep only the translation logic using
contextX, translateX and maxWidth (clamp newX), and in onEnd revert scale with
withTiming(1, { duration: 150 }) to restore the original size. Ensure you
reference the existing scale, translateX, contextX, maxWidth,
onStart/onUpdate/onEnd functions so behavior and state remain consistent.
- Line 84: The call to scheduleOnRN(onChangeTime, Math.round(currentTime.value *
1000)) from the gesture handler can cause iOS crashes when it bridges into the
main thread and calls AudioManager.setPositionAsync; update Seek.tsx to detect
iOS and avoid a single-shot scheduleOnRN call: add a platform check (iOS) around
scheduleOnRN, and implement a safe fallback such as queuing the position update
on the RN main thread with a short retry/backoff (e.g., try up to N times with
small delays) or call a native-safe bridge method instead of directly invoking
AudioManager.setPositionAsync from the worklet; ensure you reference
scheduleOnRN, onChangeTime, currentTime.value and the code path that ultimately
calls AudioManager.setPositionAsync when adding the workaround.
🪄 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: 4049f5e3-0a17-4646-b519-d0591066f65a
📒 Files selected for processing (3)
app/containers/AudioPlayer/Seek.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/RoomItem/Actions.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/containers/RoomItem/Actions.tsx
- app/containers/MessageComposer/hooks/useEmojiKeyboard.tsx
📜 Review details
🔇 Additional comments (3)
app/containers/AudioPlayer/Seek.tsx (3)
3-12: Imports look correct for the Reanimated v4 migration.The migration to
Gesture/GestureDetectorAPI andscheduleOnRNfromreact-native-workletsaligns with the PR objectives. The removal ofrunOnJSimport is expected as it's replaced byscheduleOnRN.
52-52: Good addition ofcontextXshared value.This correctly replaces the
contextparameter fromuseAnimatedGestureHandlerto capture the starting position at gesture start.
125-127: Correct usage ofGestureDetectorwrapper.The migration from
PanGestureHandlercomponent toGestureDetectorwith the composed gesture object follows the recommended react-native-gesture-handler v2+ pattern.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/containers/AudioPlayer/Seek.tsx`:
- Around line 67-81: The onEnd handler schedules onChangeTime (an async
Promise-returning function) directly via scheduleOnRN which can cause unhandled
rejections; create a RN-scoped wrapper in the component (e.g., seekOnRN) that
calls onChangeTime(Math.round(currentTime.value * 1000)), catches any rejection
and forwards it to the component's existing error path/handler, and then pass
that wrapper to scheduleOnRN in panGesture.onEnd instead of onChangeTime.
- Around line 82-85: onFinalize currently always resets isPanning and scale but
doesn’t restore state when a pan is cancelled; update the onFinalize callback to
check the provided didSucceed flag and if didSucceed === false restore the
pre-pan position by setting translateX back to the captured contextX and
resetting currentTime.value from contextX (the same context captured during
gesture start), otherwise (didSucceed === true) call onChangeTime to commit the
seek; ensure you still set isPanning.value = false and run scale.value =
withTiming(1, { duration: 150 }) in all cases.
🪄 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: 193f2f01-d5af-4b5d-b308-8a4a05d8e640
📒 Files selected for processing (3)
app/containers/AudioPlayer/Seek.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/RoomItem/Actions.tsx
✅ Files skipped from review due to trivial changes (1)
- app/containers/RoomItem/Actions.tsx
| const panGesture = Gesture.Pan() | ||
| .enabled(loaded) | ||
| .activeOffsetX([-ACTIVE_OFFSET_X, ACTIVE_OFFSET_X]) | ||
| .onStart(() => { | ||
| isPanning.value = true; | ||
| ctx.offsetX = translateX.value; | ||
| }, | ||
| onActive: ({ translationX }, ctx) => { | ||
| translateX.value = clamp(ctx.offsetX + translationX, 0, maxWidth.value); | ||
| scale.value = 1.3; | ||
| }, | ||
| onFinish() { | ||
| scale.value = 1; | ||
| contextX.value = translateX.value; | ||
| scale.value = withTiming(1.3, { duration: 150 }); | ||
| }) | ||
| .onUpdate(event => { | ||
| const newX = contextX.value + event.translationX; | ||
| translateX.value = clamp(newX, 0, maxWidth.value); | ||
| }) | ||
| .onEnd(() => { | ||
| scheduleOnRN(onChangeTime, Math.round(currentTime.value * 1000)); | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd app/containers/AudioPlayer && wc -l Seek.tsxRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 90
🏁 Script executed:
cat -n app/containers/AudioPlayer/Seek.tsx | head -100Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 4014
🏁 Script executed:
# Search for ISeek interface definition
rg "interface ISeek" -A 10Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 743
🏁 Script executed:
# Search for scheduleOnRN usage and imports
rg "scheduleOnRN" -B 2 -A 2Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 8010
🏁 Script executed:
# Search for onChangeTime type definition
rg "onChangeTime" -B 2 -A 2Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1552
Wrap the async seek callback before scheduling it.
Line 80 passes onChangeTime directly to scheduleOnRN. Since onChangeTime returns Promise<void> but scheduleOnRN is fire-and-forget (void return), any rejection from the seek operation will result in an unhandled promise rejection that bypasses error handling. Define an RN-scope wrapper in the component body that catches and routes promise rejections through the existing error path, then schedule that wrapper instead. (docs.swmansion.com)
🛡️ Proposed fix
+ const commitChangeTime = React.useCallback((time: number) => {
+ void onChangeTime(time).catch(() => {
+ // Route this through the existing error/reporting path.
+ });
+ }, [onChangeTime]);
+
const panGesture = Gesture.Pan()
.enabled(loaded)
.activeOffsetX([-ACTIVE_OFFSET_X, ACTIVE_OFFSET_X])
.onStart(() => {
isPanning.value = true;
contextX.value = translateX.value;
scale.value = withTiming(1.3, { duration: 150 });
})
.onUpdate(event => {
const newX = contextX.value + event.translationX;
translateX.value = clamp(newX, 0, maxWidth.value);
})
.onEnd(() => {
- scheduleOnRN(onChangeTime, Math.round(currentTime.value * 1000));
+ scheduleOnRN(commitChangeTime, Math.round(currentTime.value * 1000));
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/containers/AudioPlayer/Seek.tsx` around lines 67 - 81, The onEnd handler
schedules onChangeTime (an async Promise-returning function) directly via
scheduleOnRN which can cause unhandled rejections; create a RN-scoped wrapper in
the component (e.g., seekOnRN) that calls
onChangeTime(Math.round(currentTime.value * 1000)), catches any rejection and
forwards it to the component's existing error path/handler, and then pass that
wrapper to scheduleOnRN in panGesture.onEnd instead of onChangeTime.
| .onFinalize(() => { | ||
| isPanning.value = false; | ||
| runOnJS(onChangeTime)(Math.round(currentTime.value * 1000)); | ||
| } | ||
| }); | ||
| scale.value = withTiming(1, { duration: 150 }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "Seek.tsx" -type fRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 115
🏁 Script executed:
cat -n ./app/containers/AudioPlayer/Seek.tsxRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 5414
🌐 Web query:
react-native-gesture-handler pan gesture onFinalize didSucceed parameter
💡 Result:
In react-native-gesture-handler, the Pan gesture's onFinalize callback receives a second parameter called didSucceed, which is a boolean indicating whether the gesture was successfully recognized and completed. Signature: onFinalize: (event: PanHandlerData, didSucceed: boolean) => void - didSucceed is true if the gesture reached the END state (successfully completed after being ACTIVE). - didSucceed is false if the gesture FAILED or was CANCELLED. This applies to the modern API (usePanGesture in RNGH 3.x+). The callback is called whenever the handler stops recognizing the gesture, after onDeactivate if the gesture activated.
Citations:
- 1: https://docs.swmansion.com/react-native-gesture-handler/docs/fundamentals/callbacks-events
- 2: https://docs.swmansion.com/react-native-gesture-handler/docs/gestures/use-pan-gesture/
- 3: https://docs.swmansion.com/react-native-gesture-handler/docs/under-the-hood/state/
Restore the pre-pan position when the gesture is cancelled.
When a pan gesture is cancelled, onEnd does not run (it only executes for successful gestures), so onChangeTime is never called and the seek is not committed. However, onFinalize always runs and provides didSucceed to indicate the outcome. Since currentTime.value is mutated during panning (line 91), a cancelled gesture leaves the thumb at an uncommitted position until the next external update. Restore the pre-pan translateX and currentTime from the captured contextX value when didSucceed === false:
🔁 Proposed fix
- .onFinalize(() => {
+ .onFinalize((_, didSucceed) => {
+ if (isPanning.value && !didSucceed) {
+ translateX.value = contextX.value;
+ currentTime.value = (contextX.value * duration.value) / maxWidth.value || 0;
+ }
isPanning.value = false;
scale.value = withTiming(1, { duration: 150 });
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/containers/AudioPlayer/Seek.tsx` around lines 82 - 85, onFinalize
currently always resets isPanning and scale but doesn’t restore state when a pan
is cancelled; update the onFinalize callback to check the provided didSucceed
flag and if didSucceed === false restore the pre-pan position by setting
translateX back to the captured contextX and resetting currentTime.value from
contextX (the same context captured during gesture start), otherwise (didSucceed
=== true) call onChangeTime to commit the seek; ensure you still set
isPanning.value = false and run scale.value = withTiming(1, { duration: 150 })
in all cases.
|
iOS Build Available Rocket.Chat Experimental 4.71.0.108435 |
|
Android Build Available Rocket.Chat Experimental 4.71.0.108434 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNS1RtQubN6MMAV6ZUrFc1b0kUreKqDbpdWfbaPXjLGxe63axqkLb1yrncpK7nMv-7rvES64oTVal6fgsv-c |
2 participants
Proposed changes
This PR upgrades
react-native-reanimatedfrom v3 to v4.Short Summary of the changes
Issue(s)
https://rocketchat.atlassian.net/browse/COMM-46
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Refactor
Dependencies