From b5d3f3c72a2d31d2bded511bdcffb0adb64901bc Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Dec 2025 11:18:50 +0100 Subject: [PATCH] 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", );