From e2607d6399c29a97744cb8c071271202b681e711 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 25 Nov 2025 11:10:53 +0100 Subject: [PATCH 01/28] Config: UrlParams to control noiseSuppression and echoCancellation --- src/UrlParams.test.ts | 35 +++++ src/UrlParams.ts | 13 ++ src/state/CallViewModel/CallViewModel.ts | 2 + .../remoteMembers/ConnectionFactory.ts | 12 +- .../remoteMembers/ECConnectionFactory.test.ts | 134 ++++++++++++++++++ 5 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index cd8fc6d5..cd195f54 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -332,6 +332,41 @@ describe("UrlParams", () => { expect(computeUrlParams("?intent=join_existing").skipLobby).toBe(false); }); }); + + describe("noiseSuppression", () => { + it("defaults to true", () => { + expect(computeUrlParams().noiseSuppression).toBe(true); + }); + + it("is parsed", () => { + expect(computeUrlParams("?intent=start_call&noiseSuppression=true").noiseSuppression).toBe( + true, + ); + expect(computeUrlParams("?intent=start_call&noiseSuppression&bar=foo").noiseSuppression).toBe( + true, + ); + expect(computeUrlParams("?noiseSuppression=false").noiseSuppression).toBe( + false, + ); + }); + }); + + + describe("echoCancellation", () => { + it("defaults to true", () => { + expect(computeUrlParams().echoCancellation).toBe(true); + }); + + it("is parsed", () => { + expect(computeUrlParams("?echoCancellation=true").echoCancellation).toBe( + true, + ); + expect(computeUrlParams("?echoCancellation=false").echoCancellation).toBe( + false, + ); + }); + }); + describe("header", () => { it("uses header if provided", () => { expect(computeUrlParams("?header=app_bar&hideHeader=true").header).toBe( diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 4eb69298..f78841fb 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -233,6 +233,17 @@ export interface UrlConfiguration { */ waitForCallPickup: boolean; + /** + * Whether to enable echo cancellation for audio capture. + * Defaults to true. + */ + echoCancellation?: boolean; + /** + * Whether to enable noise suppression for audio capture. + * Defaults to true. + */ + noiseSuppression?: boolean; + callIntent?: RTCCallIntent; } interface IntentAndPlatformDerivedConfiguration { @@ -525,6 +536,8 @@ export const computeUrlParams = (search = "", hash = ""): UrlParams => { ]), waitForCallPickup: parser.getFlag("waitForCallPickup"), autoLeaveWhenOthersLeft: parser.getFlag("autoLeave"), + noiseSuppression: parser.getFlagParam("noiseSuppression", true), + echoCancellation: parser.getFlagParam("echoCancellation", true), }; // Log the final configuration for debugging purposes. diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 506eca1b..b4df2738 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -415,6 +415,8 @@ export function createCallViewModel$( livekitKeyProvider, getUrlParams().controlledAudioDevices, options.livekitRoomFactory, + getUrlParams().echoCancellation, + getUrlParams().noiseSuppression, ); const connectionManager = createConnectionManager$({ diff --git a/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts b/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts index f58fcb76..c3a68c54 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts @@ -41,8 +41,10 @@ export class ECConnectionFactory implements ConnectionFactory { * @param client - The OpenID client parts for authentication, needed to get openID and JWT tokens. * @param devices - Used for video/audio out/in capture options. * @param processorState$ - Effects like background blur (only for publishing connection?) - * @param e2eeLivekitOptions - The E2EE options to use for the LiveKit Room. + * @param livekitKeyProvider * @param controlledAudioDevices - Option to indicate whether audio output device is controlled externally (native mobile app). + * @param echoCancellation - Whether to enable echo cancellation for audio capture. + * @param noiseSuppression - Whether to enable noise suppression for audio capture. * @param livekitRoomFactory - Optional factory function (for testing) to create LivekitRoom instances. If not provided, a default factory is used. */ public constructor( @@ -52,6 +54,8 @@ export class ECConnectionFactory implements ConnectionFactory { livekitKeyProvider: BaseKeyProvider | undefined, private controlledAudioDevices: boolean, livekitRoomFactory?: () => LivekitRoom, + echoCancellation: boolean = true, + noiseSuppression: boolean = true, ) { const defaultFactory = (): LivekitRoom => new LivekitRoom( @@ -65,6 +69,8 @@ export class ECConnectionFactory implements ConnectionFactory { worker: new E2EEWorker(), }, this.controlledAudioDevices, + echoCancellation, + noiseSuppression, ), ); this.livekitRoomFactory = livekitRoomFactory ?? defaultFactory; @@ -95,6 +101,8 @@ function generateRoomOption( processorState: ProcessorState, e2eeLivekitOptions: E2EEOptions | undefined, controlledAudioDevices: boolean, + echoCancellation: boolean, + noiseSuppression: boolean, ): RoomOptions { return { ...defaultLiveKitOptions, @@ -106,6 +114,8 @@ function generateRoomOption( audioCaptureDefaults: { ...defaultLiveKitOptions.audioCaptureDefaults, deviceId: devices.audioInput.selected$.value?.id, + echoCancellation, + noiseSuppression, }, audioOutput: { // When using controlled audio devices, we don't want to set the diff --git a/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts b/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts new file mode 100644 index 00000000..cbf334be --- /dev/null +++ b/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts @@ -0,0 +1,134 @@ +/* +Copyright 2025 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 { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { Room as LivekitRoom } from "livekit-client"; +import { BehaviorSubject } from "rxjs"; +import fetchMock from "fetch-mock"; +import { logger } from "matrix-js-sdk/lib/logger"; +import EventEmitter from "events"; + +import { ObservableScope } from "../../ObservableScope.ts"; +import { ECConnectionFactory } from "./ConnectionFactory.ts"; +import type { OpenIDClientParts } from "../../../livekit/openIDSFU.ts"; +import { exampleTransport, mockMediaDevices } from "../../../utils/test.ts"; +import type { ProcessorState } from "../../../livekit/TrackProcessorContext.tsx"; +import { constant } from "../../Behavior"; + +// At the top of your test file, after imports +vi.mock("livekit-client", async () => { + const actual = await vi.importActual("livekit-client"); + return { + ...actual, + Room: vi.fn().mockImplementation(function (this: LivekitRoom, options) { + const emitter = new EventEmitter(); + return { + on: emitter.on.bind(emitter), + off: emitter.off.bind(emitter), + emit: emitter.emit.bind(emitter), + disconnect: vi.fn(), + remoteParticipants: new Map(), + } as unknown as LivekitRoom; + }), + }; +}); + +let testScope: ObservableScope; +let mockClient: OpenIDClientParts; + +beforeEach(() => { + testScope = new ObservableScope(); + mockClient = { + getOpenIdToken: vi.fn().mockReturnValue(""), + getDeviceId: vi.fn().mockReturnValue("DEV000"), + }; +}); + +describe("ECConnectionFactory - Audio inputs options", () => { + test.each([ + { echo: true, noise: true }, + { echo: true, noise: false }, + { echo: false, noise: true }, + { echo: false, noise: false }, + ])( + "it sets echoCancellation=$echo and noiseSuppression=$noise based on constructor parameters", + ({ echo, noise }) => { + // test("it sets echoCancellation and noiseSuppression based on constructor parameters", () => { + const RoomConstructor = vi.mocked(LivekitRoom); + + const ecConnectionFactory = new ECConnectionFactory( + mockClient, + mockMediaDevices({}), + new BehaviorSubject({ + supported: true, + processor: undefined, + }), + undefined, + false, + undefined, + echo, + noise, + ); + ecConnectionFactory.createConnection(exampleTransport, testScope, logger); + + // Check if Room was constructed with expected options + expect(RoomConstructor).toHaveBeenCalledWith( + expect.objectContaining({ + audioCaptureDefaults: expect.objectContaining({ + echoCancellation: echo, + noiseSuppression: noise, + }), + }), + ); + }, + ); +}); + +describe("ECConnectionFactory - ControlledAudioDevice", () => { + test.each([{ controlled: true }, { controlled: false }])( + "it sets controlledAudioDevice=$controlled then uses deviceId accordingly", + ({ controlled }) => { + // test("it sets echoCancellation and noiseSuppression based on constructor parameters", () => { + const RoomConstructor = vi.mocked(LivekitRoom); + + const ecConnectionFactory = new ECConnectionFactory( + mockClient, + mockMediaDevices({ + audioOutput: { + available$: constant(new Map()), + selected$: constant({ id: "DEV00", virtualEarpiece: false }), + select: () => {}, + } + }), + new BehaviorSubject({ + supported: true, + processor: undefined, + }), + undefined, + controlled, + undefined, + false, + false, + ); + ecConnectionFactory.createConnection(exampleTransport, testScope, logger); + + // Check if Room was constructed with expected options + expect(RoomConstructor).toHaveBeenCalledWith( + expect.objectContaining({ + audioOutput: expect.objectContaining({ + deviceId: controlled ? undefined : "DEV00", + }), + }), + ); + }, + ); +}); + +afterEach(() => { + testScope.end(); + fetchMock.reset(); +}); From 7f3596845c4c14a801aeb6557bab6a01a56918e7 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 25 Nov 2025 11:40:38 +0100 Subject: [PATCH 02/28] fix formatting --- src/UrlParams.test.ts | 15 ++++++++------- .../remoteMembers/ECConnectionFactory.test.ts | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index cd195f54..faba394f 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -339,19 +339,20 @@ describe("UrlParams", () => { }); it("is parsed", () => { - expect(computeUrlParams("?intent=start_call&noiseSuppression=true").noiseSuppression).toBe( - true, - ); - expect(computeUrlParams("?intent=start_call&noiseSuppression&bar=foo").noiseSuppression).toBe( - true, - ); + expect( + computeUrlParams("?intent=start_call&noiseSuppression=true") + .noiseSuppression, + ).toBe(true); + expect( + computeUrlParams("?intent=start_call&noiseSuppression&bar=foo") + .noiseSuppression, + ).toBe(true); expect(computeUrlParams("?noiseSuppression=false").noiseSuppression).toBe( false, ); }); }); - describe("echoCancellation", () => { it("defaults to true", () => { expect(computeUrlParams().echoCancellation).toBe(true); diff --git a/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts b/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts index cbf334be..78e23057 100644 --- a/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts +++ b/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts @@ -102,7 +102,7 @@ describe("ECConnectionFactory - ControlledAudioDevice", () => { available$: constant(new Map()), selected$: constant({ id: "DEV00", virtualEarpiece: false }), select: () => {}, - } + }, }), new BehaviorSubject({ supported: true, From fc39e82666182e273a40a7c3bc0a10b7bdf9267c Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 27 Nov 2025 15:52:25 +0100 Subject: [PATCH 03/28] Fix: Camera is not muted when the earpiece mode is enabled --- src/state/MuteStates.test.ts | 190 +++++++++++++++++++++++++++++++++++ src/state/MuteStates.ts | 58 ++++++++++- 2 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 src/state/MuteStates.test.ts diff --git a/src/state/MuteStates.test.ts b/src/state/MuteStates.test.ts new file mode 100644 index 00000000..7b02d190 --- /dev/null +++ b/src/state/MuteStates.test.ts @@ -0,0 +1,190 @@ +/* +Copyright 2025 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 { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { BehaviorSubject } from "rxjs"; +import { logger } from "matrix-js-sdk/lib/logger"; + +import { MuteStates, MuteState } from "./MuteStates"; +import { + type AudioOutputDeviceLabel, + type DeviceLabel, + type MediaDevice, + type SelectedAudioOutputDevice, + type SelectedDevice, +} from "./MediaDevices"; +import { constant } from "./Behavior"; +import { ObservableScope } from "./ObservableScope"; +import { flushPromises, mockMediaDevices } from "../utils/test"; + +const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); +vi.mock("../UrlParams", () => ({ getUrlParams })); + +let testScope: ObservableScope; + +beforeEach(() => { + testScope = new ObservableScope(); +}); + +afterEach(() => { + testScope.end(); +}); + +describe("MuteState", () => { + test("should automatically mute if force mute is set", async () => { + const forceMute$ = new BehaviorSubject(false); + + const deviceStub = { + available$: constant( + new Map([ + ["fbac11", { type: "name", name: "HD Camera" }], + ]), + ), + selected$: constant({ id: "fbac11" }), + select(): void {}, + } as unknown as MediaDevice; + + const muteState = new MuteState( + testScope, + deviceStub, + constant(true), + true, + forceMute$, + ); + let lastEnabled: boolean = false; + muteState.enabled$.subscribe((enabled) => { + lastEnabled = enabled; + }); + let setEnabled: ((enabled: boolean) => void) | null = null; + muteState.setEnabled$.subscribe((setter) => { + setEnabled = setter; + }); + + await flushPromises(); + + setEnabled!(true); + await flushPromises(); + expect(lastEnabled).toBe(true); + + // Now force mute + forceMute$.next(true); + await flushPromises(); + // Should automatically mute + expect(lastEnabled).toBe(false); + + // Try to unmute can not work + expect(setEnabled).toBeNull(); + + // Disable force mute + forceMute$.next(false); + await flushPromises(); + + // TODO I'd expect it to go back to previous state (enabled) + // but actually it goes back to the initial state from construction (disabled) + // Should go back to previous state (enabled) + // Skip for now + // expect(lastEnabled).toBe(true); + + // But yet it can be unmuted now + expect(setEnabled).not.toBeNull(); + + setEnabled!(true); + await flushPromises(); + expect(lastEnabled).toBe(true); + }); +}); + +describe("MuteStates", () => { + function aAudioOutputDevices(): MediaDevice< + AudioOutputDeviceLabel, + SelectedAudioOutputDevice + > { + const selected$ = new BehaviorSubject< + SelectedAudioOutputDevice | undefined + >({ + id: "default", + virtualEarpiece: false, + }); + return { + available$: constant( + new Map([ + ["default", { type: "speaker" }], + ["0000", { type: "speaker" }], + ["1111", { type: "earpiece" }], + ["222", { type: "name", name: "Bluetooth Speaker" }], + ]), + ), + selected$, + select(id: string): void { + if (!this.available$.getValue().has(id)) { + logger.warn(`Attempted to select unknown device id: ${id}`); + return; + } + selected$.next({ + id, + /** For test purposes we ignore this */ + virtualEarpiece: false, + }); + }, + }; + } + + function aVideoInput(): MediaDevice { + const selected$ = new BehaviorSubject( + undefined, + ); + return { + available$: constant( + new Map([ + ["0000", { type: "name", name: "HD Camera" }], + ["1111", { type: "name", name: "WebCam Pro" }], + ]), + ), + selected$, + select(id: string): void { + if (!this.available$.getValue().has(id)) { + logger.warn(`Attempted to select unknown device id: ${id}`); + return; + } + selected$.next({ id }); + }, + }; + } + + test("should mute camera when in earpiece mode", async () => { + const audioOutputDevice = aAudioOutputDevices(); + + const mediaDevices = mockMediaDevices({ + audioOutput: audioOutputDevice, + videoInput: aVideoInput(), + // other devices are not relevant for this test + }); + const muteStates = new MuteStates( + testScope, + mediaDevices, + // consider joined + constant(true), + ); + + let lastVideoEnabled: boolean = false; + muteStates.video.enabled$.subscribe((enabled) => { + lastVideoEnabled = enabled; + }); + + expect(muteStates.video.setEnabled$.value).toBeDefined(); + muteStates.video.setEnabled$.value?.(true); + await flushPromises(); + + expect(lastVideoEnabled).toBe(true); + + // Select earpiece audio output + audioOutputDevice.select("1111"); + await flushPromises(); + // Video should be automatically muted + expect(lastVideoEnabled).toBe(false); + }); +}); diff --git a/src/state/MuteStates.ts b/src/state/MuteStates.ts index 50be5e05..777e3aa4 100644 --- a/src/state/MuteStates.ts +++ b/src/state/MuteStates.ts @@ -27,7 +27,7 @@ import { ElementWidgetActions, widget } from "../widget"; import { Config } from "../config/Config"; import { getUrlParams } from "../UrlParams"; import { type ObservableScope } from "./ObservableScope"; -import { type Behavior } from "./Behavior"; +import { type Behavior, constant } from "./Behavior"; interface MuteStateData { enabled$: Observable; @@ -38,31 +38,55 @@ interface MuteStateData { export type Handler = (desired: boolean) => Promise; const defaultHandler: Handler = async (desired) => Promise.resolve(desired); -class MuteState { +/** + * Internal class - exported only for testing purposes. + * Do not use directly outside of tests. + */ +export class MuteState { + // TODO: rewrite this to explain behavior, it is not understandable, and cannot add logging private readonly enabledByDefault$ = this.enabledByConfig && !getUrlParams().skipLobby ? this.joined$.pipe(map((isJoined) => !isJoined)) : of(false); private readonly handler$ = new BehaviorSubject(defaultHandler); + public setHandler(handler: Handler): void { if (this.handler$.value !== defaultHandler) throw new Error("Multiple mute state handlers are not supported"); this.handler$.next(handler); } + public unsetHandler(): void { this.handler$.next(defaultHandler); } + private readonly devicesConnected$ = combineLatest([ + this.device.available$, + this.forceMute$, + ]).pipe( + map(([available, forceMute]) => { + return !forceMute && available.size > 0; + }), + ); + private readonly data$ = this.scope.behavior( - this.device.available$.pipe( - map((available) => available.size > 0), + this.devicesConnected$.pipe( + // this.device.available$.pipe( + // map((available) => available.size > 0), distinctUntilChanged(), withLatestFrom( this.enabledByDefault$, (devicesConnected, enabledByDefault) => { - if (!devicesConnected) + logger.info( + `MuteState: devices connected: ${devicesConnected}, enabled by default: ${enabledByDefault}`, + ); + if (!devicesConnected) { + logger.info( + `MuteState: devices connected: ${devicesConnected}, disabling`, + ); return { enabled$: of(false), set: null, toggle: null }; + } // Assume the default value only once devices are actually connected let enabled = enabledByDefault; @@ -135,21 +159,45 @@ class MuteState { private readonly device: MediaDevice, private readonly joined$: Observable, private readonly enabledByConfig: boolean, + /** + * An optional observable which, when it emits `true`, will force the mute. + * Used for video to stop camera when earpiece mode is on. + * @private + */ + private readonly forceMute$: Observable, ) {} } export class MuteStates { + /** + * True if the selected audio output device is an earpiece. + * Used to force-disable video when on earpiece. + */ + private readonly isEarpiece$ = combineLatest( + this.mediaDevices.audioOutput.available$, + this.mediaDevices.audioOutput.selected$, + ).pipe( + map(([available, selected]) => { + if (!selected?.id) return false; + const device = available.get(selected.id); + logger.info(`MuteStates: selected audio output device:`, device); + return device?.type === "earpiece"; + }), + ); + public readonly audio = new MuteState( this.scope, this.mediaDevices.audioInput, this.joined$, Config.get().media_devices.enable_audio, + constant(false), ); public readonly video = new MuteState( this.scope, this.mediaDevices.videoInput, this.joined$, Config.get().media_devices.enable_video, + this.isEarpiece$, ); public constructor( From 149f3d02ae583d90c5cd458c7d290106d5aba1c5 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 27 Nov 2025 18:47:33 +0100 Subject: [PATCH 04/28] fix: The force mute state was not synced to the handler --- src/state/MuteStates.test.ts | 22 ++++++++++++++++++++++ src/state/MuteStates.ts | 7 +++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/state/MuteStates.test.ts b/src/state/MuteStates.test.ts index 7b02d190..f2a6e35f 100644 --- a/src/state/MuteStates.test.ts +++ b/src/state/MuteStates.test.ts @@ -170,6 +170,13 @@ describe("MuteStates", () => { constant(true), ); + let latestSyncedState: boolean | null = null; + muteStates.video.setHandler(async (enabled: boolean): Promise => { + logger.info(`Video mute state set to: ${enabled}`); + latestSyncedState = enabled; + return Promise.resolve(enabled); + }); + let lastVideoEnabled: boolean = false; muteStates.video.enabled$.subscribe((enabled) => { lastVideoEnabled = enabled; @@ -186,5 +193,20 @@ describe("MuteStates", () => { await flushPromises(); // Video should be automatically muted expect(lastVideoEnabled).toBe(false); + expect(latestSyncedState).toBe(false); + + // Try to switch to speaker + audioOutputDevice.select("0000"); + await flushPromises(); + // TODO I'd expect it to go back to previous state (enabled)?? + // But maybe not? If you move the phone away from your ear you may not want it + // to automatically enable video? + expect(lastVideoEnabled).toBe(false); + + // But yet it can be unmuted now + expect(muteStates.video.setEnabled$.value).toBeDefined(); + muteStates.video.setEnabled$.value?.(true); + await flushPromises(); + expect(lastVideoEnabled).toBe(true); }); }); diff --git a/src/state/MuteStates.ts b/src/state/MuteStates.ts index 777e3aa4..f1d61db5 100644 --- a/src/state/MuteStates.ts +++ b/src/state/MuteStates.ts @@ -72,8 +72,6 @@ export class MuteState { private readonly data$ = this.scope.behavior( this.devicesConnected$.pipe( - // this.device.available$.pipe( - // map((available) => available.size > 0), distinctUntilChanged(), withLatestFrom( this.enabledByDefault$, @@ -85,6 +83,11 @@ export class MuteState { logger.info( `MuteState: devices connected: ${devicesConnected}, disabling`, ); + // We need to sync the mute state with the handler + // to ensure nothing is beeing published. + this.handler$.value(false).catch((err) => { + logger.error("MuteState-disable: handler error", err); + }); return { enabled$: of(false), set: null, toggle: null }; } From 60bc6f1e933fd837ba4173174cf3046185d725b9 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 11:00:32 +0100 Subject: [PATCH 05/28] refactor: Extract layout mode switch + test --- src/state/CallViewModel/CallViewModel.ts | 51 +------ src/state/CallViewModel/Layout.switch.test.ts | 134 ++++++++++++++++++ src/state/CallViewModel/LayoutSwitch.ts | 89 ++++++++++++ 3 files changed, 230 insertions(+), 44 deletions(-) create mode 100644 src/state/CallViewModel/Layout.switch.test.ts create mode 100644 src/state/CallViewModel/LayoutSwitch.ts diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 3c15958a..8fb1084c 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -15,7 +15,6 @@ import { } from "livekit-client"; import { type Room as MatrixRoom } from "matrix-js-sdk"; import { - BehaviorSubject, combineLatest, distinctUntilChanged, filter, @@ -126,6 +125,7 @@ import { } from "./remoteMembers/MatrixMemberMetadata.ts"; import { Publisher } from "./localMember/Publisher.ts"; import { type Connection } from "./remoteMembers/Connection.ts"; +import { createLayoutModeSwitch } from "./LayoutSwitch.ts"; const logger = rootLogger.getChild("[CallViewModel]"); //TODO @@ -343,6 +343,7 @@ export interface CallViewModel { // DISCUSSION own membership manager ALSO this probably can be simplifis reconnecting$: Behavior; } + /** * A view model providing all the application logic needed to show the in-call * UI (may eventually be expanded to cover the lobby and feedback screens in the @@ -980,49 +981,11 @@ export function createCallViewModel$( spotlightExpandedToggle$.pipe(accumulate(false, (expanded) => !expanded)), ); - const gridModeUserSelection$ = new BehaviorSubject("grid"); - - // Callback to set the grid mode desired by the user. - // Notice that this is only a preference, the actual grid mode can be overridden - // if there is a remote screen share active. - const setGridMode = (value: GridMode): void => { - gridModeUserSelection$.next(value); - }; - /** - * The layout mode of the media tile grid. - */ - const gridMode$ = - // If the user hasn't selected spotlight and somebody starts screen sharing, - // automatically switch to spotlight mode and reset when screen sharing ends - scope.behavior( - gridModeUserSelection$.pipe( - switchMap((userSelection): Observable => { - if (userSelection === "spotlight") { - // If already in spotlight mode, stay there - return of("spotlight"); - } else { - // Otherwise, check if there is a remote screen share active - // as this could force us into spotlight mode. - return combineLatest([hasRemoteScreenShares$, windowMode$]).pipe( - map(([hasScreenShares, windowMode]): GridMode => { - const isFlatMode = windowMode === "flat"; - if (hasScreenShares || isFlatMode) { - logger.debug( - `Forcing spotlight mode, hasScreenShares=${hasScreenShares} windowMode=${windowMode}`, - ); - // override to spotlight mode - return "spotlight"; - } else { - // respect user choice - return "grid"; - } - }), - ); - } - }), - ), - "grid", - ); + const { setGridMode, gridMode$ } = createLayoutModeSwitch( + scope, + windowMode$, + hasRemoteScreenShares$, + ); const gridLayoutMedia$: Observable = combineLatest( [grid$, spotlight$], diff --git a/src/state/CallViewModel/Layout.switch.test.ts b/src/state/CallViewModel/Layout.switch.test.ts new file mode 100644 index 00000000..f4d36b9e --- /dev/null +++ b/src/state/CallViewModel/Layout.switch.test.ts @@ -0,0 +1,134 @@ +/* +Copyright 2025 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 { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { firstValueFrom, of } from "rxjs"; + +import { createLayoutModeSwitch } from "./LayoutSwitch"; +import { ObservableScope } from "../ObservableScope"; +import { constant } from "../Behavior"; +import { withTestScheduler } from "../../utils/test"; + +let scope: ObservableScope; +beforeEach(() => { + scope = new ObservableScope(); +}); +afterEach(() => { + scope.end(); +}); + +describe("Default mode", () => { + test("Should be in grid layout by default", async () => { + const { gridMode$ } = createLayoutModeSwitch( + scope, + constant("normal"), + of(false), + ); + + const mode = await firstValueFrom(gridMode$); + expect(mode).toBe("grid"); + }); + + test("Should switch to spotlight mode when window mode is flat", async () => { + const { gridMode$ } = createLayoutModeSwitch( + scope, + constant("flat"), + of(false), + ); + + const mode = await firstValueFrom(gridMode$); + expect(mode).toBe("spotlight"); + }); +}); + +test("Should allow switching modes manually", () => { + withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { + const { gridMode$, setGridMode } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold("f", { f: false, t: true }), + ); + + schedule("--sgs", { + s: () => setGridMode("spotlight"), + g: () => setGridMode("grid"), + }); + + expectObservable(gridMode$).toBe("g-sgs", { + g: "grid", + s: "spotlight", + }); + }); +}); + +test("Should switch to spotlight mode when there is a remote screen share", () => { + withTestScheduler(({ cold, behavior, expectObservable }): void => { + const shareMarble = "f--t"; + const gridsMarble = "g--s"; + const { gridMode$ } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold(shareMarble, { f: false, t: true }), + ); + + expectObservable(gridMode$).toBe(gridsMarble, { + g: "grid", + s: "spotlight", + }); + }); +}); + +test.fails("Can manually force grid when there is a screenshare", () => { + withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { + const { gridMode$, setGridMode } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold("-ft", { f: false, t: true }), + ); + + schedule("---g", { + g: () => setGridMode("grid"), + }); + + expectObservable(gridMode$).toBe("ggsg", { + g: "grid", + s: "spotlight", + }); + }); +}); + +test("Should switch back to grid mode when the remote screen share ends", () => { + withTestScheduler(({ cold, behavior, expectObservable }): void => { + const shareMarble = "f--t--f-"; + const gridsMarble = "g--s--g-"; + const { gridMode$ } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold(shareMarble, { f: false, t: true }), + ); + + expectObservable(gridMode$).toBe(gridsMarble, { + g: "grid", + s: "spotlight", + }); + }); +}); + +test("Should auto-switch to spotlight when in flat window mode", () => { + withTestScheduler(({ cold, behavior, expectObservable }): void => { + const { gridMode$ } = createLayoutModeSwitch( + scope, + behavior("naf", { n: "normal", a: "narrow", f: "flat" }), + cold("f", { f: false, t: true }), + ); + + expectObservable(gridMode$).toBe("g-s-", { + g: "grid", + s: "spotlight", + }); + }); +}); diff --git a/src/state/CallViewModel/LayoutSwitch.ts b/src/state/CallViewModel/LayoutSwitch.ts new file mode 100644 index 00000000..b406392d --- /dev/null +++ b/src/state/CallViewModel/LayoutSwitch.ts @@ -0,0 +1,89 @@ +/* +Copyright 2025 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 { + BehaviorSubject, + combineLatest, + map, + switchMap, + type Observable, + of, +} from "rxjs"; +import { logger } from "matrix-js-sdk/lib/logger"; + +import { type GridMode, type WindowMode } from "./CallViewModel.ts"; +import { type Behavior } from "../Behavior.ts"; +import { type ObservableScope } from "../ObservableScope.ts"; + +/** + * Creates a layout mode switch that allows switching between grid and spotlight modes. + * The actual layout mode can be overridden to spotlight mode if there is a remote screen share active + * or if the window mode is flat. + * + * @param scope - The observable scope to manage subscriptions. + * @param windowMode$ - The current window mode observable. + * @param hasRemoteScreenShares$ - An observable indicating if there are remote screen shares active. + */ +export function createLayoutModeSwitch( + scope: ObservableScope, + windowMode$: Behavior, + hasRemoteScreenShares$: Observable, +): { + gridMode$: Behavior; + setGridMode: (value: GridMode) => void; +} { + const gridModeUserSelection$ = new BehaviorSubject("grid"); + + // Callback to set the grid mode desired by the user. + // Notice that this is only a preference, the actual grid mode can be overridden + // if there is a remote screen share active. + const setGridMode = (value: GridMode): void => { + gridModeUserSelection$.next(value); + }; + /** + * The layout mode of the media tile grid. + */ + const gridMode$ = + // If the user hasn't selected spotlight and somebody starts screen sharing, + // automatically switch to spotlight mode and reset when screen sharing ends + scope.behavior( + gridModeUserSelection$.pipe( + switchMap((userSelection): Observable => { + if (userSelection === "spotlight") { + // If already in spotlight mode, stay there + return of("spotlight"); + } else { + // Otherwise, check if there is a remote screen share active + // as this could force us into spotlight mode. + return combineLatest([hasRemoteScreenShares$, windowMode$]).pipe( + map(([hasScreenShares, windowMode]): GridMode => { + // TODO: strange that we do that for flat mode but not for other modes? + // TODO: Why is this not handled in layoutMedia$ like other window modes? + const isFlatMode = windowMode === "flat"; + if (hasScreenShares || isFlatMode) { + logger.debug( + `Forcing spotlight mode, hasScreenShares=${hasScreenShares} windowMode=${windowMode}`, + ); + // override to spotlight mode + return "spotlight"; + } else { + // respect user choice + return "grid"; + } + }), + ); + } + }), + ), + "grid", + ); + + return { + gridMode$, + setGridMode, + }; +} From b5d3f3c72a2d31d2bded511bdcffb0adb64901bc Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 11:18:50 +0100 Subject: [PATCH 06/28] fix: Allow switching back to grid when there is a screenshare --- src/state/CallViewModel/Layout.switch.test.ts | 25 +++++- src/state/CallViewModel/LayoutSwitch.ts | 85 ++++++++++++------- 2 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/state/CallViewModel/Layout.switch.test.ts b/src/state/CallViewModel/Layout.switch.test.ts index f4d36b9e..57df5563 100644 --- a/src/state/CallViewModel/Layout.switch.test.ts +++ b/src/state/CallViewModel/Layout.switch.test.ts @@ -82,7 +82,7 @@ test("Should switch to spotlight mode when there is a remote screen share", () = }); }); -test.fails("Can manually force grid when there is a screenshare", () => { +test("Can manually force grid when there is a screenshare", () => { withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { const { gridMode$, setGridMode } = createLayoutModeSwitch( scope, @@ -101,6 +101,29 @@ test.fails("Can manually force grid when there is a screenshare", () => { }); }); +test("Should not auto-switch after manually selected grid", () => { + withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { + const { gridMode$, setGridMode } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold("-ft-ft", { f: false, t: true }), + ); + + schedule("---g", { + g: () => setGridMode("grid"), + }); + + const expectation = "ggsg"; + // If we did not respect manual selection, the expectation would be: + // const expectation = "ggsg-s"; + + expectObservable(gridMode$).toBe(expectation, { + g: "grid", + s: "spotlight", + }); + }); +}); + test("Should switch back to grid mode when the remote screen share ends", () => { withTestScheduler(({ cold, behavior, expectObservable }): void => { const shareMarble = "f--t--f-"; diff --git a/src/state/CallViewModel/LayoutSwitch.ts b/src/state/CallViewModel/LayoutSwitch.ts index b406392d..c7a1e631 100644 --- a/src/state/CallViewModel/LayoutSwitch.ts +++ b/src/state/CallViewModel/LayoutSwitch.ts @@ -9,9 +9,8 @@ import { BehaviorSubject, combineLatest, map, - switchMap, type Observable, - of, + scan, } from "rxjs"; import { logger } from "matrix-js-sdk/lib/logger"; @@ -51,33 +50,61 @@ export function createLayoutModeSwitch( // If the user hasn't selected spotlight and somebody starts screen sharing, // automatically switch to spotlight mode and reset when screen sharing ends scope.behavior( - gridModeUserSelection$.pipe( - switchMap((userSelection): Observable => { - if (userSelection === "spotlight") { - // If already in spotlight mode, stay there - return of("spotlight"); - } else { - // Otherwise, check if there is a remote screen share active - // as this could force us into spotlight mode. - return combineLatest([hasRemoteScreenShares$, windowMode$]).pipe( - map(([hasScreenShares, windowMode]): GridMode => { - // TODO: strange that we do that for flat mode but not for other modes? - // TODO: Why is this not handled in layoutMedia$ like other window modes? - const isFlatMode = windowMode === "flat"; - if (hasScreenShares || isFlatMode) { - logger.debug( - `Forcing spotlight mode, hasScreenShares=${hasScreenShares} windowMode=${windowMode}`, - ); - // override to spotlight mode - return "spotlight"; - } else { - // respect user choice - return "grid"; - } - }), - ); - } - }), + combineLatest([ + gridModeUserSelection$, + hasRemoteScreenShares$, + windowMode$, + ]).pipe( + // Scan to keep track if we have auto-switched already or not. + // To allow the user to override the auto-switch by selecting grid mode again. + scan< + [GridMode, boolean, WindowMode], + { mode: GridMode; hasAutoSwitched: boolean } + >( + (acc, [userSelection, hasScreenShares, windowMode]) => { + const isFlatMode = windowMode === "flat"; + + // Always force spotlight in flat mode, grid layout is not supported + // in that mode. + // TODO: strange that we do that for flat mode but not for other modes? + // TODO: Why is this not handled in layoutMedia$ like other window modes? + if (isFlatMode) { + logger.debug(`Forcing spotlight mode, windowMode=${windowMode}`); + return { + mode: "spotlight", + hasAutoSwitched: acc.hasAutoSwitched, + }; + } + + // User explicitly chose spotlight. + // Respect that choice. + if (userSelection === "spotlight") { + return { + mode: "spotlight", + hasAutoSwitched: acc.hasAutoSwitched, + }; + } + + // User has chosen grid mode. If a screen share starts, we will + // auto-switch to spotlight mode for better experience. + // But we only do it once, if the user switches back to grid mode, + // we respect that choice until they explicitly change it again. + if (hasScreenShares && !acc.hasAutoSwitched) { + logger.debug( + `Auto-switching to spotlight mode, hasScreenShares=${hasScreenShares}`, + ); + return { mode: "spotlight", hasAutoSwitched: true }; + } + + // Respect user's grid choice + // XXX If we want to allow switching automatically again after we can + // return hasAutoSwitched: false here instead of keeping the previous value. + return { mode: "grid", hasAutoSwitched: acc.hasAutoSwitched }; + }, + // initial value + { mode: "grid", hasAutoSwitched: false }, + ), + map(({ mode }) => mode), ), "grid", ); From 0e04fd9433a402e29e8517382345bf6d2e7e8ae4 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 14:14:28 +0100 Subject: [PATCH 07/28] fix: The handset mode overlay is visible a split second for every call --- src/room/EarpieceOverlay.module.css | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/room/EarpieceOverlay.module.css b/src/room/EarpieceOverlay.module.css index d0757cdb..e007fc44 100644 --- a/src/room/EarpieceOverlay.module.css +++ b/src/room/EarpieceOverlay.module.css @@ -2,7 +2,7 @@ position: fixed; z-index: var(--call-view-overlay-layer); inset: 0; - display: flex; + display: none; flex-direction: column; align-items: center; justify-content: center; @@ -12,6 +12,7 @@ @keyframes fade-in { from { opacity: 0; + display: flex; } to { opacity: 1; From 83ea154e1a4c83d5e171a417e99ed4108f7f0192 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 2 Dec 2025 10:36:53 -0500 Subject: [PATCH 08/28] Fix the wrong layout being used until window size changes While looking into what had regressed https://github.com/element-hq/element-call/issues/3588, I found that 28047217b85e2e6f491c887ac7099499662fa46e had filled in a couple of behaviors with non-reactive default values, the "natural window mode" behavior being among them. This meant that the app would no longer determine the correct window mode upon joining a call, instead always guessing "normal" as the value. This change restores its reactivity. --- src/state/CallViewModel/CallViewModel.test.ts | 42 +++++++++++++++++++ src/state/CallViewModel/CallViewModel.ts | 19 ++++++--- .../CallViewModel/CallViewModelTestUtils.ts | 3 ++ 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.test.ts b/src/state/CallViewModel/CallViewModel.test.ts index ef59270f..2e5b5700 100644 --- a/src/state/CallViewModel/CallViewModel.test.ts +++ b/src/state/CallViewModel/CallViewModel.test.ts @@ -502,6 +502,48 @@ describe("CallViewModel", () => { }); }); + test("layout reacts to window size", () => { + withTestScheduler(({ behavior, schedule, expectObservable }) => { + const windowSizeInputMarbles = "abc"; + const expectedLayoutMarbles = " abc"; + withCallViewModel( + { + remoteParticipants$: constant([aliceParticipant]), + rtcMembers$: constant([localRtcMember, aliceRtcMember]), + windowSize$: behavior(windowSizeInputMarbles, { + a: { width: 300, height: 600 }, // Start very narrow, like a phone + b: { width: 1000, height: 800 }, // Go to normal desktop window size + c: { width: 200, height: 180 }, // Go to PiP size + }), + }, + (vm) => { + expectObservable(summarizeLayout$(vm.layout$)).toBe( + expectedLayoutMarbles, + { + a: { + // This is the expected one-on-one layout for a narrow window + type: "spotlight-expanded", + spotlight: [`${aliceId}:0`], + pip: `${localId}:0`, + }, + b: { + // In a larger window, expect the normal one-on-one layout + type: "one-on-one", + local: `${localId}:0`, + remote: `${aliceId}:0`, + }, + c: { + // In a PiP-sized window, we of course expect a PiP layout + type: "pip", + spotlight: [`${aliceId}:0`], + }, + }, + ); + }, + ); + }); + }); + test("spotlight speakers swap places", () => { withTestScheduler(({ behavior, schedule, expectObservable }) => { // Go immediately into spotlight mode for the test diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 3c15958a..c8f5d836 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -147,6 +147,8 @@ export interface CallViewModelOptions { livekitRoomFactory?: (options?: RoomOptions) => LivekitRoom; /** Optional behavior overriding the local connection state, mainly for testing purposes. */ connectionState$?: Behavior; + /** Optional behavior overriding the computed window size, mainly for testing purposes. */ + windowSize$?: Behavior<{ width: number; height: number }>; } // Do not play any sounds if the participant count has exceeded this @@ -949,11 +951,19 @@ export function createCallViewModel$( const pipEnabled$ = scope.behavior(setPipEnabled$, false); + const windowSize$ = + options.windowSize$ ?? + scope.behavior<{ width: number; height: number }>( + fromEvent(window, "resize").pipe( + startWith(null), + map(() => ({ width: window.innerWidth, height: window.innerHeight })), + ), + ); + + // A guess at what the window's mode should be based on its size and shape. const naturalWindowMode$ = scope.behavior( - fromEvent(window, "resize").pipe( - map(() => { - const height = window.innerHeight; - const width = window.innerWidth; + windowSize$.pipe( + map(({ width, height }) => { if (height <= 400 && width <= 340) return "pip"; // Our layouts for flat windows are better at adapting to a small width // than our layouts for narrow windows are at adapting to a small height, @@ -963,7 +973,6 @@ export function createCallViewModel$( return "normal"; }), ), - "normal", ); /** diff --git a/src/state/CallViewModel/CallViewModelTestUtils.ts b/src/state/CallViewModel/CallViewModelTestUtils.ts index f86921c5..f80b4bcb 100644 --- a/src/state/CallViewModel/CallViewModelTestUtils.ts +++ b/src/state/CallViewModel/CallViewModelTestUtils.ts @@ -75,6 +75,7 @@ export interface CallViewModelInputs { speaking: Map>; mediaDevices: MediaDevices; initialSyncState: SyncState; + windowSize$: Behavior<{ width: number; height: number }>; } const localParticipant = mockLocalParticipant({ identity: "" }); @@ -89,6 +90,7 @@ export function withCallViewModel( speaking = new Map(), mediaDevices = mockMediaDevices({}), initialSyncState = SyncState.Syncing, + windowSize$ = constant({ width: 1000, height: 800 }), }: Partial = {}, continuation: ( vm: CallViewModel, @@ -173,6 +175,7 @@ export function withCallViewModel( setE2EEEnabled: async () => Promise.resolve(), }), connectionState$, + windowSize$, }, raisedHands$, reactions$, From 44980a2744e5f5b198c83e5de91f7d03aba0ab93 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 16:39:26 +0100 Subject: [PATCH 09/28] review: rename `deviceConnected` to `canControlDevices` --- src/state/MuteStates.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/state/MuteStates.ts b/src/state/MuteStates.ts index f1d61db5..632e0426 100644 --- a/src/state/MuteStates.ts +++ b/src/state/MuteStates.ts @@ -61,7 +61,7 @@ export class MuteState { this.handler$.next(defaultHandler); } - private readonly devicesConnected$ = combineLatest([ + private readonly canControlDevices$ = combineLatest([ this.device.available$, this.forceMute$, ]).pipe( @@ -71,17 +71,17 @@ export class MuteState { ); private readonly data$ = this.scope.behavior( - this.devicesConnected$.pipe( + this.canControlDevices$.pipe( distinctUntilChanged(), withLatestFrom( this.enabledByDefault$, - (devicesConnected, enabledByDefault) => { + (canControlDevices, enabledByDefault) => { logger.info( - `MuteState: devices connected: ${devicesConnected}, enabled by default: ${enabledByDefault}`, + `MuteState: canControlDevices: ${canControlDevices}, enabled by default: ${enabledByDefault}`, ); - if (!devicesConnected) { + if (!canControlDevices) { logger.info( - `MuteState: devices connected: ${devicesConnected}, disabling`, + `MuteState: devices connected: ${canControlDevices}, disabling`, ); // We need to sync the mute state with the handler // to ensure nothing is beeing published. From be0c7eb365c863457a2f69271612f5ce0dabd854 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 17:43:58 +0100 Subject: [PATCH 10/28] review: fix mock import module --- .../CallViewModel/remoteMembers/ECConnectionFactory.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts b/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts index 78e23057..0c439a6b 100644 --- a/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts +++ b/src/state/CallViewModel/remoteMembers/ECConnectionFactory.test.ts @@ -20,10 +20,9 @@ import type { ProcessorState } from "../../../livekit/TrackProcessorContext.tsx" import { constant } from "../../Behavior"; // At the top of your test file, after imports -vi.mock("livekit-client", async () => { - const actual = await vi.importActual("livekit-client"); +vi.mock("livekit-client", async (importOriginal) => { return { - ...actual, + ...(await importOriginal()), Room: vi.fn().mockImplementation(function (this: LivekitRoom, options) { const emitter = new EventEmitter(); return { From ac9acc0158f2f4c885ef29634053a5c41674d32a Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 17:47:00 +0100 Subject: [PATCH 11/28] review: refactor convert params to object for generateRoomOption --- .../remoteMembers/ConnectionFactory.ts | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts b/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts index c3a68c54..8a3175e1 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts @@ -59,19 +59,19 @@ export class ECConnectionFactory implements ConnectionFactory { ) { const defaultFactory = (): LivekitRoom => new LivekitRoom( - generateRoomOption( - this.devices, - this.processorState$.value, - livekitKeyProvider && { + generateRoomOption({ + devices: this.devices, + processorState: this.processorState$.value, + e2eeLivekitOptions: livekitKeyProvider && { keyProvider: livekitKeyProvider, // It's important that every room use a separate E2EE worker. // They get confused if given streams from multiple rooms. worker: new E2EEWorker(), }, - this.controlledAudioDevices, + controlledAudioDevices: this.controlledAudioDevices, echoCancellation, noiseSuppression, - ), + }), ); this.livekitRoomFactory = livekitRoomFactory ?? defaultFactory; } @@ -96,14 +96,24 @@ export class ECConnectionFactory implements ConnectionFactory { /** * Generate the initial LiveKit RoomOptions based on the current media devices and processor state. */ -function generateRoomOption( - devices: MediaDevices, - processorState: ProcessorState, - e2eeLivekitOptions: E2EEOptions | undefined, - controlledAudioDevices: boolean, - echoCancellation: boolean, - noiseSuppression: boolean, -): RoomOptions { +function generateRoomOption({ + devices, + processorState, + e2eeLivekitOptions, + controlledAudioDevices, + echoCancellation, + noiseSuppression, +}: { + devices: MediaDevices; + processorState: ProcessorState; + e2eeLivekitOptions: + | E2EEManagerOptions + | { e2eeManager: BaseE2EEManager } + | undefined; + controlledAudioDevices: boolean; + echoCancellation: boolean; + noiseSuppression: boolean; +}): RoomOptions { return { ...defaultLiveKitOptions, videoCaptureDefaults: { From f6a3a371cbf6259fbb8b798e40e0aff92b3eecec Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 17:52:27 +0100 Subject: [PATCH 12/28] fix lint --- src/state/CallViewModel/remoteMembers/ConnectionFactory.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts b/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts index 8a3175e1..7c3a9eab 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionFactory.ts @@ -7,10 +7,11 @@ Please see LICENSE in the repository root for full details. import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; import { - type E2EEOptions, Room as LivekitRoom, type RoomOptions, type BaseKeyProvider, + type E2EEManagerOptions, + type BaseE2EEManager, } from "livekit-client"; import { type Logger } from "matrix-js-sdk/lib/logger"; import E2EEWorker from "livekit-client/e2ee-worker?worker"; From b85f36598c7c404f782f1a5dc091f3759cda1f57 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 08:54:52 +0100 Subject: [PATCH 13/28] fix: mistake in file name --- .../CallViewModel/{Layout.switch.test.ts => LayoutSwitch.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/state/CallViewModel/{Layout.switch.test.ts => LayoutSwitch.test.ts} (100%) diff --git a/src/state/CallViewModel/Layout.switch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts similarity index 100% rename from src/state/CallViewModel/Layout.switch.test.ts rename to src/state/CallViewModel/LayoutSwitch.test.ts From a93ceeae4ba2402c7fc8ccc3d2c774ab66006210 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 09:01:26 +0100 Subject: [PATCH 14/28] review: Keep previous behavior for now to always auto switch --- src/state/CallViewModel/LayoutSwitch.test.ts | 11 +++++++---- src/state/CallViewModel/LayoutSwitch.ts | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/state/CallViewModel/LayoutSwitch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts index 57df5563..d0034743 100644 --- a/src/state/CallViewModel/LayoutSwitch.test.ts +++ b/src/state/CallViewModel/LayoutSwitch.test.ts @@ -101,21 +101,24 @@ test("Can manually force grid when there is a screenshare", () => { }); }); -test("Should not auto-switch after manually selected grid", () => { +test("Should auto-switch after manually selected grid", () => { withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { const { gridMode$, setGridMode } = createLayoutModeSwitch( scope, behavior("n", { n: "normal" }), + // Two screenshares will happen in sequence cold("-ft-ft", { f: false, t: true }), ); + // There was a screen-share that forced spotlight, then + // the user manually switch back to grid schedule("---g", { g: () => setGridMode("grid"), }); - const expectation = "ggsg"; - // If we did not respect manual selection, the expectation would be: - // const expectation = "ggsg-s"; + // If we did want to respect manual selection, the expectation would be: + // const expectation = "ggsg"; + const expectation = "ggsg-s"; expectObservable(gridMode$).toBe(expectation, { g: "grid", diff --git a/src/state/CallViewModel/LayoutSwitch.ts b/src/state/CallViewModel/LayoutSwitch.ts index c7a1e631..cfb31d53 100644 --- a/src/state/CallViewModel/LayoutSwitch.ts +++ b/src/state/CallViewModel/LayoutSwitch.ts @@ -97,9 +97,9 @@ export function createLayoutModeSwitch( } // Respect user's grid choice - // XXX If we want to allow switching automatically again after we can - // return hasAutoSwitched: false here instead of keeping the previous value. - return { mode: "grid", hasAutoSwitched: acc.hasAutoSwitched }; + // XXX If we want to forbid switching automatically again after we can + // return hasAutoSwitched: acc.hasAutoSwitched here instead of setting to false. + return { mode: "grid", hasAutoSwitched: false }; }, // initial value { mode: "grid", hasAutoSwitched: false }, From bbd92f666be991cf4e1ffde591cbb614b66b16a3 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 3 Dec 2025 10:42:04 -0500 Subject: [PATCH 15/28] Simplify computation of analytics ID Since we now bundle a trusted Element Call widget with our messenger applications and this widget reports analytics to an endpoint determined by the messenger app, there is no longer any reason to compute a different analytics ID from the one used by the messenger app. --- src/analytics/PosthogAnalytics.ts | 32 ++++--------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/src/analytics/PosthogAnalytics.ts b/src/analytics/PosthogAnalytics.ts index 8b2aa91d..46223afe 100644 --- a/src/analytics/PosthogAnalytics.ts +++ b/src/analytics/PosthogAnalytics.ts @@ -247,9 +247,8 @@ export class PosthogAnalytics { // wins, and the first writer will send tracking with an ID that doesn't match the one on the server // until the next time account data is refreshed and this function is called (most likely on next // page load). This will happen pretty infrequently, so we can tolerate the possibility. - const accountDataAnalyticsId = analyticsIdGenerator(); - await this.setAccountAnalyticsId(accountDataAnalyticsId); - analyticsID = await this.hashedEcAnalyticsId(accountDataAnalyticsId); + analyticsID = analyticsIdGenerator(); + await this.setAccountAnalyticsId(analyticsID); } } catch (e) { // The above could fail due to network requests, but not essential to starting the application, @@ -270,37 +269,14 @@ export class PosthogAnalytics { private async getAnalyticsId(): Promise { const client: MatrixClient = window.matrixclient; - let accountAnalyticsId: string | null; if (widget) { - accountAnalyticsId = getUrlParams().posthogUserId; + return getUrlParams().posthogUserId; } else { const accountData = await client.getAccountDataFromServer( PosthogAnalytics.ANALYTICS_EVENT_TYPE, ); - accountAnalyticsId = accountData?.id ?? null; + return accountData?.id ?? null; } - if (accountAnalyticsId) { - // we dont just use the element web analytics ID because that would allow to associate - // users between the two posthog instances. By using a hash from the username and the element web analytics id - // it is not possible to conclude the element web posthog user id from the element call user id and vice versa. - return await this.hashedEcAnalyticsId(accountAnalyticsId); - } - return null; - } - - private async hashedEcAnalyticsId( - accountAnalyticsId: string, - ): Promise { - const client: MatrixClient = window.matrixclient; - const posthogIdMaterial = "ec" + accountAnalyticsId + client.getUserId(); - const bufferForPosthogId = await crypto.subtle.digest( - "sha-256", - new TextEncoder().encode(posthogIdMaterial), - ); - const view = new Int32Array(bufferForPosthogId); - return Array.from(view) - .map((b) => Math.abs(b).toString(16).padStart(2, "0")) - .join(""); } private async setAccountAnalyticsId(analyticsID: string): Promise { From 0ed7194d87175edb277f212a21f747229b219c8d Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 17:10:57 +0100 Subject: [PATCH 16/28] fix: earpiece overlay not showing + playwright test! --- playwright.config.ts | 24 +++- playwright/mobile/create-call-mobile.spec.ts | 112 +++++++++++++++++++ playwright/mobile/fixture-mobile-create.ts | 73 ++++++++++++ src/room/EarpieceOverlay.module.css | 3 +- 4 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 playwright/mobile/create-call-mobile.spec.ts create mode 100644 playwright/mobile/fixture-mobile-create.ts diff --git a/playwright.config.ts b/playwright.config.ts index 7a8ee530..391e746f 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -38,6 +38,7 @@ export default defineConfig({ projects: [ { name: "chromium", + testIgnore: "**/mobile/**", use: { ...devices["Desktop Chrome"], permissions: [ @@ -56,9 +57,9 @@ export default defineConfig({ }, }, }, - { name: "firefox", + testIgnore: "**/mobile/**", use: { ...devices["Desktop Firefox"], ignoreHTTPSErrors: true, @@ -70,6 +71,27 @@ export default defineConfig({ }, }, }, + { + name: "mobile", + testMatch: "**/mobile/**", + use: { + ...devices["Pixel 7"], + ignoreHTTPSErrors: true, + permissions: [ + "clipboard-write", + "clipboard-read", + "microphone", + "camera", + ], + launchOptions: { + args: [ + "--use-fake-ui-for-media-stream", + "--use-fake-device-for-media-stream", + "--mute-audio", + ], + }, + }, + }, // No safari for now, until I find a solution to fix `Not allowed to request resource` due to calling // clear http to the homeserver diff --git a/playwright/mobile/create-call-mobile.spec.ts b/playwright/mobile/create-call-mobile.spec.ts new file mode 100644 index 00000000..853294ea --- /dev/null +++ b/playwright/mobile/create-call-mobile.spec.ts @@ -0,0 +1,112 @@ +/* +Copyright 2025 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 { expect, test } from "@playwright/test"; + +import { mobileTest } from "./fixture-mobile-create.ts"; + +test("@mobile Start a new call then leave and show the feedback screen", async ({ + page, +}) => { + await page.goto("/"); + + await page.getByTestId("home_callName").click(); + await page.getByTestId("home_callName").fill("HelloCall"); + await page.getByTestId("home_displayName").click(); + await page.getByTestId("home_displayName").fill("John Doe"); + await page.getByTestId("home_go").click(); + + // await page.pause(); + await expect(page.locator("video")).toBeVisible(); + await expect(page.getByTestId("lobby_joinCall")).toBeVisible(); + + await page.getByRole("button", { name: "Continue in browser" }).click(); + // Join the call + await page.getByTestId("lobby_joinCall").click(); + + // Ensure that the call is connected + await page + .locator("div") + .filter({ hasText: /^HelloCall$/ }) + .click(); + // Check the number of participants + await expect(page.locator("div").filter({ hasText: /^1$/ })).toBeVisible(); + // The tooltip with the name should be visible + await expect(page.getByTestId("name_tag")).toContainText("John Doe"); + + // leave the call + await page.getByTestId("incall_leave").click(); + await expect(page.getByRole("heading")).toContainText( + "John Doe, your call has ended. How did it go?", + ); + await expect(page.getByRole("main")).toContainText( + "Why not finish by setting up a password to keep your account?", + ); + + await expect( + page.getByRole("link", { name: "Not now, return to home screen" }), + ).toBeVisible(); +}); + +mobileTest("Start a new call as widget", async ({ asMobile, browser }) => { + test.slow(); // Triples the timeout + const { creatorPage, inviteLink } = asMobile; + + // test("Show earpiece overlay when output is earpiece", async ({ browser }) => { + // Use reduce motion to disable animations that are making the tests a bit flaky + + // ======== + // ACT: The other user use the invite link to join the call as a guest + // ======== + const guestInviteeContext = await browser.newContext({ + reducedMotion: "reduce", + }); + const guestPage = await guestInviteeContext.newPage(); + await guestPage.goto(inviteLink + "&controlledAudioDevices=true"); + + await guestPage.getByRole("button", { name: "Continue in browser" }).click(); + + await guestPage.getByTestId("joincall_displayName").fill("Invitee"); + await expect(guestPage.getByTestId("joincall_joincall")).toBeVisible(); + await guestPage.getByTestId("joincall_joincall").click(); + await guestPage.getByTestId("lobby_joinCall").click(); + + // ======== + // ASSERT: check that there are two members in the call + // ======== + + // There should be two participants now + await expect( + guestPage.getByTestId("roomHeader_participants_count"), + ).toContainText("2"); + expect(await guestPage.getByTestId("videoTile").count()).toBe(2); + + // Same in creator page + await expect( + creatorPage.getByTestId("roomHeader_participants_count"), + ).toContainText("2"); + expect(await creatorPage.getByTestId("videoTile").count()).toBe(2); + + // TEST: control audio devices from the invitee page + + await guestPage.evaluate(() => { + window.controls.setAvailableAudioDevices([ + { id: "speaker", name: "Speaker", isSpeaker: true }, + { id: "earpiece", name: "Handset", isEarpiece: true }, + { id: "headphones", name: "Headphones" }, + ]); + window.controls.setAudioDevice("earpiece"); + }); + await expect( + guestPage.getByRole("heading", { name: "Handset Mode" }), + ).toBeVisible(); + await expect( + guestPage.getByRole("button", { name: "Back to Speaker Mode" }), + ).toBeVisible(); + + // await guestPage.pause(); +}); diff --git a/playwright/mobile/fixture-mobile-create.ts b/playwright/mobile/fixture-mobile-create.ts new file mode 100644 index 00000000..053335d3 --- /dev/null +++ b/playwright/mobile/fixture-mobile-create.ts @@ -0,0 +1,73 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { type Browser, type Page, test, expect } from "@playwright/test"; + +export interface MobileCreateFixtures { + asMobile: { + creatorPage: Page; + inviteLink: string; + }; +} + +export const mobileTest = test.extend({ + asMobile: async ({ browser }, puse) => { + const fixtures = await createCallAndInvite(browser); + await puse({ + creatorPage: fixtures.page, + inviteLink: fixtures.inviteLink, + }); + }, +}); + +/** + * Create a call and generate an invite link + */ +async function createCallAndInvite( + browser: Browser, +): Promise<{ page: Page; inviteLink: string }> { + const creatorContext = await browser.newContext({ reducedMotion: "reduce" }); + const creatorPage = await creatorContext.newPage(); + + await creatorPage.goto("/"); + + // ======== + // ARRANGE: The first user creates a call as guest, join it, then click the invite button to copy the invite link + // ======== + await creatorPage.getByTestId("home_callName").click(); + await creatorPage.getByTestId("home_callName").fill("Welcome"); + await creatorPage.getByTestId("home_displayName").click(); + await creatorPage.getByTestId("home_displayName").fill("Inviter"); + await creatorPage.getByTestId("home_go").click(); + await expect(creatorPage.locator("video")).toBeVisible(); + + await creatorPage + .getByRole("button", { name: "Continue in browser" }) + .click(); + // join + await creatorPage.getByTestId("lobby_joinCall").click(); + + // Get the invite link + await creatorPage.getByRole("button", { name: "Invite" }).click(); + await expect( + creatorPage.getByRole("heading", { name: "Invite to this call" }), + ).toBeVisible(); + await expect(creatorPage.getByRole("img", { name: "QR Code" })).toBeVisible(); + await expect(creatorPage.getByTestId("modal_inviteLink")).toBeVisible(); + await expect(creatorPage.getByTestId("modal_inviteLink")).toBeVisible(); + await creatorPage.getByTestId("modal_inviteLink").click(); + + const inviteLink = (await creatorPage.evaluate( + "navigator.clipboard.readText()", + )) as string; + expect(inviteLink).toContain("room/#/"); + + return { + page: creatorPage, + inviteLink, + }; +} diff --git a/src/room/EarpieceOverlay.module.css b/src/room/EarpieceOverlay.module.css index e007fc44..1718b0f2 100644 --- a/src/room/EarpieceOverlay.module.css +++ b/src/room/EarpieceOverlay.module.css @@ -12,7 +12,6 @@ @keyframes fade-in { from { opacity: 0; - display: flex; } to { opacity: 1; @@ -20,6 +19,7 @@ } .overlay[data-show="true"] { + display: flex; animation: fade-in 200ms; } @@ -29,7 +29,6 @@ } to { opacity: 0; - display: none; } } From bcb2b36888abf5d99e6be6cde104f4a12d23c91f Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 17:17:58 +0100 Subject: [PATCH 17/28] keep livekit 1.9.4 as the latest is breaking the dev backend --- dev-backend-docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-backend-docker-compose.yml b/dev-backend-docker-compose.yml index 50498c7a..c7591847 100644 --- a/dev-backend-docker-compose.yml +++ b/dev-backend-docker-compose.yml @@ -47,7 +47,7 @@ services: - ecbackend livekit: - image: livekit/livekit-server:latest + image: livekit/livekit-server:v1.9.4 pull_policy: always hostname: livekit-sfu command: --dev --config /etc/livekit.yaml @@ -67,7 +67,7 @@ services: - ecbackend livekit-1: - image: livekit/livekit-server:latest + image: livekit/livekit-server:v1.9.4 pull_policy: always hostname: livekit-sfu-1 command: --dev --config /etc/livekit.yaml From af08c1830e5212a76e136bf856131675948cd850 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 17:17:58 +0100 Subject: [PATCH 18/28] keep livekit 1.9.4 as the latest is breaking the dev backend --- dev-backend-docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-backend-docker-compose.yml b/dev-backend-docker-compose.yml index 50498c7a..c7591847 100644 --- a/dev-backend-docker-compose.yml +++ b/dev-backend-docker-compose.yml @@ -47,7 +47,7 @@ services: - ecbackend livekit: - image: livekit/livekit-server:latest + image: livekit/livekit-server:v1.9.4 pull_policy: always hostname: livekit-sfu command: --dev --config /etc/livekit.yaml @@ -67,7 +67,7 @@ services: - ecbackend livekit-1: - image: livekit/livekit-server:latest + image: livekit/livekit-server:v1.9.4 pull_policy: always hostname: livekit-sfu-1 command: --dev --config /etc/livekit.yaml From c7491c3e9747cb0325807b3e9a989c10ab9551d5 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 17:25:32 +0100 Subject: [PATCH 19/28] move fixture to correct folder --- playwright/{mobile => fixtures}/fixture-mobile-create.ts | 0 playwright/mobile/create-call-mobile.spec.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename playwright/{mobile => fixtures}/fixture-mobile-create.ts (100%) diff --git a/playwright/mobile/fixture-mobile-create.ts b/playwright/fixtures/fixture-mobile-create.ts similarity index 100% rename from playwright/mobile/fixture-mobile-create.ts rename to playwright/fixtures/fixture-mobile-create.ts diff --git a/playwright/mobile/create-call-mobile.spec.ts b/playwright/mobile/create-call-mobile.spec.ts index 853294ea..9005d510 100644 --- a/playwright/mobile/create-call-mobile.spec.ts +++ b/playwright/mobile/create-call-mobile.spec.ts @@ -7,7 +7,7 @@ Please see LICENSE in the repository root for full details. import { expect, test } from "@playwright/test"; -import { mobileTest } from "./fixture-mobile-create.ts"; +import { mobileTest } from "../fixtures/fixture-mobile-create.ts"; test("@mobile Start a new call then leave and show the feedback screen", async ({ page, From fdc66a1d62bce8e38b0d89192eba0b70056e2f3c Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 18:36:51 +0100 Subject: [PATCH 20/28] fix: existing screenshare switching twice --- src/state/CallViewModel/LayoutSwitch.test.ts | 25 ++++++++++++++++ src/state/CallViewModel/LayoutSwitch.ts | 30 ++++++++++++++------ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/state/CallViewModel/LayoutSwitch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts index d0034743..c1941eb8 100644 --- a/src/state/CallViewModel/LayoutSwitch.test.ts +++ b/src/state/CallViewModel/LayoutSwitch.test.ts @@ -144,6 +144,31 @@ test("Should switch back to grid mode when the remote screen share ends", () => }); }); +test("can switch manually to grid after screen share while manually in spotlight", () => { + withTestScheduler(({ cold, behavior, schedule, expectObservable }): void => { + // Initially, no one is sharing. Then the user manually switches to + // spotlight. After a screen share starts, the user manually switches to + // grid. + const shareMarbles = " f-t-"; + const setModeMarbles = "-s-g"; + const expectation = " gs-g"; + const { gridMode$, setGridMode } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold(shareMarbles, { f: false, t: true }), + ); + schedule(setModeMarbles, { + g: () => setGridMode("grid"), + s: () => setGridMode("spotlight"), + }); + + expectObservable(gridMode$).toBe(expectation, { + g: "grid", + s: "spotlight", + }); + }); +}); + test("Should auto-switch to spotlight when in flat window mode", () => { withTestScheduler(({ cold, behavior, expectObservable }): void => { const { gridMode$ } = createLayoutModeSwitch( diff --git a/src/state/CallViewModel/LayoutSwitch.ts b/src/state/CallViewModel/LayoutSwitch.ts index cfb31d53..65c6bcb1 100644 --- a/src/state/CallViewModel/LayoutSwitch.ts +++ b/src/state/CallViewModel/LayoutSwitch.ts @@ -59,7 +59,13 @@ export function createLayoutModeSwitch( // To allow the user to override the auto-switch by selecting grid mode again. scan< [GridMode, boolean, WindowMode], - { mode: GridMode; hasAutoSwitched: boolean } + { + mode: GridMode; + /** Remember if the change was user driven or not */ + hasAutoSwitched: boolean; + /** To know if it is new screen share or an already handled */ + prevShare: boolean; + } >( (acc, [userSelection, hasScreenShares, windowMode]) => { const isFlatMode = windowMode === "flat"; @@ -73,6 +79,7 @@ export function createLayoutModeSwitch( return { mode: "spotlight", hasAutoSwitched: acc.hasAutoSwitched, + prevShare: hasScreenShares, }; } @@ -82,6 +89,7 @@ export function createLayoutModeSwitch( return { mode: "spotlight", hasAutoSwitched: acc.hasAutoSwitched, + prevShare: hasScreenShares, }; } @@ -89,20 +97,26 @@ export function createLayoutModeSwitch( // auto-switch to spotlight mode for better experience. // But we only do it once, if the user switches back to grid mode, // we respect that choice until they explicitly change it again. - if (hasScreenShares && !acc.hasAutoSwitched) { - logger.debug( - `Auto-switching to spotlight mode, hasScreenShares=${hasScreenShares}`, - ); - return { mode: "spotlight", hasAutoSwitched: true }; + const isNewShare = hasScreenShares && !acc.prevShare; + if (isNewShare && !acc.hasAutoSwitched) { + return { + mode: "spotlight", + hasAutoSwitched: true, + prevShare: true, + }; } // Respect user's grid choice // XXX If we want to forbid switching automatically again after we can // return hasAutoSwitched: acc.hasAutoSwitched here instead of setting to false. - return { mode: "grid", hasAutoSwitched: false }; + return { + mode: "grid", + hasAutoSwitched: false, + prevShare: hasScreenShares, + }; }, // initial value - { mode: "grid", hasAutoSwitched: false }, + { mode: "grid", hasAutoSwitched: false, prevShare: false }, ), map(({ mode }) => mode), ), From be7407ea3d7fc060538f8c3e52b2aa2bb4ea25bf Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 4 Dec 2025 09:37:07 +0100 Subject: [PATCH 21/28] review: quick renaming --- src/state/CallViewModel/LayoutSwitch.test.ts | 17 +++++++++++++++ src/state/CallViewModel/LayoutSwitch.ts | 22 ++++++++++---------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/state/CallViewModel/LayoutSwitch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts index c1941eb8..ae5a3896 100644 --- a/src/state/CallViewModel/LayoutSwitch.test.ts +++ b/src/state/CallViewModel/LayoutSwitch.test.ts @@ -144,6 +144,23 @@ test("Should switch back to grid mode when the remote screen share ends", () => }); }); +test("can auto-switch to spotlight again after first screen share ends", () => { + withTestScheduler(({ cold, behavior, expectObservable }): void => { + const shareMarble = "ftft"; + const gridsMarble = "gsgs"; + const { gridMode$ } = createLayoutModeSwitch( + scope, + behavior("n", { n: "normal" }), + cold(shareMarble, { f: false, t: true }), + ); + + expectObservable(gridMode$).toBe(gridsMarble, { + g: "grid", + s: "spotlight", + }); + }); +}); + test("can switch manually to grid after screen share while manually in spotlight", () => { withTestScheduler(({ cold, behavior, schedule, expectObservable }): void => { // Initially, no one is sharing. Then the user manually switches to diff --git a/src/state/CallViewModel/LayoutSwitch.ts b/src/state/CallViewModel/LayoutSwitch.ts index 65c6bcb1..3ad93204 100644 --- a/src/state/CallViewModel/LayoutSwitch.ts +++ b/src/state/CallViewModel/LayoutSwitch.ts @@ -64,10 +64,10 @@ export function createLayoutModeSwitch( /** Remember if the change was user driven or not */ hasAutoSwitched: boolean; /** To know if it is new screen share or an already handled */ - prevShare: boolean; + hasScreenShares: boolean; } >( - (acc, [userSelection, hasScreenShares, windowMode]) => { + (prev, [userSelection, hasScreenShares, windowMode]) => { const isFlatMode = windowMode === "flat"; // Always force spotlight in flat mode, grid layout is not supported @@ -78,8 +78,8 @@ export function createLayoutModeSwitch( logger.debug(`Forcing spotlight mode, windowMode=${windowMode}`); return { mode: "spotlight", - hasAutoSwitched: acc.hasAutoSwitched, - prevShare: hasScreenShares, + hasAutoSwitched: prev.hasAutoSwitched, + hasScreenShares, }; } @@ -88,8 +88,8 @@ export function createLayoutModeSwitch( if (userSelection === "spotlight") { return { mode: "spotlight", - hasAutoSwitched: acc.hasAutoSwitched, - prevShare: hasScreenShares, + hasAutoSwitched: prev.hasAutoSwitched, + hasScreenShares, }; } @@ -97,12 +97,12 @@ export function createLayoutModeSwitch( // auto-switch to spotlight mode for better experience. // But we only do it once, if the user switches back to grid mode, // we respect that choice until they explicitly change it again. - const isNewShare = hasScreenShares && !acc.prevShare; - if (isNewShare && !acc.hasAutoSwitched) { + const isNewShare = hasScreenShares && !prev.hasScreenShares; + if (isNewShare && !prev.hasAutoSwitched) { return { mode: "spotlight", hasAutoSwitched: true, - prevShare: true, + hasScreenShares: true, }; } @@ -112,11 +112,11 @@ export function createLayoutModeSwitch( return { mode: "grid", hasAutoSwitched: false, - prevShare: hasScreenShares, + hasScreenShares, }; }, // initial value - { mode: "grid", hasAutoSwitched: false, prevShare: false }, + { mode: "grid", hasAutoSwitched: false, hasScreenShares: false }, ), map(({ mode }) => mode), ), From 940c787040b50ab3d1e1eb22010c36a6b7a8af7e Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 4 Dec 2025 10:06:45 +0100 Subject: [PATCH 22/28] review: quick renaming --- playwright/fixtures/fixture-mobile-create.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playwright/fixtures/fixture-mobile-create.ts b/playwright/fixtures/fixture-mobile-create.ts index 053335d3..3920c978 100644 --- a/playwright/fixtures/fixture-mobile-create.ts +++ b/playwright/fixtures/fixture-mobile-create.ts @@ -15,9 +15,9 @@ export interface MobileCreateFixtures { } export const mobileTest = test.extend({ - asMobile: async ({ browser }, puse) => { + asMobile: async ({ browser }, pUse) => { const fixtures = await createCallAndInvite(browser); - await puse({ + await pUse({ creatorPage: fixtures.page, inviteLink: fixtures.inviteLink, }); From 7a2c1af44b14cce336eb91b3664501fb9a874ed8 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 4 Dec 2025 10:36:57 +0100 Subject: [PATCH 23/28] review: use simple transition instead of keyframe --- src/room/EarpieceOverlay.module.css | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/src/room/EarpieceOverlay.module.css b/src/room/EarpieceOverlay.module.css index 1718b0f2..e53a1974 100644 --- a/src/room/EarpieceOverlay.module.css +++ b/src/room/EarpieceOverlay.module.css @@ -2,39 +2,22 @@ position: fixed; z-index: var(--call-view-overlay-layer); inset: 0; - display: none; + display: flex; flex-direction: column; align-items: center; justify-content: center; gap: var(--cpd-space-2x); -} - -@keyframes fade-in { - from { - opacity: 0; - } - to { - opacity: 1; - } + transition: opacity 200ms; } .overlay[data-show="true"] { - display: flex; - animation: fade-in 200ms; -} - -@keyframes fade-out { - from { - opacity: 1; - } - to { - opacity: 0; - } + opacity: 1; } .overlay[data-show="false"] { - animation: fade-out 130ms forwards; + opacity: 0; pointer-events: none; + transition-duration: 130ms; } .overlay::before { From 71bf55f3581438245210f229a13f372a18a3f473 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 4 Dec 2025 10:37:14 +0100 Subject: [PATCH 24/28] also test that video is muted when earpiece overlay is on --- playwright/mobile/create-call-mobile.spec.ts | 101 ++++++++++--------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/playwright/mobile/create-call-mobile.spec.ts b/playwright/mobile/create-call-mobile.spec.ts index 9005d510..141ffaae 100644 --- a/playwright/mobile/create-call-mobile.spec.ts +++ b/playwright/mobile/create-call-mobile.spec.ts @@ -52,61 +52,64 @@ test("@mobile Start a new call then leave and show the feedback screen", async ( ).toBeVisible(); }); -mobileTest("Start a new call as widget", async ({ asMobile, browser }) => { - test.slow(); // Triples the timeout - const { creatorPage, inviteLink } = asMobile; +mobileTest( + "Test earpiece overlay in controlledAudioDevices mode", + async ({ asMobile, browser }) => { + test.slow(); // Triples the timeout + const { creatorPage, inviteLink } = asMobile; - // test("Show earpiece overlay when output is earpiece", async ({ browser }) => { - // Use reduce motion to disable animations that are making the tests a bit flaky + // ======== + // ACT: The other user use the invite link to join the call as a guest + // ======== + const guestInviteeContext = await browser.newContext({ + reducedMotion: "reduce", + }); + const guestPage = await guestInviteeContext.newPage(); + await guestPage.goto(inviteLink + "&controlledAudioDevices=true"); - // ======== - // ACT: The other user use the invite link to join the call as a guest - // ======== - const guestInviteeContext = await browser.newContext({ - reducedMotion: "reduce", - }); - const guestPage = await guestInviteeContext.newPage(); - await guestPage.goto(inviteLink + "&controlledAudioDevices=true"); + await guestPage + .getByRole("button", { name: "Continue in browser" }) + .click(); - await guestPage.getByRole("button", { name: "Continue in browser" }).click(); + await guestPage.getByTestId("joincall_displayName").fill("Invitee"); + await expect(guestPage.getByTestId("joincall_joincall")).toBeVisible(); + await guestPage.getByTestId("joincall_joincall").click(); + await guestPage.getByTestId("lobby_joinCall").click(); - await guestPage.getByTestId("joincall_displayName").fill("Invitee"); - await expect(guestPage.getByTestId("joincall_joincall")).toBeVisible(); - await guestPage.getByTestId("joincall_joincall").click(); - await guestPage.getByTestId("lobby_joinCall").click(); + // ======== + // ASSERT: check that there are two members in the call + // ======== - // ======== - // ASSERT: check that there are two members in the call - // ======== + // There should be two participants now + await expect( + guestPage.getByTestId("roomHeader_participants_count"), + ).toContainText("2"); + expect(await guestPage.getByTestId("videoTile").count()).toBe(2); - // There should be two participants now - await expect( - guestPage.getByTestId("roomHeader_participants_count"), - ).toContainText("2"); - expect(await guestPage.getByTestId("videoTile").count()).toBe(2); + // Same in creator page + await expect( + creatorPage.getByTestId("roomHeader_participants_count"), + ).toContainText("2"); + expect(await creatorPage.getByTestId("videoTile").count()).toBe(2); - // Same in creator page - await expect( - creatorPage.getByTestId("roomHeader_participants_count"), - ).toContainText("2"); - expect(await creatorPage.getByTestId("videoTile").count()).toBe(2); + // TEST: control audio devices from the invitee page - // TEST: control audio devices from the invitee page + await guestPage.evaluate(() => { + window.controls.setAvailableAudioDevices([ + { id: "speaker", name: "Speaker", isSpeaker: true }, + { id: "earpiece", name: "Handset", isEarpiece: true }, + { id: "headphones", name: "Headphones" }, + ]); + window.controls.setAudioDevice("earpiece"); + }); + await expect( + guestPage.getByRole("heading", { name: "Handset Mode" }), + ).toBeVisible(); + await expect( + guestPage.getByRole("button", { name: "Back to Speaker Mode" }), + ).toBeVisible(); - await guestPage.evaluate(() => { - window.controls.setAvailableAudioDevices([ - { id: "speaker", name: "Speaker", isSpeaker: true }, - { id: "earpiece", name: "Handset", isEarpiece: true }, - { id: "headphones", name: "Headphones" }, - ]); - window.controls.setAudioDevice("earpiece"); - }); - await expect( - guestPage.getByRole("heading", { name: "Handset Mode" }), - ).toBeVisible(); - await expect( - guestPage.getByRole("button", { name: "Back to Speaker Mode" }), - ).toBeVisible(); - - // await guestPage.pause(); -}); + // Should auto-mute the video when earpiece is selected + await expect(guestPage.getByTestId("incall_videomute")).toBeDisabled(); + }, +); From d8b9568400eb9267a2dc5f3efdff09cfec770831 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 23:33:41 -0500 Subject: [PATCH 25/28] Stop publisher in a less brittle way --- .../CallViewModel/localMember/LocalMembership.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMembership.ts b/src/state/CallViewModel/localMember/LocalMembership.ts index 60ae79b8..71261d37 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.ts +++ b/src/state/CallViewModel/localMember/LocalMembership.ts @@ -323,12 +323,14 @@ export const createLocalMembership$ = ({ // - overwrite current publisher scope.reconcile(localConnection$, async (connection) => { if (connection !== null) { - publisher$.next(createPublisherFactory(connection)); + const publisher = createPublisherFactory(connection); + publisher$.next(publisher); + // Clean-up callback + return Promise.resolve(async (): Promise => { + await publisher.stopPublishing(); + publisher.stopTracks(); + }); } - return Promise.resolve(async (): Promise => { - await publisher$?.value?.stopPublishing(); - publisher$?.value?.stopTracks(); - }); }); // Use reconcile here to not run concurrent createAndSetupTracks calls From 9481dc401c6c06c32085560431552247a56ddb16 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 23:34:42 -0500 Subject: [PATCH 26/28] Remove extraneous 'scope running' check Semantically, behaviors are only meaningful for as long as their scope is running. Setting a behavior's value to an empty array once its scope ends is not guaranteed to work (as it depends on execution order of how the scope is ended), and subscribers should be robust enough to handle clean-up of all connections at the end of the scope either way. --- .../CallViewModel/remoteMembers/ConnectionManager.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts index 0b9f939c..09a8e79f 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts @@ -10,7 +10,7 @@ import { type LivekitTransport, type ParticipantId, } from "matrix-js-sdk/lib/matrixrtc"; -import { BehaviorSubject, combineLatest, map, of, switchMap, tap } from "rxjs"; +import { combineLatest, map, of, switchMap, tap } from "rxjs"; import { type Logger } from "matrix-js-sdk/lib/logger"; import { type LocalParticipant, type RemoteParticipant } from "livekit-client"; @@ -115,9 +115,6 @@ export function createConnectionManager$({ logger: parentLogger, }: Props): IConnectionManager { const logger = parentLogger.getChild("[ConnectionManager]"); - - const running$ = new BehaviorSubject(true); - scope.onEnd(() => running$.next(false)); // TODO logger: only construct one logger from the client and make it compatible via a EC specific sing /** @@ -129,10 +126,7 @@ export function createConnectionManager$({ * externally this is modified via `registerTransports()`. */ const transports$ = scope.behavior( - combineLatest([running$, inputTransports$]).pipe( - map(([running, transports]) => - transports.mapInner((transport) => (running ? transport : [])), - ), + inputTransports$.pipe( map((transports) => transports.mapInner(removeDuplicateTransports)), tap(({ value: transports }) => { logger.trace( From 2f3f9f95eb6ed5961ff7769c246b0a29a97d181c Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 23:38:15 -0500 Subject: [PATCH 27/28] Use more compact optional chaining and coalescing notation --- src/state/CallViewModel/remoteMembers/ConnectionManager.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts index 09a8e79f..755ba3dd 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts @@ -60,11 +60,7 @@ export class ConnectionManagerData { transport: LivekitTransport, ): (LocalParticipant | RemoteParticipant)[] { const key = transport.livekit_service_url + "|" + transport.livekit_alias; - const existing = this.store.get(key); - if (existing) { - return existing[1]; - } - return []; + return this.store.get(key)?.[1] ?? []; } /** * Get all connections where the given participant is publishing. From 6ee3ef27954d1148ad4e3d7854d84431fb6c349b Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 23:38:54 -0500 Subject: [PATCH 28/28] Edit a misleading log line The factory function is called once per item to construct the item. It is not called on future updates to the item's data. --- src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts index 2f152630..79ad933c 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts @@ -108,7 +108,7 @@ export function createMatrixLivekitMembers$({ // Each update where the key of the generator array do not change will result in updates to the `data$` observable in the factory. (scope, data$, participantId, userId) => { logger.debug( - `Updating data$ for participantId: ${participantId}, userId: ${userId}`, + `Generating member for participantId: ${participantId}, userId: ${userId}`, ); // will only get called once per `participantId, userId` pair. // updates to data$ and as a result to displayName$ and mxcAvatarUrl$ are more frequent.