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: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
WalkthroughMigrates worklet scheduling from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(200,220,255,0.5)
participant User
participant GestureDetector as GestureDetector (UI)
participant Worklet as Reanimated Worklet
participant RN as React Native (JS)
participant AppState as App State / onChangeTime
end
User->>GestureDetector: pan start / move / end
GestureDetector->>Worklet: .onStart / .onUpdate / .onEnd
Worklet->>Worklet: update shared values (contextX, translateX, scale)
alt gesture ends with change
Worklet->>RN: scheduleOnRN(onChangeTime, Math.round(currentTime * 1000))
RN->>AppState: apply time change
else gesture finalizes/no change
Worklet->>Worklet: restore translateX/currentTime, animate scale -> 1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
![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
|
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 |
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 `@app/containers/AudioPlayer/Seek.tsx`:
- Line 87: Remove the stray tab/extra whitespace on the empty line in the Seek
component file (Seek.tsx) to fix formatting; open the Seek component (function
or default export named Seek) and delete the trailing tab characters or
blank-line indentation so the file contains a clean empty line without
whitespace.
🪄 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: 11537494-2e4a-40c7-afaa-f96dc1276a6c
📒 Files selected for processing (1)
app/containers/AudioPlayer/Seek.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🪛 ESLint
app/containers/AudioPlayer/Seek.tsx
[error] 87-87: Delete ↹↹↹
(prettier/prettier)
🔇 Additional comments (3)
app/containers/AudioPlayer/Seek.tsx (3)
1-12: LGTM!Import changes correctly support the migration to the Gesture API and
scheduleOnRN.
67-90: LGTM! Gesture migration is well implemented.The migration from
useAnimatedGestureHandlertoGesture.Pan()is correct:
- Context capture in
onStartfor delta-based translation- Scale animation correctly placed in
onStart(runs once)- Position restoration on cancelled gestures via
onFinalizewithdidSucceedcheckscheduleOnRNusage is the standard pattern for bridging to JS thread from gesture handlers
130-132: LGTM!Correct usage of
GestureDetectorwrapping the interactive thumb element.
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
https://rocketchat.atlassian.net/browse/CORE-2034
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit