From 2f55d8e30c139fbd57f330a112913c274e6547e8 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Thu, 24 Jul 2025 17:46:26 +0200 Subject: [PATCH] UrlParams: Intent system update, split into configuration and propreties (#3376) * refactor UrlParams to use a preset intent system * change defaults for intend headers * add: getEnumParam to ParamParser * remove deprecated url params * only allow skip lobby in widget (more strict needs test adjustment) * fix tests that now require the url to be a widget url Co-authored-by: Robin --------- Co-authored-by: Robin --- src/UrlParams.test.ts | 68 +++++-- src/UrlParams.ts | 337 +++++++++++++++++++++-------------- src/room/MuteStates.test.tsx | 6 +- 3 files changed, 267 insertions(+), 144 deletions(-) diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index 495f7926..fbf0c095 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -10,7 +10,7 @@ import { describe, expect, it } from "vitest"; import { getRoomIdentifierFromUrl, getUrlParams, - UserIntent, + HeaderStyle, } from "../src/UrlParams"; const ROOM_NAME = "roomNameHere"; @@ -211,24 +211,68 @@ describe("UrlParams", () => { }); describe("intent", () => { - it("defaults to unknown", () => { - expect(getUrlParams().intent).toBe(UserIntent.Unknown); + const noIntentDefaults = { + confineToRoom: false, + appPrompt: true, + preload: false, + header: HeaderStyle.Standard, + showControls: true, + hideScreensharing: false, + allowIceFallback: false, + perParticipantE2EE: false, + controlledAudioDevices: false, + skipLobby: false, + returnToLobby: false, + sendNotificationType: undefined, + }; + const startNewCallDefaults = (platform: string): object => ({ + confineToRoom: true, + appPrompt: false, + preload: true, + header: platform === "desktop" ? HeaderStyle.None : HeaderStyle.AppBar, + showControls: true, + hideScreensharing: false, + allowIceFallback: true, + perParticipantE2EE: true, + controlledAudioDevices: platform === "desktop" ? false : true, + skipLobby: true, + returnToLobby: false, + sendNotificationType: "notification", + }); + const joinExistingCallDefaults = (platform: string): object => ({ + confineToRoom: true, + appPrompt: false, + preload: true, + header: platform === "desktop" ? HeaderStyle.None : HeaderStyle.AppBar, + showControls: true, + hideScreensharing: false, + allowIceFallback: true, + perParticipantE2EE: true, + controlledAudioDevices: platform === "desktop" ? false : true, + skipLobby: false, + returnToLobby: false, + sendNotificationType: "notification", + }); + it("use no-intent-defaults with unknown intent", () => { + expect(getUrlParams()).toMatchObject(noIntentDefaults); }); it("ignores intent if it is not a valid value", () => { - expect(getUrlParams("?intent=foo").intent).toBe(UserIntent.Unknown); + expect(getUrlParams("?intent=foo")).toMatchObject(noIntentDefaults); }); it("accepts start_call", () => { - expect(getUrlParams("?intent=start_call").intent).toBe( - UserIntent.StartNewCall, - ); + expect( + getUrlParams("?intent=start_call&widgetId=1234&parentUrl=parent.org"), + ).toMatchObject(startNewCallDefaults("desktop")); }); it("accepts join_existing", () => { - expect(getUrlParams("?intent=join_existing").intent).toBe( - UserIntent.JoinExistingCall, - ); + expect( + getUrlParams( + "?intent=join_existing&widgetId=1234&parentUrl=parent.org", + ), + ).toMatchObject(joinExistingCallDefaults("desktop")); }); }); @@ -260,9 +304,5 @@ describe("UrlParams", () => { ); expect(getUrlParams("?header=none&hideHeader=false").header).toBe("none"); }); - it("converts hideHeader to the correct header value", () => { - expect(getUrlParams("?hideHeader=true").header).toBe("none"); - expect(getUrlParams("?hideHeader=false").header).toBe("standard"); - }); }); }); diff --git a/src/UrlParams.ts b/src/UrlParams.ts index b3940acb..65e3d901 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -9,10 +9,12 @@ import { useMemo } from "react"; import { useLocation } from "react-router-dom"; import { logger } from "matrix-js-sdk/lib/logger"; import { type RTCNotificationType } from "matrix-js-sdk/lib/matrixrtc"; +import { pickBy } from "lodash-es"; import { Config } from "./config/Config"; import { type EncryptionSystem } from "./e2ee/sharedKeyManagement"; import { E2eeType } from "./e2ee/e2eeType"; +import { platform } from "./Platform"; interface RoomIdentifier { roomAlias: string | null; @@ -21,6 +23,7 @@ interface RoomIdentifier { } export enum UserIntent { + // TODO: add DM vs room call StartNewCall = "start_call", JoinExistingCall = "join_existing", Unknown = "unknown", @@ -32,12 +35,12 @@ export enum HeaderStyle { AppBar = "app_bar", } -// If you need to add a new flag to this interface, prefer a name that describes -// a specific behavior (such as 'confineToRoom'), rather than one that describes -// the situations that call for this behavior ('isEmbedded'). This makes it -// clearer what each flag means, and helps us avoid coupling Element Call's -// behavior to the needs of specific consumers. -export interface UrlParams { +/** + * The UrlProperties are used to pass required data to the widget. + * Those are different in different rooms, users, devices. They do not configure the behavior of the + * widget but provide the required data to the widget. + */ +export interface UrlProperties { // Widget api related params widgetId: string | null; parentUrl: string | null; @@ -49,45 +52,11 @@ export interface UrlParams { * is also not validated, where it is in useRoomIdentifier(). */ roomId: string | null; - /** - * Whether the app should keep the user confined to the current call/room. - */ - confineToRoom: boolean; - /** - * Whether upon entering a room, the user should be prompted to launch the - * native mobile app. (Affects only Android and iOS.) - * - * The app prompt must also be enabled in the config for this to take effect. - */ - appPrompt: boolean; - /** - * Whether the app should pause before joining the call until it sees an - * io.element.join widget action, allowing it to be preloaded. - */ - preload: boolean; - /** - * The style of headers to show. "standard" is the default arrangement, "none" - * hides the header entirely, and "app_bar" produces a header with a back - * button like you might see in mobile apps. The callback for the back button - * is window.controls.onBackButtonPressed. - */ - header: HeaderStyle; - /** - * Whether the controls should be shown. For screen recording no controls can be desired. - */ - showControls: boolean; - /** - * Whether to hide the screen-sharing button. - */ - hideScreensharing: boolean; - /** - * Whether to use end-to-end encryption. - */ - e2eEnabled: boolean; /** * The user's ID (only used in matryoshka mode). */ userId: string | null; + /** * The display name to use for auto-registration. */ @@ -125,14 +94,96 @@ export interface UrlParams { */ posthogApiKey: string | null; /** - * Whether the app is allowed to use fallback STUN servers for ICE in case the - * user's homeserver doesn't provide any. + * Whether to use end-to-end encryption. */ - allowIceFallback: boolean; + e2eEnabled: boolean; /** * E2EE password */ password: string | null; + /** This defines the homeserver that is going to be used when joining a room. + * It has to be set to a non default value for links to rooms + * that are not on the default homeserver, + * that is in use for the current user. + */ + viaServers: string | null; + + /** + * This defines the homeserver that is going to be used when registering + * a new (guest) user. + * This can be user to configure a non default guest user server when + * creating a spa link. + */ + homeserver: string | null; + + /** + * The rageshake submit URL. This is only used in the embedded package of Element Call. + */ + rageshakeSubmitUrl: string | null; + + /** + * The Sentry DSN. This is only used in the embedded package of Element Call. + */ + sentryDsn: string | null; + + /** + * The Sentry environment. This is only used in the embedded package of Element Call. + */ + sentryEnvironment: string | null; + /** + * The theme to use for element call. + * can be "light", "dark", "light-high-contrast" or "dark-high-contrast". + */ + theme: string | null; +} + +/** + * The configuration for the app, which can be set via URL parameters. + * Those property are different to the UrlProperties, since they are all optional + * and configure the behavior of the app. Their value is the same if EC is used in + * the same context but with different accounts/users. + * + * Their defaults can be controlled by the `intent` property. + */ +export interface UrlConfiguration { + /** + * Whether the app should keep the user confined to the current call/room. + */ + confineToRoom: boolean; + /** + * Whether upon entering a room, the user should be prompted to launch the + * native mobile app. (Affects only Android and iOS.) + * + * The app prompt must also be enabled in the config for this to take effect. + */ + appPrompt: boolean; + /** + * Whether the app should pause before joining the call until it sees an + * io.element.join widget action, allowing it to be preloaded. + */ + preload: boolean; + /** + * The style of headers to show. "standard" is the default arrangement, "none" + * hides the header entirely, and "app_bar" produces a header with a back + * button like you might see in mobile apps. The callback for the back button + * is window.controls.onBackButtonPressed. + */ + header: HeaderStyle; + /** + * Whether the controls should be shown. For screen recording no controls can be desired. + */ + showControls: boolean; + /** + * Whether to hide the screen-sharing button. + */ + hideScreensharing: boolean; + + /** + * Whether the app is allowed to use fallback STUN servers for ICE in case the + * user's homeserver doesn't provide any. + */ + allowIceFallback: boolean; + /** * Whether the app should use per participant keys for E2EE. */ @@ -154,52 +205,19 @@ export interface UrlParams { * This is useful for video rooms. */ returnToLobby: boolean; - /** - * The theme to use for element call. - * can be "light", "dark", "light-high-contrast" or "dark-high-contrast". - */ - theme: string | null; - /** This defines the homeserver that is going to be used when joining a room. - * It has to be set to a non default value for links to rooms - * that are not on the default homeserver, - * that is in use for the current user. - */ - viaServers: string | null; - /** - * This defines the homeserver that is going to be used when registering - * a new (guest) user. - * This can be user to configure a non default guest user server when - * creating a spa link. - */ - homeserver: string | null; - - /** - * The user's intent with respect to the call. - * e.g. if they clicked a Start Call button, this would be `start_call`. - * If it was a Join Call button, it would be `join_existing`. - */ - intent: string | null; - - /** - * The rageshake submit URL. This is only used in the embedded package of Element Call. - */ - rageshakeSubmitUrl: string | null; - - /** - * The Sentry DSN. This is only used in the embedded package of Element Call. - */ - sentryDsn: string | null; - - /** - * The Sentry environment. This is only used in the embedded package of Element Call. - */ - sentryEnvironment: string | null; /** * Whether and what type of notification EC should send, when the user joins the call. */ sendNotificationType?: RTCNotificationType; } +// If you need to add a new flag to this interface, prefer a name that describes +// a specific behavior (such as 'confineToRoom'), rather than one that describes +// the situations that call for this behavior ('isEmbedded'). This makes it +// clearer what each flag means, and helps us avoid coupling Element Call's +// behavior to the needs of specific consumers. +export interface UrlParams extends UrlProperties, UrlConfiguration {} + // This is here as a stopgap, but what would be far nicer is a function that // takes a UrlParams and returns a query string. That would enable us to // consolidate all the data about URL parameters and their meanings to this one @@ -240,6 +258,17 @@ class ParamParser { return this.fragmentParams.get(name) ?? this.queryParams.get(name); } + public getEnumParam( + name: string, + type: { [s: string]: T } | ArrayLike, + ): T | undefined { + const value = this.getParam(name); + if (value !== null && Object.values(type).includes(value as T)) { + return value as T; + } + return undefined; + } + public getAllParams(name: string): string[] { return [ ...this.fragmentParams.getAll(name), @@ -251,6 +280,10 @@ class ParamParser { const param = this.getParam(name); return param === null ? defaultValue : param !== "false"; } + public getFlag(name: string): boolean | undefined { + const param = this.getParam(name); + return param !== null ? param !== "false" : undefined; + } } /** @@ -267,46 +300,79 @@ export const getUrlParams = ( const fontScale = parseFloat(parser.getParam("fontScale") ?? ""); - let intent = parser.getParam("intent"); - if (!intent || !Object.values(UserIntent).includes(intent as UserIntent)) { - intent = UserIntent.Unknown; - } - - // Check hideHeader for backwards compatibility. If header is set, hideHeader - // is ignored. - const header = - parser.getParam("header") ?? - (parser.getFlagParam("hideHeader") - ? HeaderStyle.None - : HeaderStyle.Standard); - - const sendNotificationType = ["ring", "notification"].includes( - parser.getParam("sendNotificationType") ?? "", - ) - ? (parser.getParam("sendNotificationType") as RTCNotificationType) - : undefined; const widgetId = parser.getParam("widgetId"); const parentUrl = parser.getParam("parentUrl"); const isWidget = !!widgetId && !!parentUrl; - return { + /** + * The user's intent with respect to the call. + * e.g. if they clicked a Start Call button, this would be `start_call`. + * If it was a Join Call button, it would be `join_existing`. + * This is a platform specific default set of parameters, that allows to minize the configuration + * needed to start a call. And empowers the EC codebase to control the platform/intent behavior in + * a central place. + * + * In short: either provide url query parameters of UrlConfiguration or set the intent + * (or the global defaults will be used). + */ + const intent = !isWidget + ? UserIntent.Unknown + : (parser.getEnumParam("intent", UserIntent) ?? UserIntent.Unknown); + // Here we only use constants and `platform` to determine the intent preset. + let intentPreset: UrlConfiguration; + const inAppDefault = { + confineToRoom: true, + appPrompt: false, + preload: true, + header: platform === "desktop" ? HeaderStyle.None : HeaderStyle.AppBar, + showControls: true, + hideScreensharing: false, + allowIceFallback: true, + perParticipantE2EE: true, + controlledAudioDevices: platform === "desktop" ? false : true, + skipLobby: true, + returnToLobby: false, + sendNotificationType: "notification" as RTCNotificationType, + }; + switch (intent) { + case UserIntent.StartNewCall: + intentPreset = { + ...inAppDefault, + skipLobby: true, + }; + break; + case UserIntent.JoinExistingCall: + intentPreset = { + ...inAppDefault, + skipLobby: false, + }; + break; + // Non widget usecase defaults + default: + intentPreset = { + confineToRoom: false, + appPrompt: true, + preload: false, + header: HeaderStyle.Standard, + showControls: true, + hideScreensharing: false, + allowIceFallback: false, + perParticipantE2EE: false, + controlledAudioDevices: false, + skipLobby: false, + returnToLobby: false, + sendNotificationType: undefined, + }; + } + + const properties: UrlProperties = { widgetId, parentUrl, - // NB. we don't validate roomId here as we do in getRoomIdentifierFromUrl: // what would we do if it were invalid? If the widget API says that's what // the room ID is, then that's what it is. roomId: parser.getParam("roomId"), password: parser.getParam("password"), - // This flag has 'embed' as an alias for historical reasons - confineToRoom: - parser.getFlagParam("confineToRoom") || parser.getFlagParam("embed"), - appPrompt: parser.getFlagParam("appPrompt", true), - preload: isWidget ? parser.getFlagParam("preload") : false, - header: header as HeaderStyle, - showControls: parser.getFlagParam("showControls", true), - hideScreensharing: parser.getFlagParam("hideScreensharing"), - e2eEnabled: parser.getFlagParam("enableE2EE", true), userId: isWidget ? parser.getParam("userId") : null, displayName: parser.getParam("displayName"), deviceId: isWidget ? parser.getParam("deviceId") : null, @@ -314,24 +380,9 @@ export const getUrlParams = ( lang: parser.getParam("lang"), fonts: parser.getAllParams("font"), fontScale: Number.isNaN(fontScale) ? null : fontScale, - allowIceFallback: parser.getFlagParam("allowIceFallback"), - perParticipantE2EE: parser.getFlagParam("perParticipantE2EE"), - controlledAudioDevices: parser.getFlagParam( - "controlledAudioDevices", - // the deprecated property name - parser.getFlagParam("controlledMediaDevices"), - ), - skipLobby: parser.getFlagParam( - "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, homeserver: !isWidget ? parser.getParam("homeserver") : null, - intent, posthogApiHost: parser.getParam("posthogApiHost"), posthogApiKey: parser.getParam("posthogApiKey"), posthogUserId: @@ -339,7 +390,35 @@ export const getUrlParams = ( rageshakeSubmitUrl: parser.getParam("rageshakeSubmitUrl"), sentryDsn: parser.getParam("sentryDsn"), sentryEnvironment: parser.getParam("sentryEnvironment"), - sendNotificationType, + e2eEnabled: parser.getFlagParam("enableE2EE", true), + }; + + const configuration: Partial = { + confineToRoom: parser.getFlag("confineToRoom"), + appPrompt: parser.getFlag("appPrompt"), + preload: isWidget ? parser.getFlag("preload") : undefined, + // Check hideHeader for backwards compatibility. If header is set, hideHeader + // is ignored. + header: parser.getEnumParam("header", HeaderStyle), + showControls: parser.getFlag("showControls"), + hideScreensharing: parser.getFlag("hideScreensharing"), + allowIceFallback: parser.getFlag("allowIceFallback"), + perParticipantE2EE: parser.getFlag("perParticipantE2EE"), + controlledAudioDevices: parser.getFlag("controlledAudioDevices"), + skipLobby: isWidget ? parser.getFlag("skipLobby") : false, + // 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.getFlag("returnToLobby") : false, + sendNotificationType: parser.getEnumParam("sendNotificationType", [ + "ring", + "notification", + ]), + }; + + return { + ...properties, + ...intentPreset, + ...pickBy(configuration, (v) => v !== undefined), }; }; diff --git a/src/room/MuteStates.test.tsx b/src/room/MuteStates.test.tsx index 13dc8ee0..d349a5c6 100644 --- a/src/room/MuteStates.test.tsx +++ b/src/room/MuteStates.test.tsx @@ -191,7 +191,11 @@ describe("useMuteStates", () => { mockConfig(); render( - +