Conversation
|
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 selected for processing (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)
🔇 Additional comments (5)
WalkthroughMigrates worklet scheduling from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(200,220,255,0.5)
participant User
participant UI as GestureDetector
participant Worklet as Reanimated Worklet
participant RN as React Native (JS)
participant State as App State / onChangeTime
end
User->>UI: Pan start / move / end
UI->>Worklet: .onStart / .onUpdate / .onEnd handlers
Worklet->>Worklet: update shared values (contextX, translateX, scale)
alt gesture ends with change
Worklet->>RN: scheduleOnRN(onChangeTime, Math.round(currentTime * 1000))
RN->>State: 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 |
|
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
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.
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