From 29742006df0b0b4a1582882937ec2e964b336f47 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:26:11 +0200 Subject: [PATCH 1/8] Move WindowMode next to the layout types to prevent cyclic import --- src/state/CallViewModel/CallViewModel.ts | 8 +------- src/state/LayoutSwitchViewModel.ts | 2 +- src/state/layout-types.ts | 7 +++++++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 1ab4ea134..831dd0b0e 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -102,6 +102,7 @@ import { type SpotlightExpandedLayoutMedia, type SpotlightLandscapeLayoutMedia, type SpotlightPortraitLayoutMedia, + type WindowMode, } from "../layout-types.ts"; import { ElementCallError, UnknownCallError } from "../../utils/errors.ts"; import { type Epoch, type ObservableScope } from "../ObservableScope.ts"; @@ -271,13 +272,6 @@ const smallMobileCallThreshold = 3; // with the interface const showFooterMs = 4000; -/** - * The general shape of the space the call is drawn in. Called a window because - * that is what it is in the standalone app; for a component it is the container - * the host gave us, which may be a small corner of a large window. - */ -export type WindowMode = "normal" | "narrow" | "flat" | "pip"; - interface LayoutScanState { layout: Layout | null; overflowing: boolean; diff --git a/src/state/LayoutSwitchViewModel.ts b/src/state/LayoutSwitchViewModel.ts index 6e61abaaa..f495c6275 100644 --- a/src/state/LayoutSwitchViewModel.ts +++ b/src/state/LayoutSwitchViewModel.ts @@ -14,7 +14,7 @@ import { switchAll, } from "rxjs"; -import { type WindowMode } from "./CallViewModel/CallViewModel.ts"; +import { type WindowMode } from "./layout-types.ts"; import { constant, type Behavior } from "./Behavior.ts"; import { type ObservableScope } from "./ObservableScope.ts"; diff --git a/src/state/layout-types.ts b/src/state/layout-types.ts index be86f2a36..2d526617b 100644 --- a/src/state/layout-types.ts +++ b/src/state/layout-types.ts @@ -18,6 +18,13 @@ import { import { type Behavior } from "./Behavior.ts"; import { shallowEquals as arrayShallowEquals } from "../utils/array.ts"; +/** + * The general shape of the space the call is drawn in. Called a window because + * that is what it is in the standalone app; for a component it is the container + * the host gave us, which may be a small corner of a large window. + */ +export type WindowMode = "normal" | "narrow" | "flat" | "pip"; + export interface GridLayoutMedia { type: "grid"; edgeToEdge: false; From 10adf6658b82d8a35804b8e07b3b3549d954d8a0 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:27:04 +0200 Subject: [PATCH 2/8] Let layoutShallowEquals compare any layout-shaped object --- src/state/layout-types.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/state/layout-types.ts b/src/state/layout-types.ts index 2d526617b..fd9b36a69 100644 --- a/src/state/layout-types.ts +++ b/src/state/layout-types.ts @@ -150,10 +150,11 @@ export type Layout = | PipLayout; /** - * Tests whether the top-level properties and array elements of layout `a` are - * equal to those of layout `b`. Useful for deduping redundant layout updates. + * Tests whether the top-level properties and array elements of `a` are equal + * to those of `b`. Useful for deduping redundant layout and layout media + * updates. */ -export function layoutShallowEquals(a: Layout, b: Layout): boolean { +export function layoutShallowEquals(a: T, b: T): boolean { // If a and b have the same number of keys and every key in a is also in b, // then they have the same keys. const aKeys = Object.keys(a); From 232e3f7ad47942849fdc193ee147f32b8fda74ac Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:28:14 +0200 Subject: [PATCH 3/8] Extract the spotlight speaker choice into a pure function --- src/state/CallViewModel/CallViewModel.ts | 26 ++-------- src/state/layoutMedia.test.ts | 62 ++++++++++++++++++++++++ src/state/layoutMedia.ts | 33 +++++++++++++ 3 files changed, 100 insertions(+), 21 deletions(-) create mode 100644 src/state/layoutMedia.test.ts create mode 100644 src/state/layoutMedia.ts diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 831dd0b0e..3e92c4e8e 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -104,6 +104,7 @@ import { type SpotlightPortraitLayoutMedia, type WindowMode, } from "../layout-types.ts"; +import { chooseSpotlightSpeaker } from "../layoutMedia.ts"; import { ElementCallError, UnknownCallError } from "../../utils/errors.ts"; import { type Epoch, type ObservableScope } from "../ObservableScope.ts"; import { createHomeserverConnected$ } from "./localMember/HomeserverConnected.ts"; @@ -975,33 +976,16 @@ export function createCallViewModel$( mediaItems.length === 0 ? of([]) : combineLatest( - mediaItems.map((m) => - m.speaking$.pipe(map((s) => [m, s] as const)), + mediaItems.map((media) => + media.speaking$.pipe(map((speaking) => ({ media, speaking }))), ), ), ), scan< - (readonly [UserMediaViewModel, boolean])[], + { media: UserMediaViewModel; speaking: boolean }[], UserMediaViewModel | undefined, undefined - >((prev, mediaItems) => { - // Only remote users that are still in the call should be sticky - const [stickyMedia, stickySpeaking] = - (!prev?.local && mediaItems.find(([m]) => m === prev)) || []; - // Decide who to spotlight: - // If the previous speaker is still speaking, stick with them rather - // than switching eagerly to someone else - return stickySpeaking - ? stickyMedia! - : // Otherwise, select any remote user who is speaking - (mediaItems.find(([m, s]) => !m.local && s)?.[0] ?? - // Otherwise, stick with the person who was last speaking - stickyMedia ?? - // Otherwise, spotlight an arbitrary remote user - mediaItems.find(([m]) => !m.local)?.[0] ?? - // Otherwise, spotlight the local user - mediaItems.find(([m]) => m.local)?.[0]); - }, undefined), + >(chooseSpotlightSpeaker, undefined), ), ); diff --git a/src/state/layoutMedia.test.ts b/src/state/layoutMedia.test.ts new file mode 100644 index 000000000..6936b1143 --- /dev/null +++ b/src/state/layoutMedia.test.ts @@ -0,0 +1,62 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { describe, expect, it } from "vitest"; + +import { chooseSpotlightSpeaker } from "./layoutMedia"; +import { type LocalUserMediaViewModel } from "./media/LocalUserMediaViewModel"; +import { type RemoteUserMediaViewModel } from "./media/RemoteUserMediaViewModel"; + +// The layout functions only look at `type` and `local`, so stubs suffice +const local = { type: "user", local: true } as LocalUserMediaViewModel; +const alice = { type: "user", local: false } as RemoteUserMediaViewModel; +const bob = { type: "user", local: false } as RemoteUserMediaViewModel; +const carol = { type: "user", local: false } as RemoteUserMediaViewModel; + +describe("chooseSpotlightSpeaker", () => { + const items = ( + ...speaking: boolean[] + ): { + media: RemoteUserMediaViewModel | LocalUserMediaViewModel; + speaking: boolean; + }[] => + [local, alice, bob].map((media, i) => ({ + media, + speaking: speaking[i] ?? false, + })); + + it("prefers a remote speaker over the local user", () => { + expect(chooseSpotlightSpeaker(undefined, items(true, false, true))).toBe( + bob, + ); + }); + + it("falls back to any remote user, then the local user", () => { + expect(chooseSpotlightSpeaker(undefined, items())).toBe(alice); + expect( + chooseSpotlightSpeaker(undefined, [{ media: local, speaking: false }]), + ).toBe(local); + expect(chooseSpotlightSpeaker(undefined, [])).toBeUndefined(); + }); + + it("sticks with the previous speaker while they keep speaking", () => { + expect(chooseSpotlightSpeaker(bob, items(false, true, true))).toBe(bob); + }); + + it("sticks with the previous speaker when nobody is speaking", () => { + expect(chooseSpotlightSpeaker(bob, items())).toBe(bob); + }); + + it("switches when someone else speaks and the previous speaker is silent", () => { + expect(chooseSpotlightSpeaker(bob, items(false, true, false))).toBe(alice); + }); + + it("forgets a previous speaker who left, and never sticks to the local user", () => { + expect(chooseSpotlightSpeaker(carol, items())).toBe(alice); + expect(chooseSpotlightSpeaker(local, items())).toBe(alice); + }); +}); diff --git a/src/state/layoutMedia.ts b/src/state/layoutMedia.ts new file mode 100644 index 000000000..3995f0216 --- /dev/null +++ b/src/state/layoutMedia.ts @@ -0,0 +1,33 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +/** + * Picks who to spotlight based on who is speaking, preferring the previous + * pick so that the spotlight doesn't flip eagerly between speakers. + * + * @param prev The previous pick, if any. + * @param items All user media in the call along with whether each is speaking. + */ +export function chooseSpotlightSpeaker( + prev: M | undefined, + items: readonly { media: M; speaking: boolean }[], +): M | undefined { + // Only remote users that are still in the call should be sticky + const sticky = items.find((i) => i.media === prev && !i.media.local); + // If the previous speaker is still speaking, stick with them + if (sticky?.speaking) return sticky.media; + return ( + // Otherwise, select any remote user who is speaking + items.find((i) => !i.media.local && i.speaking)?.media ?? + // Otherwise, stick with the person who was last speaking + sticky?.media ?? + // Otherwise, spotlight an arbitrary remote user + items.find((i) => !i.media.local)?.media ?? + // Otherwise, spotlight the local user + items.find((i) => i.media.local)?.media + ); +} From 5a8714805c8913c1c8e3e2a31cb9769b8da156c3 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:29:41 +0200 Subject: [PATCH 4/8] Extract the spotlight and PiP choice into a pure function --- src/state/CallViewModel/CallViewModel.ts | 51 ++++++----------------- src/state/layoutMedia.test.ts | 52 +++++++++++++++++++++++- src/state/layoutMedia.ts | 35 ++++++++++++++++ 3 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 3e92c4e8e..f171f8947 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -104,7 +104,7 @@ import { type SpotlightPortraitLayoutMedia, type WindowMode, } from "../layout-types.ts"; -import { chooseSpotlightSpeaker } from "../layoutMedia.ts"; +import { chooseSpotlightSpeaker, computeSpotlight } from "../layoutMedia.ts"; import { ElementCallError, UnknownCallError } from "../../utils/errors.ts"; import { type Epoch, type ObservableScope } from "../ObservableScope.ts"; import { createHomeserverConnected$ } from "./localMember/HomeserverConnected.ts"; @@ -1029,33 +1029,12 @@ export function createCallViewModel$( ), ); - const spotlightAndPip$ = scope.behavior<{ - spotlight: MediaViewModel[]; - pip$: Observable; - }>( - ringingMedia$.pipe( - switchMap((ringingMedia) => { - if (ringingMedia !== null) - return of({ spotlight: [ringingMedia], pip$: localUserMediaForPip$ }); - - return screenShares$.pipe( - switchMap((screenShares) => { - if (screenShares.length > 0) - return of({ spotlight: screenShares, pip$: spotlightSpeaker$ }); - - return spotlightSpeaker$.pipe( - map((speaker) => ({ - spotlight: speaker ? [speaker] : [], - // Hide PiP if redundant (i.e. if local user is already in spotlight) - pip$: localUserMediaForPip$.pipe( - map((m) => (m === speaker ? undefined : m)), - ), - })), - ); - }), - ); - }), - ), + const spotlightAndPip$ = scope.behavior( + combineLatest( + [ringingMedia$, screenShares$, spotlightSpeaker$, localUserMediaForPip$], + (ringing, screenShares, speaker, localPip) => + computeSpotlight({ ringing, screenShares, speaker, localPip }), + ).pipe(distinctUntilChanged(layoutShallowEquals)), ); const spotlight$ = scope.behavior( @@ -1147,16 +1126,12 @@ export function createCallViewModel$( edgeToEdge: boolean, ): Observable => spotlightAndPip$.pipe( - switchMap(({ spotlight, pip$ }) => - pip$.pipe( - map((pip) => ({ - type: "spotlight-expanded" as const, - edgeToEdge, - spotlight, - pip: pip ?? undefined, - })), - ), - ), + map(({ spotlight, pip }) => ({ + type: "spotlight-expanded" as const, + edgeToEdge, + spotlight, + pip, + })), ); const oneOnOneLayoutMedia$: Behavior<{ diff --git a/src/state/layoutMedia.test.ts b/src/state/layoutMedia.test.ts index 6936b1143..8e164cc9a 100644 --- a/src/state/layoutMedia.test.ts +++ b/src/state/layoutMedia.test.ts @@ -7,15 +7,22 @@ Please see LICENSE in the repository root for full details. import { describe, expect, it } from "vitest"; -import { chooseSpotlightSpeaker } from "./layoutMedia"; +import { chooseSpotlightSpeaker, computeSpotlight } from "./layoutMedia"; import { type LocalUserMediaViewModel } from "./media/LocalUserMediaViewModel"; import { type RemoteUserMediaViewModel } from "./media/RemoteUserMediaViewModel"; +import { type RingingMediaViewModel } from "./media/RingingMediaViewModel"; +import { type ScreenShareViewModel } from "./media/ScreenShareViewModel"; // The layout functions only look at `type` and `local`, so stubs suffice const local = { type: "user", local: true } as LocalUserMediaViewModel; const alice = { type: "user", local: false } as RemoteUserMediaViewModel; const bob = { type: "user", local: false } as RemoteUserMediaViewModel; const carol = { type: "user", local: false } as RemoteUserMediaViewModel; +const screenShare = { + type: "screen share", + local: false, +} as ScreenShareViewModel; +const ringing = { type: "ringing" } as RingingMediaViewModel; describe("chooseSpotlightSpeaker", () => { const items = ( @@ -60,3 +67,46 @@ describe("chooseSpotlightSpeaker", () => { expect(chooseSpotlightSpeaker(local, items())).toBe(alice); }); }); + +describe("computeSpotlight", () => { + it("spotlights ringing media with the local user in the PiP", () => { + expect( + computeSpotlight({ + ringing, + screenShares: [screenShare], + speaker: alice, + localPip: local, + }), + ).toEqual({ spotlight: [ringing], pip: local }); + }); + + it("spotlights screen shares with the speaker in the PiP", () => { + expect( + computeSpotlight({ + ringing: null, + screenShares: [screenShare], + speaker: alice, + localPip: local, + }), + ).toEqual({ spotlight: [screenShare], pip: alice }); + }); + + it("otherwise spotlights the speaker and hides a redundant PiP", () => { + expect( + computeSpotlight({ + ringing: null, + screenShares: [], + speaker: alice, + localPip: local, + }), + ).toEqual({ spotlight: [alice], pip: local }); + expect( + computeSpotlight({ + ringing: null, + screenShares: [], + speaker: local, + localPip: local, + }), + ).toEqual({ spotlight: [local], pip: undefined }); + }); +}); diff --git a/src/state/layoutMedia.ts b/src/state/layoutMedia.ts index 3995f0216..64365b3a8 100644 --- a/src/state/layoutMedia.ts +++ b/src/state/layoutMedia.ts @@ -5,6 +5,11 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ +import { type MediaViewModel } from "./media/MediaViewModel.ts"; +import { type RingingMediaViewModel } from "./media/RingingMediaViewModel.ts"; +import { type ScreenShareViewModel } from "./media/ScreenShareViewModel.ts"; +import { type UserMediaViewModel } from "./media/UserMediaViewModel.ts"; + /** * Picks who to spotlight based on who is speaking, preferring the previous * pick so that the spotlight doesn't flip eagerly between speakers. @@ -31,3 +36,33 @@ export function chooseSpotlightSpeaker( items.find((i) => i.media.local)?.media ); } + +export interface SpotlightInputs { + ringing: RingingMediaViewModel | null; + screenShares: ScreenShareViewModel[]; + speaker: UserMediaViewModel | undefined; + /** Local user media suitable for showing in a PiP, if any. */ + localPip: UserMediaViewModel | undefined; +} + +/** + * Decides what goes in the spotlight, and what goes in the picture-in-picture + * tile of the expanded spotlight layout. + */ +export function computeSpotlight({ + ringing, + screenShares, + speaker, + localPip, +}: SpotlightInputs): { + spotlight: MediaViewModel[]; + pip: UserMediaViewModel | undefined; +} { + if (ringing !== null) return { spotlight: [ringing], pip: localPip }; + if (screenShares.length > 0) return { spotlight: screenShares, pip: speaker }; + return { + spotlight: speaker ? [speaker] : [], + // Hide PiP if redundant (i.e. if local user is already in spotlight) + pip: localPip === speaker ? undefined : localPip, + }; +} From 33cfd2d8adb62921d947d4f51e2e286be5b76e99 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:34:22 +0200 Subject: [PATCH 5/8] Build the layout media from one snapshot of its inputs --- src/state/CallViewModel/CallViewModel.ts | 306 ++++++++++++----------- 1 file changed, 159 insertions(+), 147 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index f171f8947..76675c824 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -30,7 +30,6 @@ import { scan, startWith, Subject, - switchAll, switchMap, switchScan, take, @@ -148,6 +147,7 @@ import { import { Publisher } from "./localMember/Publisher.ts"; import { type Connection } from "./remoteMembers/Connection.ts"; import { + type LayoutMode, type LayoutSwitchViewModel, createLayoutSwitchViewModel, } from "../LayoutSwitchViewModel.ts"; @@ -1092,52 +1092,7 @@ export function createCallViewModel$( hasRemoteScreenShares$, ); - const gridLayoutMedia$: Observable = combineLatest( - [grid$, spotlight$], - (grid, spotlight) => ({ - type: "grid", - edgeToEdge: false, - spotlight: spotlight.some((vm) => vm.type === "screen share") - ? spotlight - : undefined, - grid, - }), - ); - - const spotlightLandscapeLayoutMedia$ = ( - edgeToEdge: boolean, - ): Observable => - combineLatest([grid$, spotlight$], (grid, spotlight) => ({ - type: "spotlight-landscape", - edgeToEdge, - spotlight, - grid, - })); - - const spotlightPortraitLayoutMedia$: Observable = - combineLatest([grid$, spotlight$], (grid, spotlight) => ({ - type: "spotlight-portrait", - edgeToEdge: false, - spotlight, - grid, - })); - - const spotlightExpandedLayoutMedia$ = ( - edgeToEdge: boolean, - ): Observable => - spotlightAndPip$.pipe( - map(({ spotlight, pip }) => ({ - type: "spotlight-expanded" as const, - edgeToEdge, - spotlight, - pip, - })), - ); - - const oneOnOneLayoutMedia$: Behavior<{ - local: LocalUserMediaViewModel; - remote: UserMediaViewModel | RingingMediaViewModel; - } | null> = scope.behavior( + const oneOnOneLayoutMedia$: Behavior = scope.behavior( combineLatest([userMedia$, screenShares$]).pipe( switchMap(([userMedia, screenShares]) => { // One-on-one layout only supports 2 user media, no screen shares @@ -1175,47 +1130,12 @@ export function createCallViewModel$( ), ); - const oneOnOneDesktopLayoutMedia$: Observable = + const localVideoEnabled$ = scope.behavior( oneOnOneLayoutMedia$.pipe( - map((media) => { - if (media === null) return null; - return media.remote.type === "ringing" - ? { - type: "one-on-one-desktop" as const, - edgeToEdge: false, - spotlight: media.local, - pip: media.remote, - } - : { - type: "one-on-one-desktop" as const, - edgeToEdge: false, - spotlight: media.remote, - pip: media.local, - }; - }), - ); - - const oneOnOneMobileLayoutMedia$: Observable = - oneOnOneLayoutMedia$.pipe( - switchMap((media) => { - if (media === null) return of(null); - return media.local.videoEnabled$.pipe( - map((videoEnabled) => ({ - type: "one-on-one-mobile" as const, - edgeToEdge: true as const, - spotlight: media.remote, - pip: videoEnabled ? media.local : undefined, - })), - ); - }), - ); - - const pipLayoutMedia$: Observable = spotlight$.pipe( - map((spotlight) => ({ - type: "pip", - edgeToEdge: platform !== "desktop", - spotlight, - })), + switchMap((media) => + media === null ? of(false) : media.local.videoEnabled$, + ), + ), ); spotlight$ @@ -1250,66 +1170,28 @@ export function createCallViewModel$( * The media to be used to produce a layout. */ const layoutMedia$ = scope.behavior( - windowMode$.pipe( - switchMap((windowMode) => { - switch (windowMode) { - case "normal": - return layoutSwitchVm.layout$.pipe( - switchMap((layout) => { - switch (layout) { - case "grid": - return oneOnOneDesktopLayoutMedia$.pipe( - switchMap((oneOnOne) => - oneOnOne === null ? gridLayoutMedia$ : of(oneOnOne), - ), - ); - case "spotlight": - return spotlightExpanded$.pipe( - switchMap((expanded) => - expanded - ? spotlightExpandedLayoutMedia$(false) - : spotlightLandscapeLayoutMedia$(false), - ), - ); - } - }), - ); - case "narrow": - return oneOnOneMobileLayoutMedia$.pipe( - switchMap((oneOnOne) => - oneOnOne === null - ? combineLatest([grid$, spotlight$], (grid, spotlight) => - grid.length > smallMobileCallThreshold || - spotlight.some((vm) => vm.type === "screen share") - ? spotlightPortraitLayoutMedia$ - : gridLayoutMedia$, - ).pipe(switchAll()) - : of(oneOnOne), - ), - ); - case "flat": - return oneOnOneMobileLayoutMedia$.pipe( - switchMap((oneOnOne) => - oneOnOne === null - ? layoutSwitchVm.layout$.pipe( - switchMap((layout) => { - switch (layout) { - case "grid": - // Yes, grid mode actually gets you a "spotlight" layout in - // this window mode. - return spotlightLandscapeLayoutMedia$(true); - case "spotlight": - return spotlightExpandedLayoutMedia$(true); - } - }), - ) - : of(oneOnOne), - ), - ); - case "pip": - return pipLayoutMedia$; - } - }), + merge( + windowMode$, + layoutSwitchVm.layout$, + spotlightExpanded$, + oneOnOneLayoutMedia$, + localVideoEnabled$, + spotlightAndPip$, + grid$, + ).pipe( + map(() => + computeLayoutMedia({ + windowMode: windowMode$.value, + layoutMode: layoutSwitchVm.layout$.value, + spotlightExpanded: spotlightExpanded$.value, + oneOnOne: oneOnOneLayoutMedia$.value, + localVideoEnabled: localVideoEnabled$.value, + ...spotlightAndPip$.value, + grid: grid$.value, + desktop: platform === "desktop", + }), + ), + distinctUntilChanged(layoutShallowEquals), ), ); @@ -1891,3 +1773,133 @@ function getE2eeKeyProvider( return keyProvider; } } + +interface OneOnOneMedia { + local: LocalUserMediaViewModel; + remote: UserMediaViewModel | RingingMediaViewModel; +} + +interface LayoutMediaInputs { + windowMode: WindowMode; + /** The layout chosen with the layout switch. */ + layoutMode: LayoutMode; + spotlightExpanded: boolean; + /** Set when the call qualifies for a one-on-one layout. */ + oneOnOne: OneOnOneMedia | null; + localVideoEnabled: boolean; + spotlight: MediaViewModel[]; + grid: UserMediaViewModel[]; + pip: UserMediaViewModel | undefined; + desktop: boolean; +} + +/** + * Decides which layout to use and which media goes where in it. + */ +function computeLayoutMedia({ + windowMode, + layoutMode, + spotlightExpanded, + oneOnOne, + localVideoEnabled, + spotlight, + grid, + pip, + desktop, +}: LayoutMediaInputs): LayoutMedia { + switch (windowMode) { + case "normal": + if (layoutMode === "grid") + return oneOnOne === null + ? gridLayoutMedia(spotlight, grid) + : oneOnOneDesktopLayoutMedia(oneOnOne); + return spotlightExpanded + ? spotlightExpandedLayoutMedia(false, spotlight, pip) + : spotlightLandscapeLayoutMedia(false, spotlight, grid); + case "narrow": + if (oneOnOne !== null) + return oneOnOneMobileLayoutMedia(oneOnOne, localVideoEnabled); + return grid.length > smallMobileCallThreshold || hasScreenShare(spotlight) + ? spotlightPortraitLayoutMedia(spotlight, grid) + : gridLayoutMedia(spotlight, grid); + case "flat": + if (oneOnOne !== null) + return oneOnOneMobileLayoutMedia(oneOnOne, localVideoEnabled); + // Yes, grid mode actually gets you a "spotlight" layout in this window + // mode. + return layoutMode === "grid" + ? spotlightLandscapeLayoutMedia(true, spotlight, grid) + : spotlightExpandedLayoutMedia(true, spotlight, pip); + case "pip": + return { type: "pip", edgeToEdge: !desktop, spotlight }; + } +} + +function hasScreenShare(spotlight: MediaViewModel[]): boolean { + return spotlight.some((vm) => vm.type === "screen share"); +} + +function gridLayoutMedia( + spotlight: MediaViewModel[], + grid: UserMediaViewModel[], +): GridLayoutMedia { + return { + type: "grid", + edgeToEdge: false, + spotlight: hasScreenShare(spotlight) ? spotlight : undefined, + grid, + }; +} + +function oneOnOneDesktopLayoutMedia( + media: OneOnOneMedia, +): OneOnOneDesktopLayoutMedia { + return media.remote.type === "ringing" + ? { + type: "one-on-one-desktop", + edgeToEdge: false, + spotlight: media.local, + pip: media.remote, + } + : { + type: "one-on-one-desktop", + edgeToEdge: false, + spotlight: media.remote, + pip: media.local, + }; +} + +function spotlightExpandedLayoutMedia( + edgeToEdge: boolean, + spotlight: MediaViewModel[], + pip: UserMediaViewModel | undefined, +): SpotlightExpandedLayoutMedia { + return { type: "spotlight-expanded", edgeToEdge, spotlight, pip }; +} + +function spotlightLandscapeLayoutMedia( + edgeToEdge: boolean, + spotlight: MediaViewModel[], + grid: UserMediaViewModel[], +): SpotlightLandscapeLayoutMedia { + return { type: "spotlight-landscape", edgeToEdge, spotlight, grid }; +} + +function oneOnOneMobileLayoutMedia( + media: OneOnOneMedia, + localVideoEnabled: boolean, +): OneOnOneMobileLayoutMedia { + return { + type: "one-on-one-mobile", + edgeToEdge: true, + spotlight: media.remote, + pip: localVideoEnabled ? media.local : undefined, + }; +} + +function spotlightPortraitLayoutMedia( + spotlight: MediaViewModel[], + grid: UserMediaViewModel[], +): SpotlightPortraitLayoutMedia { + return { type: "spotlight-portrait", edgeToEdge: false, spotlight, grid }; +} From 6600c891f339151c20e4ba69ed09a7ac7fa13b73 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Mon, 28 Sep 2026 17:36:24 +0200 Subject: [PATCH 6/8] Extract the layout decision into a pure function --- src/state/CallViewModel/CallViewModel.ts | 148 +---------------------- src/state/layoutMedia.test.ts | 110 ++++++++++++++++- src/state/layoutMedia.ts | 148 +++++++++++++++++++++++ 3 files changed, 263 insertions(+), 143 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 76675c824..ae6a54697 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -93,17 +93,16 @@ import { type HostBridge, nullHostBridge } from "../../HostBridge"; import { layoutShallowEquals, type Alignment, - type GridLayoutMedia, type Layout, type LayoutMedia, - type OneOnOneDesktopLayoutMedia, - type OneOnOneMobileLayoutMedia, - type SpotlightExpandedLayoutMedia, - type SpotlightLandscapeLayoutMedia, - type SpotlightPortraitLayoutMedia, type WindowMode, } from "../layout-types.ts"; -import { chooseSpotlightSpeaker, computeSpotlight } from "../layoutMedia.ts"; +import { + chooseSpotlightSpeaker, + computeLayoutMedia, + computeSpotlight, + type OneOnOneMedia, +} from "../layoutMedia.ts"; import { ElementCallError, UnknownCallError } from "../../utils/errors.ts"; import { type Epoch, type ObservableScope } from "../ObservableScope.ts"; import { createHomeserverConnected$ } from "./localMember/HomeserverConnected.ts"; @@ -147,7 +146,6 @@ import { import { Publisher } from "./localMember/Publisher.ts"; import { type Connection } from "./remoteMembers/Connection.ts"; import { - type LayoutMode, type LayoutSwitchViewModel, createLayoutSwitchViewModel, } from "../LayoutSwitchViewModel.ts"; @@ -265,10 +263,6 @@ export function callViewModelOptionsFromParams( export const MAX_PARTICIPANT_COUNT_FOR_SOUND = 8; export const THROTTLE_SOUND_EFFECT_MS = 500; -// This is the number of participants that we think constitutes a "small" call -// on mobile. No spotlight tile should be shown below this threshold. -const smallMobileCallThreshold = 3; - // How long the footer should be shown for when hovering over or interacting // with the interface const showFooterMs = 4000; @@ -1773,133 +1767,3 @@ function getE2eeKeyProvider( return keyProvider; } } - -interface OneOnOneMedia { - local: LocalUserMediaViewModel; - remote: UserMediaViewModel | RingingMediaViewModel; -} - -interface LayoutMediaInputs { - windowMode: WindowMode; - /** The layout chosen with the layout switch. */ - layoutMode: LayoutMode; - spotlightExpanded: boolean; - /** Set when the call qualifies for a one-on-one layout. */ - oneOnOne: OneOnOneMedia | null; - localVideoEnabled: boolean; - spotlight: MediaViewModel[]; - grid: UserMediaViewModel[]; - pip: UserMediaViewModel | undefined; - desktop: boolean; -} - -/** - * Decides which layout to use and which media goes where in it. - */ -function computeLayoutMedia({ - windowMode, - layoutMode, - spotlightExpanded, - oneOnOne, - localVideoEnabled, - spotlight, - grid, - pip, - desktop, -}: LayoutMediaInputs): LayoutMedia { - switch (windowMode) { - case "normal": - if (layoutMode === "grid") - return oneOnOne === null - ? gridLayoutMedia(spotlight, grid) - : oneOnOneDesktopLayoutMedia(oneOnOne); - return spotlightExpanded - ? spotlightExpandedLayoutMedia(false, spotlight, pip) - : spotlightLandscapeLayoutMedia(false, spotlight, grid); - case "narrow": - if (oneOnOne !== null) - return oneOnOneMobileLayoutMedia(oneOnOne, localVideoEnabled); - return grid.length > smallMobileCallThreshold || hasScreenShare(spotlight) - ? spotlightPortraitLayoutMedia(spotlight, grid) - : gridLayoutMedia(spotlight, grid); - case "flat": - if (oneOnOne !== null) - return oneOnOneMobileLayoutMedia(oneOnOne, localVideoEnabled); - // Yes, grid mode actually gets you a "spotlight" layout in this window - // mode. - return layoutMode === "grid" - ? spotlightLandscapeLayoutMedia(true, spotlight, grid) - : spotlightExpandedLayoutMedia(true, spotlight, pip); - case "pip": - return { type: "pip", edgeToEdge: !desktop, spotlight }; - } -} - -function hasScreenShare(spotlight: MediaViewModel[]): boolean { - return spotlight.some((vm) => vm.type === "screen share"); -} - -function gridLayoutMedia( - spotlight: MediaViewModel[], - grid: UserMediaViewModel[], -): GridLayoutMedia { - return { - type: "grid", - edgeToEdge: false, - spotlight: hasScreenShare(spotlight) ? spotlight : undefined, - grid, - }; -} - -function oneOnOneDesktopLayoutMedia( - media: OneOnOneMedia, -): OneOnOneDesktopLayoutMedia { - return media.remote.type === "ringing" - ? { - type: "one-on-one-desktop", - edgeToEdge: false, - spotlight: media.local, - pip: media.remote, - } - : { - type: "one-on-one-desktop", - edgeToEdge: false, - spotlight: media.remote, - pip: media.local, - }; -} - -function spotlightExpandedLayoutMedia( - edgeToEdge: boolean, - spotlight: MediaViewModel[], - pip: UserMediaViewModel | undefined, -): SpotlightExpandedLayoutMedia { - return { type: "spotlight-expanded", edgeToEdge, spotlight, pip }; -} - -function spotlightLandscapeLayoutMedia( - edgeToEdge: boolean, - spotlight: MediaViewModel[], - grid: UserMediaViewModel[], -): SpotlightLandscapeLayoutMedia { - return { type: "spotlight-landscape", edgeToEdge, spotlight, grid }; -} - -function oneOnOneMobileLayoutMedia( - media: OneOnOneMedia, - localVideoEnabled: boolean, -): OneOnOneMobileLayoutMedia { - return { - type: "one-on-one-mobile", - edgeToEdge: true, - spotlight: media.remote, - pip: localVideoEnabled ? media.local : undefined, - }; -} - -function spotlightPortraitLayoutMedia( - spotlight: MediaViewModel[], - grid: UserMediaViewModel[], -): SpotlightPortraitLayoutMedia { - return { type: "spotlight-portrait", edgeToEdge: false, spotlight, grid }; -} diff --git a/src/state/layoutMedia.test.ts b/src/state/layoutMedia.test.ts index 8e164cc9a..afa159877 100644 --- a/src/state/layoutMedia.test.ts +++ b/src/state/layoutMedia.test.ts @@ -7,7 +7,12 @@ Please see LICENSE in the repository root for full details. import { describe, expect, it } from "vitest"; -import { chooseSpotlightSpeaker, computeSpotlight } from "./layoutMedia"; +import { + chooseSpotlightSpeaker, + computeLayoutMedia, + computeSpotlight, + type LayoutMediaInputs, +} from "./layoutMedia"; import { type LocalUserMediaViewModel } from "./media/LocalUserMediaViewModel"; import { type RemoteUserMediaViewModel } from "./media/RemoteUserMediaViewModel"; import { type RingingMediaViewModel } from "./media/RingingMediaViewModel"; @@ -110,3 +115,106 @@ describe("computeSpotlight", () => { ).toEqual({ spotlight: [local], pip: undefined }); }); }); + +describe("computeLayoutMedia", () => { + const base: LayoutMediaInputs = { + windowMode: "normal", + layoutMode: "grid", + spotlightExpanded: false, + oneOnOne: null, + localVideoEnabled: true, + spotlight: [alice], + grid: [local, alice, bob], + pip: local, + desktop: false, + }; + const layout = ( + overrides: Partial, + ): ReturnType => + computeLayoutMedia({ ...base, ...overrides }); + + it("uses a plain grid unless a screen share needs the spotlight", () => { + expect(layout({})).toEqual({ + type: "grid", + edgeToEdge: false, + spotlight: undefined, + grid: base.grid, + }); + expect(layout({ spotlight: [screenShare] }).spotlight).toEqual([ + screenShare, + ]); + }); + + it("switches between landscape and expanded spotlight", () => { + expect(layout({ layoutMode: "spotlight" })).toMatchObject({ + type: "spotlight-landscape", + edgeToEdge: false, + }); + expect( + layout({ layoutMode: "spotlight", spotlightExpanded: true }), + ).toEqual({ + type: "spotlight-expanded", + edgeToEdge: false, + spotlight: [alice], + pip: local, + }); + }); + + it("puts the remote user in the desktop one-on-one spotlight, or the local user when ringing", () => { + expect(layout({ oneOnOne: { local, remote: alice } })).toEqual({ + type: "one-on-one-desktop", + edgeToEdge: false, + spotlight: alice, + pip: local, + }); + expect(layout({ oneOnOne: { local, remote: ringing } })).toMatchObject({ + spotlight: local, + pip: ringing, + }); + }); + + it("uses a mobile one-on-one layout in narrow and flat windows, hiding the PiP without video", () => { + const oneOnOne = { local, remote: alice }; + expect(layout({ windowMode: "narrow", oneOnOne })).toEqual({ + type: "one-on-one-mobile", + edgeToEdge: true, + spotlight: alice, + pip: local, + }); + expect( + layout({ windowMode: "flat", oneOnOne, localVideoEnabled: false }), + ).toMatchObject({ type: "one-on-one-mobile", pip: undefined }); + }); + + it("uses a portrait spotlight in narrow windows for big calls or screen shares", () => { + expect(layout({ windowMode: "narrow" }).type).toBe("grid"); + expect( + layout({ windowMode: "narrow", grid: [local, alice, bob, carol] }).type, + ).toBe("spotlight-portrait"); + expect( + layout({ windowMode: "narrow", spotlight: [screenShare] }).type, + ).toBe("spotlight-portrait"); + }); + + it("uses edge-to-edge spotlight layouts in flat windows", () => { + expect(layout({ windowMode: "flat" })).toMatchObject({ + type: "spotlight-landscape", + edgeToEdge: true, + }); + expect( + layout({ windowMode: "flat", layoutMode: "spotlight" }), + ).toMatchObject({ + type: "spotlight-expanded", + edgeToEdge: true, + }); + }); + + it("uses the PiP layout, edge to edge except on desktop", () => { + expect(layout({ windowMode: "pip" })).toEqual({ + type: "pip", + edgeToEdge: true, + spotlight: [alice], + }); + expect(layout({ windowMode: "pip", desktop: true }).edgeToEdge).toBe(false); + }); +}); diff --git a/src/state/layoutMedia.ts b/src/state/layoutMedia.ts index 64365b3a8..56dec9979 100644 --- a/src/state/layoutMedia.ts +++ b/src/state/layoutMedia.ts @@ -5,11 +5,29 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ +import { type LayoutMode } from "./LayoutSwitchViewModel.ts"; +import { + type GridLayoutMedia, + type LayoutMedia, + type OneOnOneDesktopLayoutMedia, + type OneOnOneMobileLayoutMedia, + type SpotlightExpandedLayoutMedia, + type SpotlightLandscapeLayoutMedia, + type SpotlightPortraitLayoutMedia, + type WindowMode, +} from "./layout-types.ts"; +import { type LocalUserMediaViewModel } from "./media/LocalUserMediaViewModel.ts"; import { type MediaViewModel } from "./media/MediaViewModel.ts"; import { type RingingMediaViewModel } from "./media/RingingMediaViewModel.ts"; import { type ScreenShareViewModel } from "./media/ScreenShareViewModel.ts"; import { type UserMediaViewModel } from "./media/UserMediaViewModel.ts"; +/** + * This is the number of participants that we think constitutes a "small" call + * on mobile. No spotlight tile should be shown below this threshold. + */ +const smallMobileCallThreshold = 3; + /** * Picks who to spotlight based on who is speaking, preferring the previous * pick so that the spotlight doesn't flip eagerly between speakers. @@ -66,3 +84,133 @@ export function computeSpotlight({ pip: localPip === speaker ? undefined : localPip, }; } + +export interface OneOnOneMedia { + local: LocalUserMediaViewModel; + remote: UserMediaViewModel | RingingMediaViewModel; +} + +export interface LayoutMediaInputs { + windowMode: WindowMode; + /** The layout chosen with the layout switch. */ + layoutMode: LayoutMode; + spotlightExpanded: boolean; + /** Set when the call qualifies for a one-on-one layout. */ + oneOnOne: OneOnOneMedia | null; + localVideoEnabled: boolean; + spotlight: MediaViewModel[]; + grid: UserMediaViewModel[]; + pip: UserMediaViewModel | undefined; + desktop: boolean; +} + +/** + * Decides which layout to use and which media goes where in it. + */ +export function computeLayoutMedia({ + windowMode, + layoutMode, + spotlightExpanded, + oneOnOne, + localVideoEnabled, + spotlight, + grid, + pip, + desktop, +}: LayoutMediaInputs): LayoutMedia { + switch (windowMode) { + case "normal": + if (layoutMode === "grid") + return oneOnOne === null + ? gridLayoutMedia(spotlight, grid) + : oneOnOneDesktopLayoutMedia(oneOnOne); + return spotlightExpanded + ? spotlightExpandedLayoutMedia(false, spotlight, pip) + : spotlightLandscapeLayoutMedia(false, spotlight, grid); + case "narrow": + if (oneOnOne !== null) + return oneOnOneMobileLayoutMedia(oneOnOne, localVideoEnabled); + return grid.length > smallMobileCallThreshold || hasScreenShare(spotlight) + ? spotlightPortraitLayoutMedia(spotlight, grid) + : gridLayoutMedia(spotlight, grid); + case "flat": + if (oneOnOne !== null) + return oneOnOneMobileLayoutMedia(oneOnOne, localVideoEnabled); + // Yes, grid mode actually gets you a "spotlight" layout in this window + // mode. + return layoutMode === "grid" + ? spotlightLandscapeLayoutMedia(true, spotlight, grid) + : spotlightExpandedLayoutMedia(true, spotlight, pip); + case "pip": + return { type: "pip", edgeToEdge: !desktop, spotlight }; + } +} + +function hasScreenShare(spotlight: MediaViewModel[]): boolean { + return spotlight.some((vm) => vm.type === "screen share"); +} + +function gridLayoutMedia( + spotlight: MediaViewModel[], + grid: UserMediaViewModel[], +): GridLayoutMedia { + return { + type: "grid", + edgeToEdge: false, + spotlight: hasScreenShare(spotlight) ? spotlight : undefined, + grid, + }; +} + +function oneOnOneDesktopLayoutMedia( + media: OneOnOneMedia, +): OneOnOneDesktopLayoutMedia { + return media.remote.type === "ringing" + ? { + type: "one-on-one-desktop", + edgeToEdge: false, + spotlight: media.local, + pip: media.remote, + } + : { + type: "one-on-one-desktop", + edgeToEdge: false, + spotlight: media.remote, + pip: media.local, + }; +} + +function spotlightExpandedLayoutMedia( + edgeToEdge: boolean, + spotlight: MediaViewModel[], + pip: UserMediaViewModel | undefined, +): SpotlightExpandedLayoutMedia { + return { type: "spotlight-expanded", edgeToEdge, spotlight, pip }; +} + +function spotlightLandscapeLayoutMedia( + edgeToEdge: boolean, + spotlight: MediaViewModel[], + grid: UserMediaViewModel[], +): SpotlightLandscapeLayoutMedia { + return { type: "spotlight-landscape", edgeToEdge, spotlight, grid }; +} + +function oneOnOneMobileLayoutMedia( + media: OneOnOneMedia, + localVideoEnabled: boolean, +): OneOnOneMobileLayoutMedia { + return { + type: "one-on-one-mobile", + edgeToEdge: true, + spotlight: media.remote, + pip: localVideoEnabled ? media.local : undefined, + }; +} + +function spotlightPortraitLayoutMedia( + spotlight: MediaViewModel[], + grid: UserMediaViewModel[], +): SpotlightPortraitLayoutMedia { + return { type: "spotlight-portrait", edgeToEdge: false, spotlight, grid }; +} From 53e038b09b4e2099caea8fecc5069f4fe6ef3036 Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Thu, 8 Oct 2026 10:57:03 +0200 Subject: [PATCH 7/8] layoutShallowEquals -> shallowEquals --- src/state/CallViewModel/CallViewModel.ts | 8 +-- src/state/layout-types.test.ts | 74 ------------------------ src/state/layout-types.ts | 29 ---------- src/utils/object.test.ts | 40 +++++++++++++ src/utils/object.ts | 38 ++++++++++++ 5 files changed, 82 insertions(+), 107 deletions(-) delete mode 100644 src/state/layout-types.test.ts create mode 100644 src/utils/object.test.ts create mode 100644 src/utils/object.ts diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index ae6a54697..083666001 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -82,6 +82,7 @@ import { type ReactionOption, } from "../../reactions"; import { shallowEquals as shallowArrayEquals } from "../../utils/array"; +import { shallowEquals as shallowObjectEquals } from "../../utils/object"; import { type MediaDevices } from "../MediaDevices"; import { constant, type Behavior } from "../Behavior"; import { E2eeType } from "../../e2ee/e2eeType"; @@ -91,7 +92,6 @@ import { HeaderStyle, type UrlParams } from "../../UrlParams"; import { type ProcessorState } from "../../livekit/TrackProcessorContext"; import { type HostBridge, nullHostBridge } from "../../HostBridge"; import { - layoutShallowEquals, type Alignment, type Layout, type LayoutMedia, @@ -1028,7 +1028,7 @@ export function createCallViewModel$( [ringingMedia$, screenShares$, spotlightSpeaker$, localUserMediaForPip$], (ringing, screenShares, speaker, localPip) => computeSpotlight({ ringing, screenShares, speaker, localPip }), - ).pipe(distinctUntilChanged(layoutShallowEquals)), + ).pipe(distinctUntilChanged(shallowObjectEquals)), ); const spotlight$ = scope.behavior( @@ -1185,7 +1185,7 @@ export function createCallViewModel$( desktop: platform === "desktop", }), ), - distinctUntilChanged(layoutShallowEquals), + distinctUntilChanged(shallowObjectEquals), ), ); @@ -1486,7 +1486,7 @@ export function createCallViewModel$( layoutInternals$.pipe( map(({ layout }) => layout), // Drop redundant layout updates before they would hit React. - distinctUntilChanged(layoutShallowEquals), + distinctUntilChanged(shallowObjectEquals), ), ); diff --git a/src/state/layout-types.test.ts b/src/state/layout-types.test.ts deleted file mode 100644 index 468c15bb5..000000000 --- a/src/state/layout-types.test.ts +++ /dev/null @@ -1,74 +0,0 @@ -/* -Copyright 2026 Element Creations Ltd. - -SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial -Please see LICENSE in the repository root for full details. -*/ - -import { test, expect } from "vitest"; -import { - type Alignment, - layoutShallowEquals, - type Layout, -} from "./layout-types"; -import { - type SpotlightTileViewModel, - type GridTileViewModel, -} from "./TileViewModel"; -import { BehaviorSubject } from "rxjs"; - -const spotlightTile = {} as unknown as SpotlightTileViewModel; -const gridTile = {} as unknown as GridTileViewModel; -const pipAlignment$ = new BehaviorSubject({ - inline: "end", - block: "end", -}); - -const spotlightExpanded: Layout = { - type: "spotlight-expanded", - spotlight: spotlightTile, - pipAlignment$, -}; - -const spotlightPortrait: Layout = { - type: "spotlight-portrait", - spotlight: spotlightTile, - grid: [gridTile], - setVisibleTiles: () => {}, -}; - -test("layoutShallowEquals considers a layout to be equal to its shallow clone", () => - expect(layoutShallowEquals(spotlightExpanded, { ...spotlightExpanded })).toBe( - true, - )); - -test("layoutShallowEquals detects a missing key", () => { - expect( - layoutShallowEquals(spotlightExpanded, { - ...spotlightExpanded, - pip: gridTile, - }), - ).toBe(false); - expect( - layoutShallowEquals( - { ...spotlightExpanded, pip: gridTile }, - spotlightExpanded, - ), - ).toBe(false); -}); - -test("layoutShallowEquals considers grid arrays with equal contents to be equal", () => - expect( - layoutShallowEquals(spotlightPortrait, { - ...spotlightPortrait, - grid: [...spotlightPortrait.grid], - }), - ).toBe(true)); - -test("layoutShallowEquals detects grid arrays with different contents", () => - expect( - layoutShallowEquals(spotlightPortrait, { - ...spotlightPortrait, - grid: [...spotlightPortrait.grid, gridTile], - }), - ).toBe(false)); diff --git a/src/state/layout-types.ts b/src/state/layout-types.ts index fd9b36a69..3d80b361e 100644 --- a/src/state/layout-types.ts +++ b/src/state/layout-types.ts @@ -16,7 +16,6 @@ import { type SpotlightTileViewModel, } from "./TileViewModel.ts"; import { type Behavior } from "./Behavior.ts"; -import { shallowEquals as arrayShallowEquals } from "../utils/array.ts"; /** * The general shape of the space the call is drawn in. Called a window because @@ -148,31 +147,3 @@ export type Layout = | OneOnOneDesktopLayout | OneOnOneMobileLayout | PipLayout; - -/** - * Tests whether the top-level properties and array elements of `a` are equal - * to those of `b`. Useful for deduping redundant layout and layout media - * updates. - */ -export function layoutShallowEquals(a: T, b: T): boolean { - // If a and b have the same number of keys and every key in a is also in b, - // then they have the same keys. - const aKeys = Object.keys(a); - const bKeys = Object.keys(b); - if (aKeys.length !== bKeys.length) return false; - - for (const key of aKeys) { - if (!(key in b)) return false; - - // Now check that they have the same values. - const aValue = (a as any)[key]; - const bValue = (b as any)[key]; - if (Array.isArray(aValue) && Array.isArray(bValue)) { - // Special case for arrays so we can detect when the grid tiles arrays are - // essentially the same. - if (!arrayShallowEquals(aValue, bValue)) return false; - } else if (aValue !== bValue) return false; - } - - return true; -} diff --git a/src/utils/object.test.ts b/src/utils/object.test.ts new file mode 100644 index 000000000..6b55ac7f3 --- /dev/null +++ b/src/utils/object.test.ts @@ -0,0 +1,40 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { test, expect } from "vitest"; +import { shallowEquals } from "./object"; + +interface Thing { + type: string; + items: object[]; + value: number; + extra?: object; +} + +const item = {}; +const withArray: Thing = { type: "a", items: [item], value: 1 }; + +test("shallowEquals considers an object to be equal to its shallow clone", () => + expect(shallowEquals(withArray, { ...withArray })).toBe(true)); + +test("shallowEquals detects a missing key", () => { + expect(shallowEquals(withArray, { ...withArray, extra: item })).toBe(false); + expect(shallowEquals({ ...withArray, extra: item }, withArray)).toBe(false); +}); + +test("shallowEquals detects a differing value", () => + expect(shallowEquals(withArray, { ...withArray, value: 2 })).toBe(false)); + +test("shallowEquals considers arrays with equal contents to be equal", () => + expect( + shallowEquals(withArray, { ...withArray, items: [...withArray.items] }), + ).toBe(true)); + +test("shallowEquals detects arrays with different contents", () => + expect( + shallowEquals(withArray, { ...withArray, items: [...withArray.items, {}] }), + ).toBe(false)); diff --git a/src/utils/object.ts b/src/utils/object.ts new file mode 100644 index 000000000..7fc7b5b1d --- /dev/null +++ b/src/utils/object.ts @@ -0,0 +1,38 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { shallowEquals as arrayShallowEquals } from "./array.ts"; + +/** + * Determine whether two objects are equal by shallow comparison of their + * top-level properties. Array-valued properties are themselves compared + * shallowly, so two objects holding distinct arrays with the same elements are + * considered equal. + */ +export function shallowEquals( + a: Readonly, + b: Readonly, +): boolean { + // If a and b have the same number of keys and every key in a is also in b, + // then they have the same keys. + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (aKeys.length !== bKeys.length) return false; + + for (const key of aKeys) { + if (!(key in b)) return false; + + // Now check that they have the same values. + const aValue = (a as any)[key]; + const bValue = (b as any)[key]; + if (Array.isArray(aValue) && Array.isArray(bValue)) { + if (!arrayShallowEquals(aValue, bValue)) return false; + } else if (aValue !== bValue) return false; + } + + return true; +} From 4fd2f669490c94dbd604fbb6a8bab7cbadb9fdaa Mon Sep 17 00:00:00 2001 From: Johannes Marbach Date: Thu, 8 Oct 2026 11:14:26 +0200 Subject: [PATCH 8/8] Explain why we use merge --- src/state/CallViewModel/CallViewModel.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 083666001..90c4cff3b 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -1164,6 +1164,16 @@ export function createCallViewModel$( * The media to be used to produce a layout. */ const layoutMedia$ = scope.behavior( + // We deliberately don't use combineLatest here. Several of the inputs + // below derive from the same source. So one upstream change cascades + // through them one at a time. combineLatest would see each step of the + // cascade and emit layouts built from inputs that never coexisted. + // Instead, we treat the emissions as a mere "something changed" signal + // and read every input's current value. The derived behaviors were + // subscribed before this one and a BehaviorSubject updates its value + // before notifying. So by the time the first signal arrives every value + // is already consistent. The duplicate signals from the rest of the + // cascade are dropped by distinctUntilChanged. merge( windowMode$, layoutSwitchVm.layout$,