From 19d0f84f028da5afe15aa0285d13b28e54acb8d7 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 18 Dec 2024 15:30:33 +0000 Subject: [PATCH 1/3] Introduce intent URL param and make it change the default lobby behaviour (#2828) * Introduce `intent` URL param and make it change the default lobby behaviour * Mark skipLobby as deprecated * Add support for unknown intent which is default for when not specified --- docs/url-params.md | 57 ++++++++++++++++++++++--------------------- src/UrlParams.test.ts | 50 ++++++++++++++++++++++++++++++++++++- src/UrlParams.ts | 23 ++++++++++++++++- 3 files changed, 100 insertions(+), 30 deletions(-) diff --git a/docs/url-params.md b/docs/url-params.md index 010a4ec8..740e0218 100644 --- a/docs/url-params.md +++ b/docs/url-params.md @@ -32,31 +32,32 @@ There are two formats for Element Call urls. ## Parameters -| Name | Values | Required for widget | Required for SPA | Description | -| ------------------------- | ---------------------------------------------------------------------------------------------------- | ----------------------- | ----------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `allowIceFallback` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Allows use of fallback STUN servers for ICE if the user's homeserver doesn’t provide any. | -| `analyticsID` | Posthog analytics ID | No | No | Available only with user's consent for sharing telemetry in Element Web. | -| `appPrompt` | `true` or `false` | No, defaults to `true` | No, defaults to `true` | Prompts the user to launch the native mobile app upon entering a room, applicable only on Android and iOS, and must be enabled in config. | -| `baseUrl` | | Yes | Not applicable | The base URL of the homeserver to use for media lookups. | -| `confineToRoom` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Keeps the user confined to the current call/room. | -| `deviceId` | Matrix device ID | Yes | Not applicable | The Matrix device ID for the widget host. | -| `displayName` | | No | No | Display name used for auto-registration. | -| `enableE2EE` (deprecated) | `true` or `false` | No, defaults to `true` | No, defaults to `true` | Legacy flag to enable end-to-end encryption, not used in the `livekit` branch. | -| `fontScale` | A decimal number such as `0.9` | No, defaults to `1.0` | No, defaults to `1.0` | Factor by which to scale the interface's font size. | -| `fonts` | | No | No | Defines the font(s) used by the interface. Multiple font parameters can be specified: `?font=font-one&font=font-two...`. | -| `hideHeader` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Hides the room header when in a call. | -| `hideScreensharing` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Hides the screen-sharing button. | -| `homeserver` | | Not applicable | No | Homeserver for registering a new (guest) user, configures non-default guest user server when creating a spa link. | -| `lang` | [BCP 47](https://www.rfc-editor.org/info/bcp47) code | No | No | The language the app should use. | -| `parentUrl` | | Yes | Not applicable | The url used to send widget action postMessages. This should be the domain of the client or the webview the widget is hosted in. (in case the widget is not in an Iframe but in a dedicated webview we send the postMessages same WebView the widget lives in. Filtering is done in the widget so it ignores the messages it receives from itself) | -| `password` | | No | No | E2EE password when using a shared secret. (For individual sender keys in embedded mode this is not required.) | -| `perParticipantE2EE` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Enables per participant encryption with Keys exchanged over encrypted matrix room messages. | -| `preload` | `true` or `false` | No, defaults to `false` | Not applicable | Pauses app before joining a call until an `io.element.join` widget action is seen, allowing preloading. | -| `returnToLobby` | `true` or `false` | No, defaults to `false` | Not applicable | Displays the lobby in widget mode after leaving a call; shows a blank page if set to `false`. Useful for video rooms. | -| `roomId` | [Matrix Room ID](https://spec.matrix.org/v1.12/appendices/#room-ids) | Yes | No | Anything about what room we're pointed to should be from useRoomIdentifier which parses the path and resolves alias with respect to the default server name, however roomId is an exception as we need the room ID in embedded widget mode, and not the room alias (or even the via params because we are not trying to join it). This is also not validated, where it is in `useRoomIdentifier()`. | -| `showControls` | `true` or `false` | No, defaults to `true` | No, defaults to `true` | Displays controls like mute, screen-share, invite, and hangup buttons during a call. | -| `skipLobby` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Skips the lobby to join a call directly, can be combined with preload in widget. When `true` the audio and video inputs will be muted by default. (This means there currently is no way to start without muted video if one wants to skip the lobby. Also not in widget mode.) | -| `theme` | One of: `light`, `dark`, `light-high-contrast`, `dark-high-contrast` | No, defaults to `dark` | No, defaults to `dark` | UI theme to use. | -| `userId` | [Matrix User Identifier](https://spec.matrix.org/v1.12/appendices/#user-identifiers) | Yes | Not applicable | The Matrix user ID. | -| `viaServers` | Comma separated list of [Matrix Server Names](https://spec.matrix.org/v1.12/appendices/#server-name) | Not applicable | No | Homeserver for joining a room, non-empty value required for rooms not on the user’s default homeserver. | -| `widgetId` | [MSC2774](https://github.com/matrix-org/matrix-spec-proposals/pull/2774) format widget ID | Yes | Not applicable | The id used by the widget. The presence of this parameter implies that element call will not connect to a homeserver directly and instead tries to establish postMessage communication via the `parentUrl`. | +| Name | Values | Required for widget | Required for SPA | Description | +| ---------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `allowIceFallback` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Allows use of fallback STUN servers for ICE if the user's homeserver doesn’t provide any. | +| `analyticsID` | Posthog analytics ID | No | No | Available only with user's consent for sharing telemetry in Element Web. | +| `appPrompt` | `true` or `false` | No, defaults to `true` | No, defaults to `true` | Prompts the user to launch the native mobile app upon entering a room, applicable only on Android and iOS, and must be enabled in config. | +| `baseUrl` | | Yes | Not applicable | The base URL of the homeserver to use for media lookups. | +| `confineToRoom` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Keeps the user confined to the current call/room. | +| `deviceId` | Matrix device ID | Yes | Not applicable | The Matrix device ID for the widget host. | +| `displayName` | | No | No | Display name used for auto-registration. | +| `enableE2EE` (deprecated) | `true` or `false` | No, defaults to `true` | No, defaults to `true` | Legacy flag to enable end-to-end encryption, not used in the `livekit` branch. | +| `fontScale` | A decimal number such as `0.9` | No, defaults to `1.0` | No, defaults to `1.0` | Factor by which to scale the interface's font size. | +| `fonts` | | No | No | Defines the font(s) used by the interface. Multiple font parameters can be specified: `?font=font-one&font=font-two...`. | +| `hideHeader` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Hides the room header when in a call. | +| `hideScreensharing` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Hides the screen-sharing button. | +| `homeserver` | | Not applicable | No | Homeserver for registering a new (guest) user, configures non-default guest user server when creating a spa link. | +| `intent` | `start_call` or `join_existing` | No, defaults to `start_call` | No, defaults to `start_call` | The intent of the user 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`. | +| `lang` | [BCP 47](https://www.rfc-editor.org/info/bcp47) code | No | No | The language the app should use. | +| `parentUrl` | | Yes | Not applicable | The url used to send widget action postMessages. This should be the domain of the client or the webview the widget is hosted in. (in case the widget is not in an Iframe but in a dedicated webview we send the postMessages same WebView the widget lives in. Filtering is done in the widget so it ignores the messages it receives from itself) | +| `password` | | No | No | E2EE password when using a shared secret. (For individual sender keys in embedded mode this is not required.) | +| `perParticipantE2EE` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Enables per participant encryption with Keys exchanged over encrypted matrix room messages. | +| `preload` | `true` or `false` | No, defaults to `false` | Not applicable | Pauses app before joining a call until an `io.element.join` widget action is seen, allowing preloading. | +| `returnToLobby` | `true` or `false` | No, defaults to `false` | Not applicable | Displays the lobby in widget mode after leaving a call; shows a blank page if set to `false`. Useful for video rooms. | +| `roomId` | [Matrix Room ID](https://spec.matrix.org/v1.12/appendices/#room-ids) | Yes | No | Anything about what room we're pointed to should be from useRoomIdentifier which parses the path and resolves alias with respect to the default server name, however roomId is an exception as we need the room ID in embedded widget mode, and not the room alias (or even the via params because we are not trying to join it). This is also not validated, where it is in `useRoomIdentifier()`. | +| `showControls` | `true` or `false` | No, defaults to `true` | No, defaults to `true` | Displays controls like mute, screen-share, invite, and hangup buttons during a call. | +| `skipLobby` (deprecated: use `intent` instead) | `true` or `false` | No. If `intent` is explicitly `start_call` then defaults to `true`. Otherwise defaults to `false` | No, defaults to `false` | Skips the lobby to join a call directly, can be combined with preload in widget. When `true` the audio and video inputs will be muted by default. (This means there currently is no way to start without muted video if one wants to skip the lobby. Also not in widget mode.) | +| `theme` | One of: `light`, `dark`, `light-high-contrast`, `dark-high-contrast` | No, defaults to `dark` | No, defaults to `dark` | UI theme to use. | +| `userId` | [Matrix User Identifier](https://spec.matrix.org/v1.12/appendices/#user-identifiers) | Yes | Not applicable | The Matrix user ID. | +| `viaServers` | Comma separated list of [Matrix Server Names](https://spec.matrix.org/v1.12/appendices/#server-name) | Not applicable | No | Homeserver for joining a room, non-empty value required for rooms not on the user’s default homeserver. | +| `widgetId` | [MSC2774](https://github.com/matrix-org/matrix-spec-proposals/pull/2774) format widget ID | Yes | Not applicable | The id used by the widget. The presence of this parameter implies that element call will not connect to a homeserver directly and instead tries to establish postMessage communication via the `parentUrl`. | diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index 10f1386b..092b51d3 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -7,7 +7,11 @@ Please see LICENSE in the repository root for full details. import { describe, expect, it } from "vitest"; -import { getRoomIdentifierFromUrl, getUrlParams } from "../src/UrlParams"; +import { + getRoomIdentifierFromUrl, + getUrlParams, + UserIntent, +} from "../src/UrlParams"; const ROOM_NAME = "roomNameHere"; const ROOM_ID = "!d45f138fsd"; @@ -195,4 +199,48 @@ describe("UrlParams", () => { expect(getUrlParams("?homeserver=asd").homeserver).toBe("asd"); }); }); + + describe("intent", () => { + it("defaults to unknown", () => { + expect(getUrlParams().intent).toBe(UserIntent.Unknown); + }); + + it("ignores intent if it is not a valid value", () => { + expect(getUrlParams("?intent=foo").intent).toBe(UserIntent.Unknown); + }); + + it("accepts start_call", () => { + expect(getUrlParams("?intent=start_call").intent).toBe( + UserIntent.StartNewCall, + ); + }); + + it("accepts join_existing", () => { + expect(getUrlParams("?intent=join_existing").intent).toBe( + UserIntent.JoinExistingCall, + ); + }); + }); + + describe("skipLobby", () => { + it("defaults to false", () => { + expect(getUrlParams().skipLobby).toBe(false); + }); + + it("defaults to false if intent is start_call in SPA mode", () => { + expect(getUrlParams("?intent=start_call").skipLobby).toBe(false); + }); + + it("defaults to true if intent is start_call in widget mode", () => { + expect( + getUrlParams( + "?intent=start_call&widgetId=12345&parentUrl=https%3A%2F%2Flocalhost%2Ffoo", + ).skipLobby, + ).toBe(true); + }); + + it("default to false if intent is join_existing", () => { + expect(getUrlParams("?intent=join_existing").skipLobby).toBe(false); + }); + }); }); diff --git a/src/UrlParams.ts b/src/UrlParams.ts index e0aae237..423235ae 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -19,6 +19,12 @@ interface RoomIdentifier { viaServers: string[]; } +export enum UserIntent { + StartNewCall = "start_call", + JoinExistingCall = "join_existing", + Unknown = "unknown", +} + // 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 @@ -142,6 +148,13 @@ export interface UrlParams { * 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; } // This is here as a stopgap, but what would be far nicer is a function that @@ -211,6 +224,10 @@ 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; + } const widgetId = parser.getParam("widgetId"); const parentUrl = parser.getParam("parentUrl"); const isWidget = !!widgetId && !!parentUrl; @@ -243,11 +260,15 @@ export const getUrlParams = ( analyticsID: parser.getParam("analyticsID"), allowIceFallback: parser.getFlagParam("allowIceFallback"), perParticipantE2EE: parser.getFlagParam("perParticipantE2EE"), - skipLobby: parser.getFlagParam("skipLobby"), + skipLobby: parser.getFlagParam( + "skipLobby", + isWidget && intent === UserIntent.StartNewCall, + ), returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : true, theme: parser.getParam("theme"), viaServers: !isWidget ? parser.getParam("viaServers") : null, homeserver: !isWidget ? parser.getParam("homeserver") : null, + intent, }; }; From ba5da7e9af13a71d45cf9ee33a08523194e23656 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 18 Dec 2024 15:31:45 +0000 Subject: [PATCH 2/3] Inform user that their camera is starting in Lobby (#2869) * Inform user that their camera is starting Instead of just showing a grey box. * Review feedback * Show spinner from design suggestion * useMemo * Lint * Lint * Feedback from review * Use colour that actually exists * Refactor into Avatar superclass * . * Remove size limit behaviour * Add VideoPreview tests --- locales/en/app.json | 1 + src/Avatar.tsx | 2 +- src/room/VideoPreview.module.css | 11 +++++ src/room/VideoPreview.test.tsx | 73 ++++++++++++++++++++++++++++++++ src/room/VideoPreview.tsx | 37 +++++++++++----- src/tile/TileAvatar.module.css | 20 +++++++++ src/tile/TileAvatar.test.tsx | 27 ++++++++++++ src/tile/TileAvatar.tsx | 30 +++++++++++++ 8 files changed, 189 insertions(+), 12 deletions(-) create mode 100644 src/room/VideoPreview.test.tsx create mode 100644 src/tile/TileAvatar.module.css create mode 100644 src/tile/TileAvatar.test.tsx create mode 100644 src/tile/TileAvatar.tsx diff --git a/locales/en/app.json b/locales/en/app.json index a47e5beb..0e71fd4e 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -195,6 +195,7 @@ "version": "{{productName}} version: {{version}}", "video_tile": { "always_show": "Always show", + "camera_starting": "Video loading...", "change_fit_contain": "Fit to frame", "collapse": "Collapse", "expand": "Expand", diff --git a/src/Avatar.tsx b/src/Avatar.tsx index dcdead7a..a76afbca 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -33,7 +33,7 @@ export const sizes = new Map([ [Size.XL, 90], ]); -interface Props { +export interface Props { id: string; name: string; className?: string; diff --git a/src/room/VideoPreview.module.css b/src/room/VideoPreview.module.css index 89422af7..eeb9276b 100644 --- a/src/room/VideoPreview.module.css +++ b/src/room/VideoPreview.module.css @@ -25,6 +25,17 @@ video.mirror { transform: scaleX(-1); } +.preview .cameraStarting { + position: absolute; + top: var(--cpd-space-10x); + left: 0; + right: 0; + bottom: 0; + display: flex; + justify-content: center; + color: var(--cpd-color-text-secondary); +} + .avatarContainer { position: absolute; top: 0; diff --git a/src/room/VideoPreview.test.tsx b/src/room/VideoPreview.test.tsx new file mode 100644 index 00000000..068ad050 --- /dev/null +++ b/src/room/VideoPreview.test.tsx @@ -0,0 +1,73 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + +import { expect, describe, it, vi, beforeAll } from "vitest"; +import { render } from "@testing-library/react"; + +import { type MatrixInfo, VideoPreview } from "./VideoPreview"; +import { type MuteStates } from "./MuteStates"; +import { E2eeType } from "../e2ee/e2eeType"; + +function mockMuteStates({ audio = true, video = true } = {}): MuteStates { + return { + audio: { enabled: audio, setEnabled: vi.fn() }, + video: { enabled: video, setEnabled: vi.fn() }, + }; +} + +describe("VideoPreview", () => { + const matrixInfo: MatrixInfo = { + userId: "@a:example.org", + displayName: "Alice", + avatarUrl: "", + roomId: "", + roomName: "", + e2eeSystem: { kind: E2eeType.NONE }, + roomAlias: null, + roomAvatar: null, + }; + + beforeAll(() => { + window.ResizeObserver = class ResizeObserver { + public observe(): void { + // do nothing + } + public unobserve(): void { + // do nothing + } + public disconnect(): void { + // do nothing + } + }; + }); + + it("shows avatar with video disabled", () => { + const { queryByRole } = render( + } + />, + ); + expect(queryByRole("img", { name: "@a:example.org" })).toBeVisible(); + }); + + it("shows loading status with video enabled but no track", () => { + const { queryByRole } = render( + } + />, + ); + expect(queryByRole("status")).toHaveTextContent( + "video_tile.camera_starting", + ); + }); +}); diff --git a/src/room/VideoPreview.tsx b/src/room/VideoPreview.tsx index f9609b99..e2d8303f 100644 --- a/src/room/VideoPreview.tsx +++ b/src/room/VideoPreview.tsx @@ -5,12 +5,13 @@ SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { useEffect, useRef, type FC, type ReactNode } from "react"; +import { useEffect, useMemo, useRef, type FC, type ReactNode } from "react"; import useMeasure from "react-use-measure"; import { facingModeFromLocalTrack, type LocalVideoTrack } from "livekit-client"; import classNames from "classnames"; +import { useTranslation } from "react-i18next"; -import { Avatar } from "../Avatar"; +import { TileAvatar } from "../tile/TileAvatar"; import styles from "./VideoPreview.module.css"; import { type MuteStates } from "./MuteStates"; import { type EncryptionSystem } from "../e2ee/sharedKeyManagement"; @@ -39,6 +40,7 @@ export const VideoPreview: FC = ({ videoTrack, children, }) => { + const { t } = useTranslation(); const [previewRef, previewBounds] = useMeasure(); const videoEl = useRef(null); @@ -53,6 +55,11 @@ export const VideoPreview: FC = ({ }; }, [videoTrack]); + const cameraIsStarting = useMemo( + () => muteStates.video.enabled && !videoTrack, + [muteStates.video.enabled, videoTrack], + ); + return (
diff --git a/src/tile/TileAvatar.module.css b/src/tile/TileAvatar.module.css new file mode 100644 index 00000000..fa05c552 --- /dev/null +++ b/src/tile/TileAvatar.module.css @@ -0,0 +1,20 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + +.loading { + position: absolute; + top: 0; + left: 0; + right: 0; + bottom: 0; + display: flex; + justify-content: center; + align-items: center; + opacity: 0.5; + /* TODO: make this --cpd-color-fg-primary when available. */ + color: var(--cpd-color-text-primary); +} diff --git a/src/tile/TileAvatar.test.tsx b/src/tile/TileAvatar.test.tsx new file mode 100644 index 00000000..ae5ab610 --- /dev/null +++ b/src/tile/TileAvatar.test.tsx @@ -0,0 +1,27 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + +import { expect, describe, it } from "vitest"; +import { render } from "@testing-library/react"; + +import { TileAvatar } from "./TileAvatar"; + +describe("TileAvatar", () => { + it("should show loading spinner when loading", () => { + const { container } = render( + , + ); + expect(container.querySelector(".loading")).toBeInTheDocument(); + }); + + it("should not show loading spinner when not loading", () => { + const { container } = render( + , + ); + expect(container.querySelector(".loading")).not.toBeInTheDocument(); + }); +}); diff --git a/src/tile/TileAvatar.tsx b/src/tile/TileAvatar.tsx new file mode 100644 index 00000000..bba826cd --- /dev/null +++ b/src/tile/TileAvatar.tsx @@ -0,0 +1,30 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only +Please see LICENSE in the repository root for full details. +*/ + +import { type FC } from "react"; +import { InlineSpinner } from "@vector-im/compound-web"; + +import styles from "./TileAvatar.module.css"; +import { Avatar, type Props as AvatarProps } from "../Avatar"; + +interface Props extends AvatarProps { + size: number; + loading?: boolean; +} + +export const TileAvatar: FC = ({ size, loading, ...props }) => { + return ( +
+ {loading && ( +
+ +
+ )} + +
+ ); +}; From 6d5dc0dfb77823fdf8d13c1536d027a4d15f636a Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 18 Dec 2024 17:03:29 +0000 Subject: [PATCH 3/3] Fix loading of matrix-sdk-crypto-wasm when running in local development mode (#2915) Fixes problem loading rust crypto when using `yarn dev` --- vite.config.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/vite.config.js b/vite.config.js index 1feb7d66..7f088801 100644 --- a/vite.config.js +++ b/vite.config.js @@ -109,5 +109,12 @@ export default defineConfig(({ mode }) => { "@radix-ui/react-dismissable-layer", ], }, + // Vite is using esbuild in development mode, which doesn't work with the wasm loader + // in matrix-sdk-crypto-wasm, so we need to exclude it here. This doesn't affect the + // production build (which uses rollup) which still works as expected. + // https://vite.dev/guide/why.html#why-not-bundle-with-esbuild + optimizeDeps: { + exclude: ["@matrix-org/matrix-sdk-crypto-wasm"], + }, }; });