From c83ad1faa78ab818548f4775149269386a2f0a06 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 21 Apr 2022 07:41:38 -0400 Subject: [PATCH] Add local echo of connected devices in video rooms (#8368) --- src/components/views/rooms/RoomTile.tsx | 22 +++++--- src/components/views/voip/VideoLobby.tsx | 10 ++-- src/utils/VideoChannelUtils.ts | 38 ++++++++++---- test/components/views/rooms/RoomTile-test.tsx | 51 +++++++++++++++---- .../components/views/voip/VideoLobby-test.tsx | 45 ++++++++++++---- test/test-utils/test-utils.ts | 1 + test/test-utils/video.ts | 2 +- 7 files changed, 126 insertions(+), 43 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 530b22571a..7b0a8e95de 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -81,7 +81,7 @@ interface IState { messagePreview?: string; videoStatus: VideoStatus; // Active video channel members, according to room state - videoMembers: RoomMember[]; + videoMembers: Set; // Active video channel members, according to Jitsi jitsiParticipants: IJitsiParticipant[]; } @@ -124,7 +124,7 @@ export default class RoomTile extends React.PureComponent { // generatePreview() will return nothing if the user has previews disabled messagePreview: "", videoStatus, - videoMembers: getConnectedMembers(this.props.room.currentState), + videoMembers: getConnectedMembers(this.props.room, videoStatus === VideoStatus.Connected), jitsiParticipants: VideoChannelStore.instance.participants, }; this.generatePreview(); @@ -593,7 +593,9 @@ export default class RoomTile extends React.PureComponent { } private updateVideoMembers = () => { - this.setState({ videoMembers: getConnectedMembers(this.props.room.currentState) }); + this.setState(state => ({ + videoMembers: getConnectedMembers(this.props.room, state.videoStatus === VideoStatus.Connected), + })); }; private updateVideoStatus = () => { @@ -610,7 +612,10 @@ export default class RoomTile extends React.PureComponent { private onConnectVideo = (roomId: string) => { if (roomId === this.props.room?.roomId) { - this.setState({ videoStatus: VideoStatus.Connected }); + this.setState({ + videoStatus: VideoStatus.Connected, + videoMembers: getConnectedMembers(this.props.room, true), + }); VideoChannelStore.instance.on(VideoChannelEvent.Participants, this.updateJitsiParticipants); } }; @@ -623,7 +628,10 @@ export default class RoomTile extends React.PureComponent { private onDisconnectVideo = (roomId: string) => { if (roomId === this.props.room?.roomId) { - this.setState({ videoStatus: VideoStatus.Disconnected }); + this.setState({ + videoStatus: VideoStatus.Disconnected, + videoMembers: getConnectedMembers(this.props.room, false), + }); VideoChannelStore.instance.off(VideoChannelEvent.Participants, this.updateJitsiParticipants); } }; @@ -668,12 +676,12 @@ export default class RoomTile extends React.PureComponent { case VideoStatus.Disconnected: videoText = _t("Video"); videoActive = false; - participantCount = this.state.videoMembers.length; + participantCount = this.state.videoMembers.size; break; case VideoStatus.Connecting: videoText = _t("Connecting..."); videoActive = true; - participantCount = this.state.videoMembers.length; + participantCount = this.state.videoMembers.size; break; case VideoStatus.Connected: videoText = _t("Connected"); diff --git a/src/components/views/voip/VideoLobby.tsx b/src/components/views/voip/VideoLobby.tsx index 84bc470273..f9e9508927 100644 --- a/src/components/views/voip/VideoLobby.tsx +++ b/src/components/views/voip/VideoLobby.tsx @@ -110,7 +110,7 @@ const MAX_FACES = 8; const VideoLobby: FC<{ room: Room }> = ({ room }) => { const [connecting, setConnecting] = useState(false); const me = useMemo(() => room.getMember(room.myUserId), [room]); - const connectedMembers = useConnectedMembers(room.currentState); + const connectedMembers = useConnectedMembers(room, false); const videoRef = useRef(); const devices = useAsyncMemo(async () => { @@ -172,12 +172,12 @@ const VideoLobby: FC<{ room: Room }> = ({ room }) => { }; let facePile; - if (connectedMembers.length) { - const shownMembers = connectedMembers.slice(0, MAX_FACES); - const overflow = connectedMembers.length > shownMembers.length; + if (connectedMembers.size) { + const shownMembers = [...connectedMembers].slice(0, MAX_FACES); + const overflow = connectedMembers.size > shownMembers.length; facePile =
- { _t("%(count)s people connected", { count: connectedMembers.length }) } + { _t("%(count)s people connected", { count: connectedMembers.size }) }
; } diff --git a/src/utils/VideoChannelUtils.ts b/src/utils/VideoChannelUtils.ts index 11a1a9a35f..cc3c99d980 100644 --- a/src/utils/VideoChannelUtils.ts +++ b/src/utils/VideoChannelUtils.ts @@ -17,7 +17,8 @@ limitations under the License. import { useState } from "react"; import { throttle } from "lodash"; import { CallType } from "matrix-js-sdk/src/webrtc/call"; -import { RoomState, RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; +import { Room } from "matrix-js-sdk/src/models/room"; +import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import { useTypedEventEmitter } from "../hooks/useEventEmitter"; @@ -42,17 +43,32 @@ export const addVideoChannel = async (roomId: string, roomName: string) => { await WidgetUtils.addJitsiWidget(roomId, CallType.Video, "Video channel", VIDEO_CHANNEL, roomName); }; -export const getConnectedMembers = (state: RoomState): RoomMember[] => - state.getStateEvents(VIDEO_CHANNEL_MEMBER) - // Must have a device connected and still be joined to the room - .filter(e => e.getContent()?.devices?.length) - .map(e => state.getMember(e.getStateKey())) - .filter(member => member?.membership === "join"); +export const getConnectedMembers = (room: Room, connectedLocalEcho: boolean): Set => { + const members = new Set(); -export const useConnectedMembers = (state: RoomState, throttleMs = 100) => { - const [members, setMembers] = useState(getConnectedMembers(state)); - useTypedEventEmitter(state, RoomStateEvent.Update, throttle(() => { - setMembers(getConnectedMembers(state)); + for (const e of room.currentState.getStateEvents(VIDEO_CHANNEL_MEMBER)) { + const member = room.getMember(e.getStateKey()); + let devices = e.getContent()?.devices ?? []; + + // Apply local echo for the disconnected case + if (!connectedLocalEcho && member?.userId === room.client.getUserId()) { + devices = devices.filter(d => d !== room.client.getDeviceId()); + } + // Must have a device connected and still be joined to the room + if (devices.length && member?.membership === "join") members.add(member); + } + + // Apply local echo for the connected case + if (connectedLocalEcho) members.add(room.getMember(room.client.getUserId())); + return members; +}; + +export const useConnectedMembers = ( + room: Room, connectedLocalEcho: boolean, throttleMs = 100, +): Set => { + const [members, setMembers] = useState>(getConnectedMembers(room, connectedLocalEcho)); + useTypedEventEmitter(room.currentState, RoomStateEvent.Update, throttle(() => { + setMembers(getConnectedMembers(room, connectedLocalEcho)); }, throttleMs, { leading: true, trailing: true })); return members; }; diff --git a/test/components/views/rooms/RoomTile-test.tsx b/test/components/views/rooms/RoomTile-test.tsx index d209c32f0f..d07360f6d4 100644 --- a/test/components/views/rooms/RoomTile-test.tsx +++ b/test/components/views/rooms/RoomTile-test.tsx @@ -18,6 +18,8 @@ import React from "react"; import { mount } from "enzyme"; import { act } from "react-dom/test-utils"; import { mocked } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/client"; +import { Room } from "matrix-js-sdk/src/models/room"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import { @@ -26,6 +28,7 @@ import { mkRoom, mkVideoChannelMember, stubVideoChannelStore, + StubVideoChannelStore, } from "../../../test-utils"; import RoomTile from "../../../../src/components/views/rooms/RoomTile"; import SettingsStore from "../../../../src/settings/SettingsStore"; @@ -39,9 +42,8 @@ describe("RoomTile", () => { jest.spyOn(PlatformPeg, 'get') .mockReturnValue({ overrideBrowserShortcuts: () => false } as unknown as BasePlatform); - let cli; - let store; - + let cli: MatrixClient; + let store: StubVideoChannelStore; beforeEach(() => { const realGetValue = SettingsStore.getValue; SettingsStore.getValue = (name: string, roomId?: string): T => { @@ -52,7 +54,7 @@ describe("RoomTile", () => { }; stubClient(); - cli = mocked(MatrixClientPeg.get()); + cli = MatrixClientPeg.get(); store = stubVideoChannelStore(); DMRoomMap.makeShared(); }); @@ -60,8 +62,11 @@ describe("RoomTile", () => { afterEach(() => jest.clearAllMocks()); describe("video rooms", () => { - const room = mkRoom(cli, "!1:example.org"); - room.isElementVideoRoom.mockReturnValue(true); + let room: Room; + beforeEach(() => { + room = mkRoom(cli, "!1:example.org"); + mocked(room.isElementVideoRoom).mockReturnValue(true); + }); it("tracks connection state", () => { const tile = mount( @@ -97,7 +102,7 @@ describe("RoomTile", () => { mkVideoChannelMember("@chris:example.org", ["device 1"]), ])); - mocked(room.currentState).getMember.mockImplementation(userId => ({ + mocked(room).getMember.mockImplementation(userId => ({ userId, membership: userId === "@chris:example.org" ? "leave" : "join", name: userId, @@ -117,8 +122,36 @@ describe("RoomTile", () => { ); // Only Alice should display as connected - const participants = tile.find(".mx_RoomTile_videoParticipants"); - expect(participants.text()).toEqual("1"); + expect(tile.find(".mx_RoomTile_videoParticipants").text()).toEqual("1"); + }); + + it("reflects local echo in connected members", () => { + mocked(room.currentState).getStateEvents.mockImplementation(mockStateEventImplementation([ + // Make the remote echo claim that we're connected, while leaving the store disconnected + mkVideoChannelMember(cli.getUserId(), [cli.getDeviceId()]), + ])); + + mocked(room).getMember.mockImplementation(userId => ({ + userId, + membership: "join", + name: userId, + rawDisplayName: userId, + roomId: "!1:example.org", + getAvatarUrl: () => {}, + getMxcAvatarUrl: () => {}, + }) as unknown as RoomMember); + + const tile = mount( + , + ); + + // Because of our local echo, we should still appear as disconnected + expect(tile.find(".mx_RoomTile_videoParticipants").exists()).toEqual(false); }); }); }); diff --git a/test/components/views/voip/VideoLobby-test.tsx b/test/components/views/voip/VideoLobby-test.tsx index 4e7afb12c4..2d69709dc7 100644 --- a/test/components/views/voip/VideoLobby-test.tsx +++ b/test/components/views/voip/VideoLobby-test.tsx @@ -18,11 +18,14 @@ import React from "react"; import { mount } from "enzyme"; import { act } from "react-dom/test-utils"; import { mocked } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/client"; +import { Room } from "matrix-js-sdk/src/models/room"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import { stubClient, stubVideoChannelStore, + StubVideoChannelStore, mkRoom, mkVideoChannelMember, mockStateEventImplementation, @@ -33,7 +36,6 @@ import MemberAvatar from "../../../../src/components/views/avatars/MemberAvatar" import VideoLobby from "../../../../src/components/views/voip/VideoLobby"; describe("VideoLobby", () => { - stubClient(); Object.defineProperty(navigator, "mediaDevices", { value: { enumerateDevices: jest.fn(), @@ -42,19 +44,17 @@ describe("VideoLobby", () => { }); jest.spyOn(HTMLMediaElement.prototype, "play").mockImplementation(async () => {}); - const cli = MatrixClientPeg.get(); - const room = mkRoom(cli, "!1:example.org"); - - let store; + let cli: MatrixClient; + let store: StubVideoChannelStore; + let room: Room; beforeEach(() => { + stubClient(); + cli = MatrixClientPeg.get(); store = stubVideoChannelStore(); + room = mkRoom(cli, "!1:example.org"); mocked(navigator.mediaDevices.enumerateDevices).mockResolvedValue([]); }); - afterEach(() => { - jest.clearAllMocks(); - }); - describe("connected members", () => { it("hides when no one is connected", async () => { const lobby = mount(); @@ -75,7 +75,7 @@ describe("VideoLobby", () => { mkVideoChannelMember("@chris:example.org", ["device 1"]), ])); - mocked(room.currentState).getMember.mockImplementation(userId => ({ + mocked(room).getMember.mockImplementation(userId => ({ userId, membership: userId === "@chris:example.org" ? "leave" : "join", name: userId, @@ -95,6 +95,31 @@ describe("VideoLobby", () => { expect(memberText).toEqual("1 person connected"); expect(lobby.find(FacePile).find(MemberAvatar).props().member.userId).toEqual("@alice:example.org"); }); + + it("doesn't include remote echo of this device being connected", async () => { + mocked(room.currentState).getStateEvents.mockImplementation(mockStateEventImplementation([ + // Make the remote echo claim that we're connected, while leaving the store disconnected + mkVideoChannelMember(cli.getUserId(), [cli.getDeviceId()]), + ])); + + mocked(room).getMember.mockImplementation(userId => ({ + userId, + membership: "join", + name: userId, + rawDisplayName: userId, + roomId: "!1:example.org", + getAvatarUrl: () => {}, + getMxcAvatarUrl: () => {}, + }) as unknown as RoomMember); + + const lobby = mount(); + // Wait for state to settle + await act(() => Promise.resolve()); + lobby.update(); + + // Because of our local echo, we should still appear as disconnected + expect(lobby.find(".mx_VideoLobby_connectedMembers").exists()).toEqual(false); + }); }); describe("device buttons", () => { diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index fc85a825f3..a590474ffe 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -379,6 +379,7 @@ export function mkStubRoom(roomId: string = null, name: string, client: MatrixCl getJoinRule: jest.fn().mockReturnValue("invite"), loadMembersIfNeeded: jest.fn(), client, + myUserId: client?.getUserId(), canInvite: jest.fn(), } as unknown as Room; } diff --git a/test/test-utils/video.ts b/test/test-utils/video.ts index 79c657a0c6..77fdfb8fcc 100644 --- a/test/test-utils/video.ts +++ b/test/test-utils/video.ts @@ -21,7 +21,7 @@ import { mkEvent } from "./test-utils"; import { VIDEO_CHANNEL_MEMBER } from "../../src/utils/VideoChannelUtils"; import VideoChannelStore, { VideoChannelEvent, IJitsiParticipant } from "../../src/stores/VideoChannelStore"; -class StubVideoChannelStore extends EventEmitter { +export class StubVideoChannelStore extends EventEmitter { private _roomId: string; public get roomId(): string { return this._roomId; } private _connected: boolean;