Skip to content

fix(VoIP): align NewMediaCall sheet layout - #7235

Merged
diegolmello merged 5 commits into
feat.voip-lib-newfrom
fix.newmediacall-ui
Apr 27, 2026
Merged

fix(VoIP): align NewMediaCall sheet layout#7235
diegolmello merged 5 commits into
feat.voip-lib-newfrom
fix.newmediacall-ui

Conversation

@diegolmello

@diegolmello diegolmello commented Apr 23, 2026

Copy link
Copy Markdown
Member

Proposed changes

Aligns app/containers/NewMediaCall/* with the Figma design across iPhone (portrait + landscape) and iPad by letting the bottom sheet hug its content and respecting safe-area insets.

  • useNewMediaCall.tsx: drop the explicit portraitSnaps/landscapeSnaps/fullContainer overrides — the sheet now hugs content on every device + orientation per the design. Only enableContentPanningGesture: false is kept.
  • NewMediaCall.tsx: root View uses paddingBottom: useSafeAreaInsets().bottom + 16 so the Call button clears the home indicator / nav bar instead of being flush against the device edge.
  • Known issue: Android is a little bit trickier to get it right than iOS. It's working, but we're going to improve the UI no a separate PR.

Issue(s)

https://rocketchat.atlassian.net/browse/VMUX-86

How to test or reproduce

  1. Build and run the app.
  2. Open a 1:1 DM → tap the call icon in the room header (seeds a chip). Confirm the sheet hugs its content and the Call button sits clear of the home indicator.
  3. Repeat on Android.
  4. From Sidebar → new call entry (no pre-seeded chip). Type a search → list expands, sheet still hugs content, Call button stays at the bottom with safe-area padding.
  5. Rotate the device on iPhone and iPad — sheet re-lays out correctly in landscape.

Screenshots

iOS Android
Screenshot 2026-04-27 at 13 34 25 Screenshot_20260427-133432_ DEBUG  RocketChat Experimental

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable) — visual layout change, covered by manual + design review
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Targets feat.voip-lib-new (the VoIP feature branch), not develop.

Summary by CodeRabbit

  • Bug Fixes

    • Adjusted bottom padding to respect device safe areas so content no longer overlaps system UI.
    • Prevented an empty participant/autocomplete list from rendering when there are no options.
    • Action sheet presentation now uses platform-aware full-screen behavior for more consistent interactions across devices.
  • Tests

    • Added coverage for empty peer-list behavior.
    • Added tests validating platform-specific action sheet presentation.

…iPad

- Drop fullContainer on iPad and iPhone landscape so the sheet hugs content
- Pin Call button to the sheet bottom via justifyContent: space-between + PeerList flex:1
- Use safe-area bottom inset for root paddingBottom
@coderabbitai

coderabbitai Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

NewMediaCall applies bottom safe-area padding; useNewMediaCall sets action-sheet fullContainer based on Android and removes explicit snap/panning options; PeerList avoids rendering when no options and tests cover the platform-driven action-sheet behavior and empty-peer case.

Changes

Cohort / File(s) Summary
Safe-area padding
app/containers/NewMediaCall/NewMediaCall.tsx
Import and use useSafeAreaInsets(); add bottom padding (bottom + 16) to the root container.
Action-sheet config change
app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
showActionSheetRef call simplified: removed explicit portraitSnaps, landscapeSnaps, and enableContentPanningGesture; fullContainer is now set conditionally based on isAndroid.
Peer list rendering & tests
app/containers/NewMediaCall/PeerList.tsx, app/containers/NewMediaCall/PeerList.test.tsx
PeerList returns null when options is empty; added test asserting null render for empty options with no selected peer.
Hook tests adjusted for platform behavior
app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
Tests mock deviceInfo.isAndroid, reinitialize the module to re-import the hook, and assert actionSheetArgs.fullContainer true for Android and false otherwise while preserving existing assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: fixing the NewMediaCall sheet layout to align with design specifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • VMUX-86: Request failed with status code 401

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/containers/NewMediaCall/NewMediaCall.tsx (1)

30-39: Nit: justifyContent: 'space-between' is effectively redundant.

Since styles.content already has flex: 1, it consumes all remaining vertical space and naturally pushes CreateCall to the bottom; space-between on the root has no additional effect in this configuration. It's harmless, just noting for cleanup if you want to keep the style minimal.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/NewMediaCall/NewMediaCall.tsx` around lines 30 - 39, Remove
the redundant justifyContent: 'space-between' from the styles object: open the
StyleSheet definition and delete the justifyContent property from styles.screen
(since styles.content already has flex: 1 and naturally pushes CreateCall to the
bottom), leaving styles.screen with flex, paddingHorizontal and paddingTop only;
keep styles.content as-is to preserve layout.
🤖 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/containers/NewMediaCall/NewMediaCall.tsx`:
- Around line 30-39: Remove the redundant justifyContent: 'space-between' from
the styles object: open the StyleSheet definition and delete the justifyContent
property from styles.screen (since styles.content already has flex: 1 and
naturally pushes CreateCall to the bottom), leaving styles.screen with flex,
paddingHorizontal and paddingTop only; keep styles.content as-is to preserve
layout.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bc3d6ee5-9ebd-4fe1-8f24-5dd77e46e1f2

📥 Commits

Reviewing files that changed from the base of the PR and between afb84a3 and 826ee26.

⛔ Files ignored due to path filters (2)
  • app/containers/NewMediaCall/__snapshots__/CreateCall.test.tsx.snap is excluded by !**/*.snap
  • app/containers/NewMediaCall/__snapshots__/PeerList.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (4)
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/containers/NewMediaCall/NewMediaCall.tsx
  • app/containers/NewMediaCall/PeerList.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
