From b5f5edba0925b70c0466c5417bd77f6499ce2179 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 4 Mar 2025 15:06:47 -0500 Subject: [PATCH 1/3] Fix the control flow of GroupCallView render function 2bb5b020e60c3d8b6034be799db42fa3d3164cc4 refactored the end of the GroupCallView render function to not use any early returns, and clumsily failed to account for the fall-through case that makes returnToLobby work (as opposed to sitting on a blank screen). --- src/room/GroupCallView.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 95d1d12c..66f14821 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -495,9 +495,7 @@ export const GroupCallView: FC = ({ } } else if (left && widget !== null) { // Left in widget mode: - if (!returnToLobby) { - body = null; - } + body = returnToLobby ? lobbyView : null; } else if (preload || skipLobby) { body = null; } else { From 28c45c61072126023381446877fb99c4c36fd8ac Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 4 Mar 2025 15:09:59 -0500 Subject: [PATCH 2/3] Avoid closing the widget in returnToLobby mode If returnToLobby is enabled then we obviously want to keep the widget open once the user leaves the call. --- src/UrlParams.test.ts | 4 +-- src/UrlParams.ts | 2 +- src/rtcSessionHelpers.test.ts | 57 +++++++++++++++++++++++------------ src/rtcSessionHelpers.ts | 3 +- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index 8e185abc..dce46754 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -110,8 +110,8 @@ describe("UrlParams", () => { }); describe("returnToLobby", () => { - it("is true in SPA mode", () => { - expect(getUrlParams("?returnToLobby=false").returnToLobby).toBe(true); + it("is false in SPA mode", () => { + expect(getUrlParams("?returnToLobby=true").returnToLobby).toBe(false); }); it("defaults to false in widget mode", () => { diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 61b777c7..23b7ecbd 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -264,7 +264,7 @@ export const getUrlParams = ( "skipLobby", isWidget && intent === UserIntent.StartNewCall, ), - returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : true, + returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : false, theme: parser.getParam("theme"), viaServers: !isWidget ? parser.getParam("viaServers") : null, homeserver: !isWidget ? parser.getParam("homeserver") : null, diff --git a/src/rtcSessionHelpers.test.ts b/src/rtcSessionHelpers.test.ts index 21ee2cd3..972d6e75 100644 --- a/src/rtcSessionHelpers.test.ts +++ b/src/rtcSessionHelpers.test.ts @@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details. */ import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; -import { expect, test, vi } from "vitest"; +import { expect, onTestFinished, test, vi } from "vitest"; import { AutoDiscovery } from "matrix-js-sdk/src/autodiscovery"; import EventEmitter from "events"; @@ -15,11 +15,17 @@ import { mockConfig } from "./utils/test"; import { ElementWidgetActions, widget } from "./widget"; import { ErrorCode } from "./utils/errors.ts"; +const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); +vi.mock("./UrlParams", () => ({ getUrlParams })); + const actualWidget = await vi.hoisted(async () => vi.importActual("./widget")); vi.mock("./widget", () => ({ ...actualWidget, widget: { - api: { transport: { send: vi.fn(), reply: vi.fn(), stop: vi.fn() } }, + api: { + setAlwaysOnScreen: (): void => {}, + transport: { send: vi.fn(), reply: vi.fn(), stop: vi.fn() }, + }, lazyActions: new EventEmitter(), }, })); @@ -109,34 +115,45 @@ test("It joins the correct Session", async () => { ); }); -test("leaveRTCSession closes the widget on a normal hangup", async () => { +async function testLeaveRTCSession( + cause: "user" | "error", + expectClose: boolean, +): Promise { vi.clearAllMocks(); const session = { leaveRoomSession: vi.fn() } as unknown as MatrixRTCSession; - await leaveRTCSession(session, "user"); + await leaveRTCSession(session, cause); expect(session.leaveRoomSession).toHaveBeenCalled(); expect(widget!.api.transport.send).toHaveBeenCalledWith( ElementWidgetActions.HangupCall, expect.anything(), ); - expect(widget!.api.transport.send).toHaveBeenCalledWith( - ElementWidgetActions.Close, - expect.anything(), - ); + if (expectClose) { + expect(widget!.api.transport.send).toHaveBeenCalledWith( + ElementWidgetActions.Close, + expect.anything(), + ); + expect(widget!.api.transport.stop).toHaveBeenCalled(); + } else { + expect(widget!.api.transport.send).not.toHaveBeenCalledWith( + ElementWidgetActions.Close, + expect.anything(), + ); + expect(widget!.api.transport.stop).not.toHaveBeenCalled(); + } +} + +test("leaveRTCSession closes the widget on a normal hangup", async () => { + await testLeaveRTCSession("user", true); }); test("leaveRTCSession doesn't close the widget on a fatal error", async () => { - vi.clearAllMocks(); - const session = { leaveRoomSession: vi.fn() } as unknown as MatrixRTCSession; - await leaveRTCSession(session, "error"); - expect(session.leaveRoomSession).toHaveBeenCalled(); - expect(widget!.api.transport.send).toHaveBeenCalledWith( - ElementWidgetActions.HangupCall, - expect.anything(), - ); - expect(widget!.api.transport.send).not.toHaveBeenCalledWith( - ElementWidgetActions.Close, - expect.anything(), - ); + await testLeaveRTCSession("error", false); +}); + +test("leaveRTCSession doesn't close the widget when returning to lobby", async () => { + getUrlParams.mockReturnValue({ returnToLobby: true }); + onTestFinished(() => void getUrlParams.mockReset()); + await testLeaveRTCSession("user", false); }); test("It fails with configuration error if no live kit url config is set in fallback", async () => { diff --git a/src/rtcSessionHelpers.ts b/src/rtcSessionHelpers.ts index 719af998..943eaf82 100644 --- a/src/rtcSessionHelpers.ts +++ b/src/rtcSessionHelpers.ts @@ -19,6 +19,7 @@ import { PosthogAnalytics } from "./analytics/PosthogAnalytics"; import { Config } from "./config/Config"; import { ElementWidgetActions, widget, type WidgetHelpers } from "./widget"; import { MatrixRTCFocusMissingError } from "./utils/errors.ts"; +import { getUrlParams } from "./UrlParams.ts"; const FOCI_WK_KEY = "org.matrix.msc4143.rtc_foci"; @@ -149,7 +150,7 @@ const widgetPostHangupProcedure = async ( } // On a normal user hangup we can shut down and close the widget. But if an // error occurs we should keep the widget open until the user reads it. - if (cause === "user") { + if (cause === "user" && !getUrlParams().returnToLobby) { try { await widget.api.transport.send(ElementWidgetActions.Close, {}); } catch (e) { From 359812d8b1b9ab20fdc69f2220583d2f6c0b7d25 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 10:40:37 -0500 Subject: [PATCH 3/3] Explain why returnToLobby is false in SPA --- src/UrlParams.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 23b7ecbd..fda4a95f 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -264,6 +264,8 @@ export const getUrlParams = ( "skipLobby", isWidget && intent === UserIntent.StartNewCall, ), + // In SPA mode the user should always exit to the home screen when hanging + // up, rather than being sent back to the lobby returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : false, theme: parser.getParam("theme"), viaServers: !isWidget ? parser.getParam("viaServers") : null,