Skip to content

feat: Separate video call and voice call room actions #35700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 7 commits into from
Apr 17, 2025
Merged

feat: Separate video call and voice call room actions #35700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c10597d
move call room actions
yash-rajpal Apr 3, 2025
8d3e9f7
improve tests
yash-rajpal Apr 7, 2025
e055e46
fix conflicts and merge
yash-rajpal Apr 7, 2025
286ba5d
add some tests for video call action in federated rooms
yash-rajpal Apr 7, 2025
75fc8b1
add changeset
yash-rajpal Apr 8, 2025
14570b5
chore: replace data-qa-id with role selectors
yash-rajpal Apr 8, 2025
118602b
Merge branch 'develop' into call-room-actions
kodiakhq[bot] Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wet-penguins-end.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': minor
---

Separates voice call and video call room actions on room header.
1 change: 0 additions & 1 deletion apps/meteor/client/hooks/roomActions/useStartCallRoomAction/index.ts
View file
Open in desktop

This file was deleted.

41 changes: 0 additions & 41 deletions apps/meteor/client/hooks/roomActions/useStartCallRoomAction/useStartCallRoomAction.tsx
View file
Open in desktop

This file was deleted.

47 changes: 18 additions & 29 deletions ...allRoomAction/useVideoConfMenuOptions.tsx → ...ks/roomActions/useVideoCallRoomAction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { isOmnichannelRoom, isRoomFederated } from '@rocket.chat/core-typings';
import { Box } from '@rocket.chat/fuselage';
import { isRoomFederated } from '@rocket.chat/core-typings';
import { useEffectEvent, useStableArray } from '@rocket.chat/fuselage-hooks';
import type { GenericMenuItemProps } from '@rocket.chat/ui-client';
import { usePermission, useSetting, useUser } from '@rocket.chat/ui-contexts';
import {
useVideoConfDispatchOutgoing,
Expand All @@ -12,11 +10,11 @@ import {
import { useMemo } from 'react';
import { useTranslation } from 'react-i18next';

import { useRoom } from '../../../views/room/contexts/RoomContext';
import type { RoomToolboxActionConfig } from '../../../views/room/contexts/RoomToolboxContext';
import { useVideoConfWarning } from '../../../views/room/contextualBar/VideoConference/hooks/useVideoConfWarning';
import { useRoom } from '../../views/room/contexts/RoomContext';
import type { RoomToolboxActionConfig } from '../../views/room/contexts/RoomToolboxContext';
import { useVideoConfWarning } from '../../views/room/contextualBar/VideoConference/hooks/useVideoConfWarning';

const useVideoConfMenuOptions = () => {
export const useVideoCallRoomAction = () => {
const { t } = useTranslation();
const room = useRoom();
const user = useUser();
Expand Down Expand Up @@ -54,7 +52,6 @@ const useVideoConfMenuOptions = () => {
const allowed = visible && permittedToCallManagement && (!user?.username || !room.muted?.includes(user.username)) && !ownUser;
const disabled = federated || (!!room.ro && !permittedToPostReadonly);
const tooltip = disabled ? t('core.Video_Call_unavailable_for_this_type_of_room') : '';
const order = isOmnichannelRoom(room) ? -1 : 4;

const handleOpenVideoConf = useEffectEvent(async () => {
if (isCalling || isRinging) {
Expand All @@ -69,29 +66,21 @@ const useVideoConfMenuOptions = () => {
}
});

return useMemo(() => {
const items: GenericMenuItemProps[] = [
{
id: 'start-video-call',
icon: 'video',
disabled,
onClick: handleOpenVideoConf,
content: (
<Box is='span' title={tooltip}>
{t('Video_call')}
</Box>
),
},
];
return useMemo((): RoomToolboxActionConfig | undefined => {
if (!allowed) {
return undefined;
}

return {
items,
disabled,
allowed,
order,
id: 'start-video-call',
title: 'Video_call',
icon: 'video',
featured: true,
action: handleOpenVideoConf,
order: -1,
groups,
disabled,
tooltip,
};
}, [allowed, disabled, groups, handleOpenVideoConf, order, t, tooltip]);
}, [allowed, groups, disabled, handleOpenVideoConf, tooltip]);
};

export default useVideoConfMenuOptions;
48 changes: 17 additions & 31 deletions ...tartCallRoomAction/useVoipMenuOptions.tsx → ...ks/roomActions/useVoiceCallRoomAction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { Box } from '@rocket.chat/fuselage';
import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
import type { GenericMenuItemProps } from '@rocket.chat/ui-client';
import { usePermission, useUserId } from '@rocket.chat/ui-contexts';
import { useVoipAPI, useVoipState } from '@rocket.chat/ui-voip';
import { useMemo } from 'react';
import { useTranslation } from 'react-i18next';

import { useMediaPermissions } from '../../../views/room/composer/messageBox/hooks/useMediaPermissions';
import { useRoom } from '../../../views/room/contexts/RoomContext';
import { useUserInfoQuery } from '../../useUserInfoQuery';
import { useVoipWarningModal } from '../../useVoipWarningModal';
import { useMediaPermissions } from '../../views/room/composer/messageBox/hooks/useMediaPermissions';
import { useRoom } from '../../views/room/contexts/RoomContext';
import type { RoomToolboxActionConfig } from '../../views/room/contexts/RoomToolboxContext';
import { useUserInfoQuery } from '../useUserInfoQuery';
import { useVoipWarningModal } from '../useVoipWarningModal';

const useVoipMenuOptions = () => {
export const useVoiceCallRoomAction = () => {
const { t } = useTranslation();
const { uids = [] } = useRoom();
const ownUserId = useUserId();
Expand All @@ -32,10 +31,10 @@ const useVoipMenuOptions = () => {
const isDM = members.length === 1;

const disabled = isMicPermissionDenied || !isDM || isInCall || isPending;
const allowed = isDM && !isInCall && !isPending;
const allowed = canStartVoiceCall && isDM && !isInCall && !isPending;
const canMakeVoipCall = allowed && isRemoteRegistered && isRegistered && isEnabled && !isMicPermissionDenied;

const title = useMemo(() => {
const tooltip = useMemo(() => {
if (isMicPermissionDenied) {
return t('Microphone_access_not_allowed');
}
Expand All @@ -54,33 +53,20 @@ const useVoipMenuOptions = () => {
dispatchWarning();
});
Comment on lines 53 to 54
Copy link

@kody-ai kody-ai bot Apr 17, 2025

Choose a reason for hiding this comment

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

kody code-review Potential Issues high

if (typeof remoteUser?.freeSwitchExtension === 'string') {
    return makeCall(remoteUser.freeSwitchExtension);
}
dispatchWarning();

The remoteUser?.freeSwitchExtension is being cast to a string without ensuring it is a valid string, which could lead to runtime errors.

This issue appears in multiple locations:

  • apps/meteor/client/hooks/roomActions/useVoiceCallRoomAction.tsx: Lines 53-54
  • apps/meteor/client/hooks/roomActions/useVoiceCallRoomAction.tsx: Lines 53-54
    Please add a type check to ensure freeSwitchExtension is a valid string before making the call to prevent potential runtime errors.

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​


return useMemo(() => {
if (!canStartVoiceCall) {
return useMemo((): RoomToolboxActionConfig | undefined => {
if (!allowed) {
return undefined;
}

const items: GenericMenuItemProps[] = [
{
id: 'start-voip-call',
icon: 'phone',
disabled,
onClick: handleOnClick,
content: (
<Box is='span' title={title}>
{t('Voice_call')}
</Box>
),
},
];

return {
items,
id: 'start-voice-call',
title: 'Voice_Call',
icon: 'phone',
featured: true,
action: handleOnClick,
groups: ['direct'] as const,
disabled,
order: 4,
allowed,
tooltip,
};
}, [disabled, title, t, handleOnClick, allowed, canStartVoiceCall]);
}, [allowed, disabled, handleOnClick, tooltip]);
};

export default useVoipMenuOptions;
6 changes: 4 additions & 2 deletions apps/meteor/client/ui.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ import { usePushNotificationsRoomAction } from './hooks/roomActions/usePushNotif
import { useRocketSearchRoomAction } from './hooks/roomActions/useRocketSearchRoomAction';
import { useRoomInfoRoomAction } from './hooks/roomActions/useRoomInfoRoomAction';
import { useStarredMessagesRoomAction } from './hooks/roomActions/useStarredMessagesRoomAction';
import { useStartCallRoomAction } from './hooks/roomActions/useStartCallRoomAction';
import { useTeamChannelsRoomAction } from './hooks/roomActions/useTeamChannelsRoomAction';
import { useTeamInfoRoomAction } from './hooks/roomActions/useTeamInfoRoomAction';
import { useThreadRoomAction } from './hooks/roomActions/useThreadRoomAction';
import { useUploadedFilesListRoomAction } from './hooks/roomActions/useUploadedFilesListRoomAction';
import { useUserInfoGroupRoomAction } from './hooks/roomActions/useUserInfoGroupRoomAction';
import { useUserInfoRoomAction } from './hooks/roomActions/useUserInfoRoomAction';
import { useVideoCallRoomAction } from './hooks/roomActions/useVideoCallRoomAction';
import { useVoIPRoomInfoRoomAction } from './hooks/roomActions/useVoIPRoomInfoRoomAction';
import { useVoiceCallRoomAction } from './hooks/roomActions/useVoiceCallRoomAction';
import { useWebRTCVideoRoomAction } from './hooks/roomActions/useWebRTCVideoRoomAction';
import type { RoomToolboxActionConfig } from './views/room/contexts/RoomToolboxContext';
import type { QuickActionsActionConfig } from './views/room/lib/quickActions';
Expand Down Expand Up @@ -63,12 +64,13 @@ export const roomActionHooks = [
useRocketSearchRoomAction,
useRoomInfoRoomAction,
useStarredMessagesRoomAction,
useStartCallRoomAction,
useTeamChannelsRoomAction,
useUploadedFilesListRoomAction,
useVoIPRoomInfoRoomAction,
useWebRTCVideoRoomAction,
useAppsRoomStarActions,
useVideoCallRoomAction,
useVoiceCallRoomAction,
] satisfies (() => RoomToolboxActionConfig | undefined)[];

export const quickActionHooks = [
Expand Down
4 changes: 4 additions & 0 deletions apps/meteor/tests/e2e/federation/page-objects/fragments/home-flextab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export class FederationHomeFlextab {
return this.page.locator('[data-qa-id="ToolBoxAction-phone"]');
}

get btnVideoCall(): Locator {
return this.page.locator('[role=toolbar][aria-label="Primary Room actions"]').getByRole('button', { name: 'Video call' });
}

get btnDiscussion(): Locator {
return this.page.locator('[data-qa-id="ToolBoxAction-discussion"]');
}
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/tests/e2e/federation/tests/channel/dm.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,9 +770,11 @@ test.describe.parallel('Federation - Direct Messages', () => {
await poFederationChannelServer1.content.sendMessage('hello world');

await expect(poFederationChannelServer1.tabs.btnCall).toBeDisabled();
await expect(poFederationChannelServer1.tabs.btnVideoCall).toBeDisabled();

await poFederationChannelServer2.sidenav.openChat(usernameWithDomainFromServer1);
await expect(poFederationChannelServer2.tabs.btnCall).toBeDisabled();
await expect(poFederationChannelServer1.tabs.btnVideoCall).toBeDisabled();

await pageForServer2.close();
});
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/tests/e2e/federation/tests/channel/private.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,8 @@ test.describe.parallel('Federation - Group Creation', () => {

await expect(poFederationChannelServer1.tabs.btnCall).toBeDisabled();
await expect(poFederationChannelServer2.tabs.btnCall).toBeDisabled();
await expect(poFederationChannelServer1.tabs.btnVideoCall).toBeDisabled();
await expect(poFederationChannelServer2.tabs.btnVideoCall).toBeDisabled();

await pageForServer2.close();
});
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/tests/e2e/federation/tests/channel/public.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,8 @@ test.describe.parallel('Federation - Channel Creation', () => {

await expect(poFederationChannelServer1.tabs.btnCall).toBeDisabled();
await expect(poFederationChannelServer2.tabs.btnCall).toBeDisabled();
await expect(poFederationChannelServer1.tabs.btnVideoCall).toBeDisabled();
await expect(poFederationChannelServer2.tabs.btnVideoCall).toBeDisabled();

await pageForServer2.close();
});
Expand Down
8 changes: 2 additions & 6 deletions apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,8 @@ export class HomeContent {
return this.page.locator('[data-qa-id="ToolBoxAction-pause-unfilled"]');
}

get btnCall(): Locator {
return this.page.locator('[data-qa-id="ToolBoxAction-phone"]');
}

get menuItemVideoCall(): Locator {
return this.page.locator('role=menuitem[name="Video call"]');
get btnVideoCall(): Locator {
return this.page.locator('[role=toolbar][aria-label="Primary Room actions"]').getByRole('button', { name: 'Video call' });
}

get btnStartVideoCall(): Locator {
Expand Down
3 changes: 1 addition & 2 deletions apps/meteor/tests/e2e/video-conference-ring.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ test.describe('video conference ringing', () => {

await auxContext.poHomeChannel.sidenav.openChat('user1');
await test.step('should user1 calls user2', async () => {
await poHomeChannel.content.btnCall.click();
await poHomeChannel.content.menuItemVideoCall.click();
await poHomeChannel.content.btnVideoCall.click();
await poHomeChannel.content.btnStartVideoCall.click();

await expect(poHomeChannel.content.getVideoConfPopupByName('Calling user2')).toBeVisible();
Expand Down
14 changes: 5 additions & 9 deletions apps/meteor/tests/e2e/video-conference.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ test.describe('video conference', () => {
test('expect create video conference in a "targetChannel"', async () => {
await poHomeChannel.sidenav.openChat(targetChannel);

await poHomeChannel.content.btnCall.click();
await poHomeChannel.content.menuItemVideoCall.click();
await poHomeChannel.content.btnVideoCall.click();
await poHomeChannel.content.btnStartVideoCall.click();
await expect(poHomeChannel.content.videoConfMessageBlock.last()).toBeVisible();
});
Expand Down Expand Up @@ -64,8 +63,7 @@ test.describe('video conference', () => {
test('expect create video conference in a direct', async () => {
await poHomeChannel.sidenav.openChat('user2');

await poHomeChannel.content.btnCall.click();
await poHomeChannel.content.menuItemVideoCall.click();
await poHomeChannel.content.btnVideoCall.click();
await poHomeChannel.content.btnStartVideoCall.click();
await expect(poHomeChannel.content.videoConfMessageBlock.last()).toBeVisible();
});
Expand All @@ -81,8 +79,7 @@ test.describe('video conference', () => {
test('expect create video conference in a "targetTeam"', async () => {
await poHomeChannel.sidenav.openChat(targetTeam);

await poHomeChannel.content.btnCall.click();
await poHomeChannel.content.menuItemVideoCall.click();
await poHomeChannel.content.btnVideoCall.click();
await poHomeChannel.content.btnStartVideoCall.click();
await expect(poHomeChannel.content.videoConfMessageBlock.last()).toBeVisible();
});
Expand All @@ -98,8 +95,7 @@ test.describe('video conference', () => {
test('expect create video conference in a direct multiple', async () => {
await poHomeChannel.sidenav.openChat('rocketchat.internal.admin.test, user2');

await poHomeChannel.content.btnCall.click();
await poHomeChannel.content.menuItemVideoCall.click();
await poHomeChannel.content.btnVideoCall.click();
await poHomeChannel.content.btnStartVideoCall.click();
await expect(poHomeChannel.content.videoConfMessageBlock.last()).toBeVisible();
});
Expand All @@ -115,6 +111,6 @@ test.describe('video conference', () => {
test('expect create video conference not available in a "targetReadOnlyChannel"', async () => {
await poHomeChannel.sidenav.openChat(targetReadOnlyChannel);

await expect(poHomeChannel.content.btnCall).hasAttribute('disabled');
await expect(poHomeChannel.content.btnVideoCall).hasAttribute('disabled');
});
});
Loading