💤 Files with no reviewable changes (1)
  • app/containers/NewMediaCall/CreateCall.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions

Files:

  • app/containers/NewMediaCall/PeerList.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/containers/NewMediaCall/NewMediaCall.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers

**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms

Files:

  • app/containers/NewMediaCall/PeerList.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/containers/NewMediaCall/NewMediaCall.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with @rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/NewMediaCall/PeerList.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
  • app/containers/NewMediaCall/NewMediaCall.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Create reusable UI components in app/containers/ directory

Files:

  • app/containers/NewMediaCall/PeerList.tsx
  • app/containers/NewMediaCall/NewMediaCall.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/containers/**/*.{ts,tsx} : Create reusable UI components in app/containers/ directory

Applied to files:

  • app/containers/NewMediaCall/PeerList.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/hooks/useResponsiveLayout/**/*.{ts,tsx} : Implement responsive layouts using useResponsiveLayout hook to switch between master-detail on tablets and single stack on phones

Applied to files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
📚 Learning: 2026-03-17T19:15:30.463Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.

Applied to files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration

Applied to files:

  • app/containers/NewMediaCall/NewMediaCall.tsx
🔇 Additional comments (3)
app/containers/NewMediaCall/PeerList.tsx (1)

24-38: LGTM

Applying flex: 1 to the FlatList style correctly lets the list fill the middle of the new content wrapper in NewMediaCall, matching the space-between layout.

app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx (1)

15-34: Orientation is captured at open time — confirm intended behavior on rotation after opening.

isLandscape reflects dimensions at the time openNewMediaCall is called; the fullContainer flag is passed to the action sheet once and doesn't update if the user rotates while the sheet is open. The existing portraitSnaps/landscapeSnaps handle the snap fraction, but fullContainer (min inner height) will remain at whatever was computed on open. If the design expects the sheet to re-layout to hug content when rotating to landscape (or vice-versa) after opening, this will not happen. Please confirm this matches design.

app/containers/NewMediaCall/NewMediaCall.tsx (1)

18-40: Verify PeerList rendering when sheet hugs content (iPad / iPhone landscape).

With fullContainer = false on iPad/landscape, the action sheet parent hugs its content, which means flex: 1 on styles.screen and styles.content has no effect on the vertical main axis and the nested PeerList FlatList (also flex: 1) can collapse to its intrinsic rows height only. Please verify on iPad and iPhone landscape that:

  • An empty or short list still leaves room for the CreateCall button to be visible.
  • A long list scrolls correctly inside the sheet rather than being clipped or pushing CreateCall off-screen.

