From 232e3f7ad47942849fdc193ee147f32b8fda74ac Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:28:14 +0200 Subject: [PATCH] Extract the spotlight speaker choice into a pure function --- src/state/CallViewModel/CallViewModel.ts | 26 ++-------- src/state/layoutMedia.test.ts | 62 ++++++++++++++++++++++++ src/state/layoutMedia.ts | 33 +++++++++++++ 3 files changed, 100 insertions(+), 21 deletions(-) create mode 100644 src/state/layoutMedia.test.ts create mode 100644 src/state/layoutMedia.ts diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 831dd0b0e..3e92c4e8e 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -104,6 +104,7 @@ import { type SpotlightPortraitLayoutMedia, type WindowMode, } from "../layout-types.ts"; +import { chooseSpotlightSpeaker } from "../layoutMedia.ts"; import { ElementCallError, UnknownCallError } from "../../utils/errors.ts"; import { type Epoch, type ObservableScope } from "../ObservableScope.ts"; import { createHomeserverConnected$ } from "./localMember/HomeserverConnected.ts"; @@ -975,33 +976,16 @@ export function createCallViewModel$( mediaItems.length === 0 ? of([]) : combineLatest( - mediaItems.map((m) => - m.speaking$.pipe(map((s) => [m, s] as const)), + mediaItems.map((media) => + media.speaking$.pipe(map((speaking) => ({ media, speaking }))), ), ), ), scan< - (readonly [UserMediaViewModel, boolean])[], + { media: UserMediaViewModel; speaking: boolean }[], UserMediaViewModel | undefined, undefined - >((prev, mediaItems) => { - // Only remote users that are still in the call should be sticky - const [stickyMedia, stickySpeaking] = - (!prev?.local && mediaItems.find(([m]) => m === prev)) || []; - // Decide who to spotlight: - // If the previous speaker is still speaking, stick with them rather - // than switching eagerly to someone else - return stickySpeaking - ? stickyMedia! - : // Otherwise, select any remote user who is speaking - (mediaItems.find(([m, s]) => !m.local && s)?.[0] ?? - // Otherwise, stick with the person who was last speaking - stickyMedia ?? - // Otherwise, spotlight an arbitrary remote user - mediaItems.find(([m]) => !m.local)?.[0] ?? - // Otherwise, spotlight the local user - mediaItems.find(([m]) => m.local)?.[0]); - }, undefined), + >(chooseSpotlightSpeaker, undefined), ), ); diff --git a/src/state/layoutMedia.test.ts b/src/state/layoutMedia.test.ts new file mode 100644 index 000000000..6936b1143 --- /dev/null +++ b/src/state/layoutMedia.test.ts @@ -0,0 +1,62 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { describe, expect, it } from "vitest"; + +import { chooseSpotlightSpeaker } from "./layoutMedia"; +import { type LocalUserMediaViewModel } from "./media/LocalUserMediaViewModel"; +import { type RemoteUserMediaViewModel } from "./media/RemoteUserMediaViewModel"; + +// The layout functions only look at `type` and `local`, so stubs suffice +const local = { type: "user", local: true } as LocalUserMediaViewModel; +const alice = { type: "user", local: false } as RemoteUserMediaViewModel; +const bob = { type: "user", local: false } as RemoteUserMediaViewModel; +const carol = { type: "user", local: false } as RemoteUserMediaViewModel; + +describe("chooseSpotlightSpeaker", () => { + const items = ( + ...speaking: boolean[] + ): { + media: RemoteUserMediaViewModel | LocalUserMediaViewModel; + speaking: boolean; + }[] => + [local, alice, bob].map((media, i) => ({ + media, + speaking: speaking[i] ?? false, + })); + + it("prefers a remote speaker over the local user", () => { + expect(chooseSpotlightSpeaker(undefined, items(true, false, true))).toBe( + bob, + ); + }); + + it("falls back to any remote user, then the local user", () => { + expect(chooseSpotlightSpeaker(undefined, items())).toBe(alice); + expect( + chooseSpotlightSpeaker(undefined, [{ media: local, speaking: false }]), + ).toBe(local); + expect(chooseSpotlightSpeaker(undefined, [])).toBeUndefined(); + }); + + it("sticks with the previous speaker while they keep speaking", () => { + expect(chooseSpotlightSpeaker(bob, items(false, true, true))).toBe(bob); + }); + + it("sticks with the previous speaker when nobody is speaking", () => { + expect(chooseSpotlightSpeaker(bob, items())).toBe(bob); + }); + + it("switches when someone else speaks and the previous speaker is silent", () => { + expect(chooseSpotlightSpeaker(bob, items(false, true, false))).toBe(alice); + }); + + it("forgets a previous speaker who left, and never sticks to the local user", () => { + expect(chooseSpotlightSpeaker(carol, items())).toBe(alice); + expect(chooseSpotlightSpeaker(local, items())).toBe(alice); + }); +}); diff --git a/src/state/layoutMedia.ts b/src/state/layoutMedia.ts new file mode 100644 index 000000000..3995f0216 --- /dev/null +++ b/src/state/layoutMedia.ts @@ -0,0 +1,33 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +/** + * Picks who to spotlight based on who is speaking, preferring the previous + * pick so that the spotlight doesn't flip eagerly between speakers. + * + * @param prev The previous pick, if any. + * @param items All user media in the call along with whether each is speaking. + */ +export function chooseSpotlightSpeaker( + prev: M | undefined, + items: readonly { media: M; speaking: boolean }[], +): M | undefined { + // Only remote users that are still in the call should be sticky + const sticky = items.find((i) => i.media === prev && !i.media.local); + // If the previous speaker is still speaking, stick with them + if (sticky?.speaking) return sticky.media; + return ( + // Otherwise, select any remote user who is speaking + items.find((i) => !i.media.local && i.speaking)?.media ?? + // Otherwise, stick with the person who was last speaking + sticky?.media ?? + // Otherwise, spotlight an arbitrary remote user + items.find((i) => !i.media.local)?.media ?? + // Otherwise, spotlight the local user + items.find((i) => i.media.local)?.media + ); +}