From e2607d6399c29a97744cb8c071271202b681e711 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 25 Nov 2025 11:10:53 +0100 Subject: [PATCH 01/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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 44980a2744e5f5b198c83e5de91f7d03aba0ab93 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 16:39:26 +0100 Subject: [PATCH 07/16] 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 08/16] 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 09/16] 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 10/16] 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 11/16] 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 12/16] 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 13/16] 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 af08c1830e5212a76e136bf856131675948cd850 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 17:17:58 +0100 Subject: [PATCH 14/16] 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 fdc66a1d62bce8e38b0d89192eba0b70056e2f3c Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Dec 2025 18:36:51 +0100 Subject: [PATCH 15/16] 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 16/16] 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), ),