From 9176e06195a11996026b54d186eda4352cb8b136 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 21 Nov 2024 11:01:43 +0000 Subject: [PATCH 1/3] Some tsdoc and explicit typing (#2809) * Some tsdoc and explicit typing Pulled out of https://github.com/element-hq/element-call/pull/2701 * Extra typing --- src/state/CallViewModel.ts | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 05bf3616..58f8a389 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -341,12 +341,16 @@ export class CallViewModel extends ViewModel { }), ); - private readonly rawRemoteParticipants = connectedParticipantsObserver( - this.livekitRoom, - ).pipe(this.scope.state()); + /** + * The raw list of RemoteParticipants as reported by LiveKit + */ + private readonly rawRemoteParticipants: Observable = + connectedParticipantsObserver(this.livekitRoom).pipe(this.scope.state()); - // Lists of participants to "hold" on display, even if LiveKit claims that - // they've left + /** + * Lists of RemoteParticipants to "hold" on display, even if LiveKit claims that + * they've left + */ private readonly remoteParticipantHolds: Observable = this.connectionState.pipe( withLatestFrom(this.rawRemoteParticipants), @@ -381,6 +385,9 @@ export class CallViewModel extends ViewModel { ), ); + /** + * The RemoteParticipants including those that are being "held" on the screen + */ private readonly remoteParticipants: Observable = combineLatest( [this.rawRemoteParticipants, this.remoteParticipantHolds], @@ -402,6 +409,9 @@ export class CallViewModel extends ViewModel { }, ); + /** + * List of MediaItems that we want to display + */ private readonly mediaItems: Observable = combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), @@ -471,6 +481,9 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); + /** + * List of MediaItems that we want to display, that are of type UserMedia + */ private readonly userMedia: Observable = this.mediaItems.pipe( map((mediaItems) => mediaItems.filter((m): m is UserMedia => m instanceof UserMedia), @@ -482,6 +495,9 @@ export class CallViewModel extends ViewModel { map((ms) => ms.find((m) => m.vm.local)!.vm as LocalUserMediaViewModel), ); + /** + * List of MediaItems that we want to display, that are of type ScreenShare + */ private readonly screenShares: Observable = this.mediaItems.pipe( map((mediaItems) => @@ -940,7 +956,7 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - public readonly showFooter = this.windowMode.pipe( + public readonly showFooter: Observable = this.windowMode.pipe( switchMap((mode) => { switch (mode) { case "pip": From 3885eefa4c77e7788b6292f4a512fb4e2589fe19 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 21 Nov 2024 11:02:05 +0000 Subject: [PATCH 2/3] Disambiguate between types of "member" (#2807) We have Matrix room members and MatrixRTC session memberships. Livekit also has rooms. So, this attempts to make it more obvious as to what type you are referring to. --- src/state/CallViewModel.test.ts | 10 +++++----- src/state/CallViewModel.ts | 4 ++-- src/utils/test.ts | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 2bfab9b4..0c9c663d 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -30,7 +30,7 @@ import { mockLivekitRoom, mockLocalParticipant, mockMatrixRoom, - mockMember, + mockMatrixRoomMember, mockRemoteParticipant, withTestScheduler, } from "../utils/test"; @@ -42,10 +42,10 @@ import { E2eeType } from "../e2ee/e2eeType"; vi.mock("@livekit/components-core"); -const alice = mockMember({ userId: "@alice:example.org" }); -const bob = mockMember({ userId: "@bob:example.org" }); -const carol = mockMember({ userId: "@carol:example.org" }); -const dave = mockMember({ userId: "@dave:example.org" }); +const alice = mockMatrixRoomMember({ userId: "@alice:example.org" }); +const bob = mockMatrixRoomMember({ userId: "@bob:example.org" }); +const carol = mockMatrixRoomMember({ userId: "@carol:example.org" }); +const dave = mockMatrixRoomMember({ userId: "@dave:example.org" }); const aliceId = `${alice.userId}:AAAA`; const bobId = `${bob.userId}:BBBB`; diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 58f8a389..8999dc89 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -306,7 +306,7 @@ class ScreenShare { type MediaItem = UserMedia | ScreenShare; -function findMatrixMember( +function findMatrixRoomMember( room: MatrixRoom, id: string, ): RoomMember | undefined { @@ -428,7 +428,7 @@ export class CallViewModel extends ViewModel { function* (this: CallViewModel): Iterable<[string, MediaItem]> { for (const p of [localParticipant, ...remoteParticipants]) { const id = p === localParticipant ? "local" : p.identity; - const member = findMatrixMember(this.matrixRoom, id); + const member = findMatrixRoomMember(this.matrixRoom, id); if (member === undefined) logger.warn( `Ruh, roh! No matrix member found for SFU participant '${p.identity}': creating g-g-g-ghost!`, diff --git a/src/utils/test.ts b/src/utils/test.ts index 771dd574..5988dd6f 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -99,7 +99,7 @@ function mockEmitter(): EmitterMock { // Maybe it'd be good to move this to matrix-js-sdk? Our testing needs are // rather simple, but if one util to mock a member is good enough for us, maybe // it's useful for matrix-js-sdk consumers in general. -export function mockMember(member: Partial): RoomMember { +export function mockMatrixRoomMember(member: Partial): RoomMember { return { ...mockEmitter(), ...member } as RoomMember; } @@ -149,7 +149,7 @@ export async function withLocalMedia( const localParticipant = mockLocalParticipant({}); const vm = new LocalUserMediaViewModel( "local", - mockMember(member), + mockMatrixRoomMember(member), localParticipant, { kind: E2eeType.PER_PARTICIPANT, @@ -184,7 +184,7 @@ export async function withRemoteMedia( const remoteParticipant = mockRemoteParticipant(participant); const vm = new RemoteUserMediaViewModel( "remote", - mockMember(member), + mockMatrixRoomMember(member), remoteParticipant, { kind: E2eeType.PER_PARTICIPANT, From b7b97715776a851dac08abdb6d6dc04742d53791 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 21 Nov 2024 11:03:16 +0000 Subject: [PATCH 3/3] Use hot test input marbles instead of cold (#2810) * Use hot test input marbles instead of cold These will be needed for https://github.com/element-hq/element-call/pull/2701 * Revert for "spotlight speakers swap places" test --- src/state/CallViewModel.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 0c9c663d..aa49f048 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -229,7 +229,7 @@ function withCallViewModel( } test("participants are retained during a focus switch", () => { - withTestScheduler(({ cold, expectObservable }) => { + withTestScheduler(({ hot, expectObservable }) => { // Participants disappear on frame 2 and come back on frame 3 const participantInputMarbles = "a-ba"; // Start switching focus on frame 1 and reconnect on frame 3 @@ -238,11 +238,11 @@ test("participants are retained during a focus switch", () => { const expectedLayoutMarbles = " a"; withCallViewModel( - cold(participantInputMarbles, { + hot(participantInputMarbles, { a: [aliceParticipant, bobParticipant], b: [], }), - cold(connectionInputMarbles, { + hot(connectionInputMarbles, { c: ConnectionState.Connected, s: ECAddonConnectionState.ECSwitchingFocus, }), @@ -264,7 +264,7 @@ test("participants are retained during a focus switch", () => { }); test("screen sharing activates spotlight layout", () => { - withTestScheduler(({ cold, schedule, expectObservable }) => { + withTestScheduler(({ hot, schedule, expectObservable }) => { // Start with no screen shares, then have Alice and Bob share their screens, // then return to no screen shares, then have just Alice share for a bit const participantInputMarbles = " abcda-ba"; @@ -277,7 +277,7 @@ test("screen sharing activates spotlight layout", () => { const expectedLayoutMarbles = " abcdaefeg"; const expectedShowSpeakingMarbles = "y----nyny"; withCallViewModel( - cold(participantInputMarbles, { + hot(participantInputMarbles, { a: [aliceParticipant, bobParticipant], b: [aliceSharingScreen, bobParticipant], c: [aliceSharingScreen, bobSharingScreen], @@ -347,7 +347,7 @@ test("screen sharing activates spotlight layout", () => { }); test("participants stay in the same order unless to appear/disappear", () => { - withTestScheduler(({ cold, schedule, expectObservable }) => { + withTestScheduler(({ hot, schedule, expectObservable }) => { const modeInputMarbles = " a"; // First Bob speaks, then Dave, then Alice const aSpeakingInputMarbles = "n- 1998ms - 1999ms y"; @@ -363,9 +363,9 @@ test("participants stay in the same order unless to appear/disappear", () => { of([aliceParticipant, bobParticipant, daveParticipant]), of(ConnectionState.Connected), new Map([ - [aliceParticipant, cold(aSpeakingInputMarbles, { y: true, n: false })], - [bobParticipant, cold(bSpeakingInputMarbles, { y: true, n: false })], - [daveParticipant, cold(dSpeakingInputMarbles, { y: true, n: false })], + [aliceParticipant, hot(aSpeakingInputMarbles, { y: true, n: false })], + [bobParticipant, hot(bSpeakingInputMarbles, { y: true, n: false })], + [daveParticipant, hot(dSpeakingInputMarbles, { y: true, n: false })], ]), (vm) => { schedule(modeInputMarbles, {