If there are issues, consider setting a maxHeight on the content wrapper or using flexGrow with a bounded parent, instead of relying on flex: 1 within a hug-content parent.

@diegolmello
diegolmello temporarily deployed to experimental_ios_build April 23, 2026 21:50 — with GitHub Actions Inactive
@diegolmello
diegolmello had a problem deploying to experimental_android_build April 23, 2026 21:50 — with GitHub Actions Error
@diegolmello
diegolmello had a problem deploying to official_android_build April 23, 2026 21:50 — with GitHub Actions Error
- Drop fullContainer and snaps so the ActionSheet detent is driven by content height on all devices
- Remove the intermediate flex wrapper and space-between so the Call button sits below the chip with marginTop 32 instead of being pinned to the sheet bottom
- Revert PeerList flex:1 now that the sheet no longer forces a fixed inner height
@diegolmello
diegolmello had a problem deploying to official_android_build April 24, 2026 17:24 — with GitHub Actions Error
@diegolmello
diegolmello temporarily deployed to experimental_ios_build April 24, 2026 17:24 — with GitHub Actions Inactive
@diegolmello
diegolmello temporarily deployed to experimental_android_build April 24, 2026 17:24 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat Experimental 4.72.0.108649

@diegolmello
diegolmello temporarily deployed to upload_experimental_android April 26, 2026 18:11 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

Android Build Available

Rocket.Chat Experimental 4.72.0.108652

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNRSgnkaI_ukeXFrmLhKB3US3JbowLMFXuKh7w5mUq9p2Pkgl3PUUtxrJ07MGcBRoaOuITpUuXC0mh6Ws5LP

@github-actions

Copy link
Copy Markdown

Android Build Available

Rocket.Chat Experimental 4.72.0.108652

@diegolmello diegolmello changed the title fix(VoIP): align NewMediaCall sheet layout with design on iPhone and iPad fix(VoIP): align NewMediaCall sheet layout Apr 27, 2026
… on device type

- Updated the useNewMediaCall hook to conditionally set fullContainer to true for Android devices.
- Enhanced unit tests to verify the correct behavior of the action sheet when isAndroid is true.
- Added a condition to return null when there are no options or a selected peer.
- Enhanced unit tests to verify the behavior of PeerList when options are empty.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
app/containers/NewMediaCall/PeerList.test.tsx (1)

70-75: Optional: minor cleanup for consistency.

Two small nits in this new test:

  • usePeerAutocompleteStore.setState({ options: [], selectedPeer: null }) on line 71 duplicates the state already set by beforeEach (line 41) and can be dropped.
  • Unlike the other tests in this file, the render skips the Wrapper (Redux Provider). It works today because PeerList short-circuits to null before touching Redux, but that coupling is a hidden assumption — using Wrapper here would keep the suite uniform and stay robust if PeerList ever reads from the store before the empty-options check.
