feat(voip): tap-to-hide call controls with animations#7078
feat(voip): tap-to-hide call controls with animations#7078diegolmello merged 12 commits intofeat.voip-lib-newfrom
Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
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:
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
🧹 Nitpick comments (2)
app/lib/services/voip/useCallStore.ts (1)
208-210: Consider whethershowControlsis needed.The
showControlsaction is defined but the auto-reveal logic uses inlineset({ controlsVisible: true })instead. This action may be useful for external consumers or future enhancements. If it's not needed externally, you could simplify by removing it, or alternatively refactor the inline sets to use this action for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services/voip/useCallStore.ts` around lines 208 - 210, The store defines a showControls action but other code uses inline set({ controlsVisible: true }), causing duplication; either remove the showControls action if it has no external usage, or replace the inline set({ controlsVisible: true }) calls with calls to showControls for consistency. Locate the showControls function in useCallStore and the places that call set({ controlsVisible: true }) and choose one approach: delete showControls and keep inline sets (and remove its export), or refactor those inline sets to call showControls so all visibility changes go through the single action.app/lib/services/voip/useCallStore.test.ts (1)
33-35: Forward event payloads in the mock emitter.This helper currently invokes listeners without args, which can mask regressions if store handlers later depend on event payloads.
♻️ Proposed tweak
- const emit = (ev: string) => { - listeners[ev]?.forEach(fn => fn()); - }; + const emit = (ev: string, ...args: unknown[]) => { + listeners[ev]?.forEach(fn => fn(...args)); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services/voip/useCallStore.test.ts` around lines 33 - 35, The mock emitter function emit currently calls listeners without forwarding payloads; update the emit helper in useCallStore.test.ts so its signature accepts variadic payloads (e.g., emit(ev: string, ...args)) and invoke each listener with those payloads (fn(...args)); also adjust the listeners typing if necessary so listener functions can receive arguments, ensuring tests exercise real event payload handling used by the store handlers.
🤖 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/views/CallView/components/CallButtons.tsx`:
- Around line 30-33: The animated worklet in containerStyle captures
controlsVisible at creation time so it won't react to zustand updates; convert
the zustand boolean (from useControlsVisible) into a reanimated shared value
(useSharedValue) and sync it in a useEffect when the selector changes, then
reference that shared value inside useAnimatedStyle so opacity/translateY use
withTiming against the shared value (keep CONTROLS_ANIMATION_DURATION as-is) to
restore reactivity.
In `@app/views/CallView/components/CallerInfo.tsx`:
- Around line 17-20: The animated style uses the plain JS boolean
controlsVisible inside useAnimatedStyle which harms Reanimated performance;
convert controlsVisible into a Reanimated shared value and reference that inside
callerRowStyle. Specifically, create a useSharedValue (e.g.,
controlsVisibleShared), update it when the React prop/store changes (via
useEffect or a derived value) and then replace references to controlsVisible in
the useAnimatedStyle callback for callerRowStyle with
controlsVisibleShared.value while keeping CONTROLS_ANIMATION_DURATION and the
same withTiming logic.
---
Nitpick comments:
In `@app/lib/services/voip/useCallStore.test.ts`:
- Around line 33-35: The mock emitter function emit currently calls listeners
without forwarding payloads; update the emit helper in useCallStore.test.ts so
its signature accepts variadic payloads (e.g., emit(ev: string, ...args)) and
invoke each listener with those payloads (fn(...args)); also adjust the
listeners typing if necessary so listener functions can receive arguments,
ensuring tests exercise real event payload handling used by the store handlers.
In `@app/lib/services/voip/useCallStore.ts`:
- Around line 208-210: The store defines a showControls action but other code
uses inline set({ controlsVisible: true }), causing duplication; either remove
the showControls action if it has no external usage, or replace the inline set({
controlsVisible: true }) calls with calls to showControls for consistency.
Locate the showControls function in useCallStore and the places that call set({
controlsVisible: true }) and choose one approach: delete showControls and keep
inline sets (and remove its export), or refactor those inline sets to call
showControls so all visibility changes go through the single action.
🪄 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: 38495b52-7fdb-4b65-86d3-caa69559972d
⛔ Files ignored due to path filters (3)
app/containers/MediaCallHeader/__snapshots__/MediaCallHeader.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/__snapshots__/index.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/components/__snapshots__/CallerInfo.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
app/containers/MediaCallHeader/MediaCallHeader.test.tsxapp/containers/MediaCallHeader/MediaCallHeader.tsxapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/useCallStore.tsapp/views/CallView/components/CallButtons.test.tsxapp/views/CallView/components/CallButtons.tsxapp/views/CallView/components/CallerInfo.test.tsxapp/views/CallView/components/CallerInfo.tsxapp/views/CallView/index.test.tsxapp/views/CallView/styles.tsprogress-controls-animation.md
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/views/CallView/components/CallerInfo.test.tsxapp/views/CallView/components/CallButtons.tsxapp/views/CallView/components/CallerInfo.tsx
🔇 Additional comments (13)
app/views/CallView/styles.ts (1)
5-6: LGTM!Good practice extracting the animation duration as a shared constant. This ensures consistent timing across all animated components (
CallButtons,CallerInfo,MediaCallHeader) and makes future adjustments easy.app/lib/services/voip/useCallStore.ts (1)
204-210: LGTM - clean state management implementation.The
toggleControlsVisibleandshowControlsactions follow Zustand patterns correctly. Auto-revealing controls onstateChange,trackStateChange, andtoggleFocusprovides good UX by ensuring users see the controls during important call events.app/views/CallView/components/CallButtons.tsx (1)
52-55: LGTM for the Animated.View setup.The
pointerEventstoggle is a good practice to prevent accidental taps on hidden controls, and maintainingtestIDensures testability.app/views/CallView/components/CallerInfo.test.tsx (1)
57-70: LGTM - good test coverage for toggle behavior.The test properly verifies the complete toggle cycle (true → false → true) and directly accesses store state which is appropriate for testing Zustand stores.
app/containers/MediaCallHeader/MediaCallHeader.test.tsx (1)
179-210: LGTM - comprehensive test coverage for pointer events behavior.The three test cases properly cover the visibility matrix:
focused=true, controlsVisible=false→pointerEvents='none'focused=true, controlsVisible=true→pointerEvents='auto'focused=false→pointerEvents='auto'(header always interactive when collapsed)This correctly validates the conditional hiding logic.
app/views/CallView/index.test.tsx (1)
94-95: LGTM - testID references updated consistently.All assertions correctly updated from
caller-infotocaller-info-toggleto match the renamed/restructuredCallerInfocomponent.Also applies to: 105-106, 135-138, 302-303, 314-315
app/views/CallView/components/CallerInfo.tsx (1)
26-35: LGTM - clean implementation of tap-to-hide with correct separation.The avatar correctly remains outside the animated view (always visible), while only the caller name row animates. The
Pressableprovides the expected tap target covering the entire caller info area.app/containers/MediaCallHeader/MediaCallHeader.tsx (2)
33-38: Logic is correct; same animated style note applies.The
shouldHide = focused && !controlsVisiblelogic correctly ensures:
- Header hides only when the CallView is focused AND controls are toggled off
- Header stays visible when collapsed (not focused), regardless of
controlsVisibleThis matches the PR objective: "collapsed header bar remains visible."
50-61: LGTM - well-structured animated header.The Animated.View correctly applies the combined styles and manages pointer events. The empty state branch appropriately remains a plain View since no animation is needed when there's no call.
app/lib/services/voip/useCallStore.test.ts (2)
57-118: Good coverage forcontrolsVisiblelifecycle.The suite exercises default state, action transitions, reset behavior, focus toggling, and auto-reveal on call events—this is solid coverage for the new store contract.
174-174: Nice fix in stale-timer setup.Passing
createMockCall('x').callkeeps the test aligned withsetCallinput shape while preserving the helper’s event emitter API.app/views/CallView/components/CallButtons.test.tsx (1)
41-63: Targeted interaction-gating tests look good.Verifying
pointerEventsfor both hidden and visible states gives clear coverage for preventing ghost taps when controls are hidden.progress-controls-animation.md (1)
1-123: Well-structured rollout documentation.The slice breakdown, demos, and decision log are clear and actionable for both implementation tracking and QA validation.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Add toggle/show controls visibility for tap-to-hide animation support in CallView. Includes convenience selector and animation duration constant.
Wrap CallerInfo with Pressable to toggle controlsVisible on tap. Animate caller name row with fade + slide using Reanimated. Avatar remains always visible and centered.
Wrap CallButtons in Animated.View with opacity and translateY animations driven by controlsVisible store state. Set pointerEvents to 'none' when hidden to block ghost taps.
Update 'caller-info' to 'caller-info-toggle' to match the testID rename from the CallerInfo toggle implementation.
Replace active-call View with Animated.View that slides up and fades out when controlsVisible is false. Animation only applies when focused (expanded call view); collapsed header bar remains always visible.
Show controls automatically when stateChange, trackStateChange events fire or when toggling focus, so users never miss important state updates.
- Call showControls() from stateChange, trackStateChange, and toggleFocus instead of inlining controlsVisible in set() patches - Mock media emitter forwards variadic args in useCallStore tests - Add test and JSDoc for controls visibility / animation consumers Made-with: Cursor
The MediaCallHeader used translateY + opacity to hide, but its surfaceNeutral background remained visible as a dark bar. Animate backgroundColor and borderBottomColor to transparent so CallView shows through seamlessly.
…emove progress doc Merge controlsVisible: true into existing Zustand set() calls instead of separate showControls() to avoid double renders. Update stale MediaCallHeader snapshots and remove planning doc from repo.
diegolmello
force-pushed
the
feat.controls-animation
branch
from
45f3ef2 to
a8ae63c
Compare
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/views/CallView/components/CallButtons.tsx (1)
37-41: Placeholderalert()for Message functionality.The TODO is acknowledged. The
alert('Message')is fine for development but should be replaced with actual navigation before shipping.Would you like me to help scaffold the navigation to the chat room once the implementation details are known?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/CallView/components/CallButtons.tsx` around lines 37 - 41, The placeholder alert in handleMessage should be replaced with actual navigation to the chat room: remove alert('Message') and call the app's navigation method to open the RoomView (use the previously commented Navigation.navigate('RoomView', { rid, t: 'd' }) or the project's navigation helper). Ensure handleMessage has access to the caller room id (rid) and required navigation object (import or receive Navigation from props/context) and keep the route params { rid, t: 'd' } when invoking Navigation.navigate so tapping Message opens the correct chat room.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/views/CallView/components/CallButtons.tsx`:
- Around line 37-41: The placeholder alert in handleMessage should be replaced
with actual navigation to the chat room: remove alert('Message') and call the
app's navigation method to open the RoomView (use the previously commented
Navigation.navigate('RoomView', { rid, t: 'd' }) or the project's navigation
helper). Ensure handleMessage has access to the caller room id (rid) and
required navigation object (import or receive Navigation from props/context) and
keep the route params { rid, t: 'd' } when invoking Navigation.navigate so
tapping Message opens the correct chat room.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 312e07ae-8a70-4f5f-8637-44d86d5c7275
⛔ Files ignored due to path filters (3)
app/containers/MediaCallHeader/__snapshots__/MediaCallHeader.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/__snapshots__/index.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/components/__snapshots__/CallerInfo.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
app/containers/MediaCallHeader/MediaCallHeader.test.tsxapp/containers/MediaCallHeader/MediaCallHeader.tsxapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/useCallStore.tsapp/views/CallView/components/CallButtons.test.tsxapp/views/CallView/components/CallButtons.tsxapp/views/CallView/components/CallerInfo.test.tsxapp/views/CallView/components/CallerInfo.tsxapp/views/CallView/index.test.tsxapp/views/CallView/styles.ts
💤 Files with no reviewable changes (1)
- app/lib/services/voip/MediaSessionInstance.test.ts
✅ Files skipped from review due to trivial changes (4)
- app/views/CallView/styles.ts
- app/views/CallView/index.test.tsx
- app/containers/MediaCallHeader/MediaCallHeader.test.tsx
- app/views/CallView/components/CallButtons.test.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- app/containers/MediaCallHeader/MediaCallHeader.tsx
- app/lib/services/voip/useCallStore.test.ts
- app/lib/services/voip/useCallStore.ts
- app/views/CallView/components/CallerInfo.tsx
- app/views/CallView/components/CallerInfo.test.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
🧠 Learnings (3)
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/views/CallView/components/CallButtons.tsx
📚 Learning: 2026-03-04T20:13:17.288Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RocketChat Watch App/Views/MessageComposerView.swift:37-55
Timestamp: 2026-03-04T20:13:17.288Z
Learning: In the WatchOS app (ios/RocketChat Watch App) for Rocket.Chat React Native, using SwiftUI `Button` inside a `ScrollView` on WatchOS causes accidental message sends because button tap targets can be triggered during scroll gestures. `Text` with `.onTapGesture` is the preferred pattern for tappable items in scroll views on WatchOS. To preserve accessibility, add `.accessibilityAddTraits(.isButton)` and `.accessibilityLabel()` to the `Text` element instead.
Applied to files:
app/views/CallView/components/CallButtons.tsx
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/views/CallView/components/CallButtons.tsx
🔇 Additional comments (3)
app/views/CallView/components/CallButtons.tsx (3)
28-33: Reanimated reactivity with zustand state was previously flagged.The concern about
useAnimatedStylenot automatically reacting to plain JavaScript values (from the zustand selector) was already raised in a past review. The PR objectives indicate this is an intentional architectural choice where components re-render to drive the animated style rather than mirroring state into shared values.If animations appear janky or don't trigger reliably in testing, consider the suggested pattern of syncing to a shared value via
useEffect.
3-8: LGTM on imports.Clean import organization with centralized animation duration constant from styles.
52-55: Good use ofpointerEventsfor interaction gating.Setting
pointerEvents="none"when controls are hidden prevents accidental button presses during or after the hide animation. Style composition order is correct.
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
1 participant
Proposed changes
This PR adds tap-to-hide (and tap-to-show) VoIP call controls on the in-call screen, with Reanimated opacity and translation transitions and a single source of truth in the call Zustand store.
User-facing behavior
Pressablearound avatar + name) toggles whether secondary controls are visible.pointerEventsis set tononeso taps do not hit invisible controls.controlsVisible, so the mini header bar remains usable.Store and lifecycle
controlsVisibleflag (defaulttrue) withtoggleControlsVisible,showControls, and selectoruseControlsVisiblefor components that animate or gate interaction.showControls()is the single path for forcing controls back on after important events:stateChangeandtrackStateChangeon the media call emitter, andtoggleFocuswhen switching between expanded and collapsed call UI. That keeps “force visible” in one place and avoids scatteringcontrolsVisiblepatches next to unrelatedset({ … })updates.reset()restores initial store state (includingcontrolsVisible) like other call UI fields.Shared animation timing
CONTROLS_ANIMATION_DURATION(300 ms) is defined once in CallView styles and reused by CallButtons, CallerInfo, and MediaCallHeader so timing stays consistent.Tests
stateChange/trackStateChange. The in-memory media-call mock’semitforwards variadic arguments to listeners (aligned with real emitters) with a test that locks that behavior in.caller-info-toggletestID, pointerEvents when controls are hidden vs visible (CallButtons and MediaCallHeader), and existing CallView flows.Architecture note (Zustand vs Reanimated shared values)
controlsVisiblestays in Zustand. Subscribers re-render when it changes;useAnimatedStylereads the boolean on those updates. This PR does not mirror that flag into a Reanimated shared value for these animations—intentionally, to avoid duplicating state and to keep the feature easy to follow in one store.Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-19
How to test or reproduce
yarn test. Pay particular attention toapp/lib/services/voip/useCallStore.test.ts, CallView-related tests,CallerInfo,CallButtons, andMediaCallHeadertests.Screenshots
controls.webm
Types of changes
Checklist
Further comments
Suggested reply if a reviewer asks to drive these animations only from Reanimated shared values synced via
useEffect: We keepcontrolsVisiblein Zustand; components that subscribe re-render when it changes, which updatesuseAnimatedStyle. We are not mirroring this flag into shared values for this feature.Summary by CodeRabbit
New Features
Tests