♻️ Proposed tweak
-	it('should return null when there are no options', () => {
-		usePeerAutocompleteStore.setState({ options: [], selectedPeer: null });
-		const { toJSON } = render(<PeerList />);
-		expect(toJSON()).toBeNull();
-	});
+	it('should return null when there are no options', () => {
+		const { toJSON } = render(
+			<Wrapper>
+				<PeerList />
+			</Wrapper>
+		);
+		expect(toJSON()).toBeNull();
+	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/NewMediaCall/PeerList.test.tsx` around lines 70 - 75, Remove
the redundant state reset and render with the same test wrapper: delete the
duplicated call to usePeerAutocompleteStore.setState({ options: [],
selectedPeer: null }) in the test and change the render(<PeerList />) to use the
same Wrapper (e.g., render(<PeerList />, { wrapper: Wrapper }) or
render(<Wrapper><PeerList/></Wrapper>) consistent with other tests) so the test
remains uniform with beforeEach and robust if PeerList later reads from the
Redux store.
app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx (1)

112-135: Consider jest.isolateModules to scope the Android re-mock.

The jest.resetModules() + jest.doMock(...) + require(...) sequence mutates the module registry for the remainder of the suite. It happens to work today only because this is the last test in the describe block — adding a new test after it (or reordering) would silently inherit isAndroid: true and the re-required NewMediaCall mock. Wrapping the re-import in jest.isolateModules (and restoring with jest.dontMock in a cleanup) keeps the override scoped to this test.

♻️ Example using jest.isolateModules
 	it('should pass fullContainer to the action sheet when isAndroid is true', () => {
-		jest.resetModules();
-		jest.doMock('../../methods/helpers/deviceInfo', () => ({
-			isAndroid: true
-		}));
-		// Must load hook after doMock so `fullContainer: isAndroid` uses Android.
-		// eslint-disable-next-line `@typescript-eslint/no-var-requires`
-		const { useNewMediaCall: useNewMediaCallAndroid } = require('./useNewMediaCall');
-
-		mockUseSubscription.mockReturnValue(undefined);
-		mockUseMediaCallPermission.mockReturnValue(false);
-
-		const { result } = renderHook(() => useNewMediaCallAndroid('room-id'));
-
-		act(() => {
-			result.current.openNewMediaCall();
-		});
+		mockUseSubscription.mockReturnValue(undefined);
+		mockUseMediaCallPermission.mockReturnValue(false);
+
+		jest.isolateModules(() => {
+			jest.doMock('../../methods/helpers/deviceInfo', () => ({ isAndroid: true }));
+			// eslint-disable-next-line `@typescript-eslint/no-var-requires`
+			const { useNewMediaCall: useNewMediaCallAndroid } = require('./useNewMediaCall');
+
+			const { result } = renderHook(() => useNewMediaCallAndroid('room-id'));
+			act(() => {
+				result.current.openNewMediaCall();
+			});
+		});
 
-		// Re-required hook uses a fresh NewMediaCall mock ref; only assert fullContainer and element shape.
 		expect(mockShowActionSheetRef).toHaveBeenCalledTimes(1);
 		const [actionSheetArgs] = mockShowActionSheetRef.mock.calls[0];
 		expect(React.isValidElement(actionSheetArgs.children)).toBe(true);
 		expect(actionSheetArgs.fullContainer).toBe(true);
 	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx` around lines 112 -
135, The test mutates the module registry with jest.resetModules + jest.doMock +
require so subsequent tests may inherit isAndroid: true; wrap the re-import in
jest.isolateModules to scope the Android mock to this test (call
jest.isolateModules(() => { jest.doMock('../../methods/helpers/deviceInfo', ()
=> ({ isAndroid: true })); const { useNewMediaCall: useNewMediaCallAndroid } =
require('./useNewMediaCall'); /* run test code */ });) and then clean up the
mock with jest.dontMock or restore the original module in afterEach; target the
existing sequence that creates useNewMediaCallAndroid and the call to
result.current.openNewMediaCall() so only this test sees isAndroid: true and the
NewMediaCall mock ref used by mockShowActionSheetRef remains isolated.
🤖 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/containers/NewMediaCall/PeerList.test.tsx`:
- Around line 70-75: Remove the redundant state reset and render with the same
test wrapper: delete the duplicated call to usePeerAutocompleteStore.setState({
options: [], selectedPeer: null }) in the test and change the render(<PeerList
/>) to use the same Wrapper (e.g., render(<PeerList />, { wrapper: Wrapper }) or
render(<Wrapper><PeerList/></Wrapper>) consistent with other tests) so the test
remains uniform with beforeEach and robust if PeerList later reads from the
Redux store.

In `@app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx`:
- Around line 112-135: The test mutates the module registry with
jest.resetModules + jest.doMock + require so subsequent tests may inherit
isAndroid: true; wrap the re-import in jest.isolateModules to scope the Android
mock to this test (call jest.isolateModules(() => {
jest.doMock('../../methods/helpers/deviceInfo', () => ({ isAndroid: true }));
const { useNewMediaCall: useNewMediaCallAndroid } =
require('./useNewMediaCall'); /* run test code */ });) and then clean up the
mock with jest.dontMock or restore the original module in afterEach; target the
existing sequence that creates useNewMediaCallAndroid and the call to
result.current.openNewMediaCall() so only this test sees isAndroid: true and the
NewMediaCall mock ref used by mockShowActionSheetRef remains isolated.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fe57fb2-b52b-4fac-af72-ccec7a2bf28b

📥 Commits

Reviewing files that changed from the base of the PR and between 7bab3d4 and 67099a3.

📒 Files selected for processing (4)
  • app/containers/NewMediaCall/PeerList.test.tsx
  • app/containers/NewMediaCall/PeerList.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
✅ Files skipped from review due to trivial changes (1)
  • app/containers/NewMediaCall/PeerList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.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). (1)
  • GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions

Files:

  • app/containers/NewMediaCall/PeerList.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers

**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms

Files:

  • app/containers/NewMediaCall/PeerList.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with @rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/NewMediaCall/PeerList.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Create reusable UI components in app/containers/ directory

Files:

  • app/containers/NewMediaCall/PeerList.test.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/hooks/useResponsiveLayout/**/*.{ts,tsx} : Implement responsive layouts using useResponsiveLayout hook to switch between master-detail on tablets and single stack on phones

Applied to files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to **/*.{ts,tsx} : Support iOS 13.4+ and Android 6.0+ as minimum target platforms

Applied to files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
📚 Learning: 2026-03-17T19:15:30.463Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.

Applied to files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
🔇 Additional comments (1)
app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx (1)

42-44: The mock is appropriate for this test's scope.

The mock only exposes isAndroid because useNewMediaCall.tsx only imports isAndroid from deviceInfo. The other exports (hasNotch, isIOS, getReadableVersion, getBundleId, getDeviceModel, supportSystemTheme, isTablet) are not consumed by this hook or its tested dependencies. The test includes coverage for both isAndroid: false and isAndroid: true behavior.

			> Likely an incorrect or invalid review comment.

Resolves test conflict by keeping all three new tests (isAndroid->fullContainer
on PR side; isInActiveCall exposure + openNewMediaCall no-op on base side).
Hook auto-merged cleanly: keeps the early return on isInActiveCall and
fullContainer: isAndroid.
@diegolmello
diegolmello had a problem deploying to official_android_build April 27, 2026 16:57 — with GitHub Actions Failure
@diegolmello
diegolmello had a problem deploying to experimental_android_build April 27, 2026 16:57 — with GitHub Actions Failure
@diegolmello
diegolmello had a problem deploying to experimental_ios_build April 27, 2026 16:57 — with GitHub Actions Failure

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/lib/hooks/useNewMediaCall/useNewMediaCall.tsx`:
- Around line 25-28: The call to showActionSheetRef that renders <NewMediaCall
/> dropped the enableContentPanningGesture: false option which allows content
(search input/chips/peer list) to handle drag gestures without dismissing the
sheet; restore enableContentPanningGesture: false in the props passed to
showActionSheetRef and keep the existing fullContainer: isAndroid only if the
Android-specific full-container behavior is intentional—if not, remove or adjust
the fullContainer override to match the PR description. Reference:
showActionSheetRef, NewMediaCall, enableContentPanningGesture, fullContainer,
isAndroid.
🪄 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: c1359f54-3253-4f7a-be51-5a61cc32a1dd

📥 Commits

Reviewing files that changed from the base of the PR and between 67099a3 and aa55677.

📒 Files selected for processing (2)
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.test.tsx
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/lib/hooks/useNewMediaCall/useNewMediaCall.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
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions

Files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers

**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms

Files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with @rocket.chat/eslint-config base including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration

Applied to files:

  • app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx

Comment thread app/lib/hooks/useNewMediaCall/useNewMediaCall.tsx
@diegolmello
diegolmello merged commit a482f1a into feat.voip-lib-new Apr 27, 2026
7 of 12 checks passed
@diegolmello
diegolmello deleted the fix.newmediacall-ui branch April 27, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant