diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 00000000..787ddc73 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,39 @@ + + +## Content + + + +## Motivation and context + + + +## Screenshots / GIFs + + + +## Tests + + + +- Step 1 +- Step 2 +- Step ... +- + +## Checklist + +- [ ] I have read through [CONTRIBUTING.md](https://github.com/element-hq/element-call/blob/livekit/CONTRIBUTING.md). +- [ ] Pull request includes screenshots or videos if containing UI changes +- [ ] Tests written for new code (and old code if feasible). +- [ ] Linter and other CI checks pass. +- [ ] I have licensed the changes to Element by completing the [Contributor License Agreement (CLA)](https://cla-assistant.io/element-hq/element-call) diff --git a/.github/workflows/build-and-publish-docker.yaml b/.github/workflows/build-and-publish-docker.yaml index 68f7131c..6e8f01f5 100644 --- a/.github/workflows/build-and-publish-docker.yaml +++ b/.github/workflows/build-and-publish-docker.yaml @@ -20,7 +20,8 @@ jobs: runs-on: ubuntu-latest permissions: contents: write # required to upload release asset - packages: write + packages: write # needed for publishing packages to GHCR + id-token: write # needed for login into tailscale with GitHub OIDC Token steps: - name: Check it out uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 @@ -69,7 +70,7 @@ jobs: services/-repositories/secret/data/oci.element.io password | OCI_PASSWORD ; - name: Login to oci.element.io Registry - uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef # v3 + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3 if: github.event_name != 'pull_request' with: registry: oci-push.vpn.infra.element.io diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 9b86215e..4f9e80f2 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -49,6 +49,7 @@ jobs: permissions: contents: write packages: write + id-token: write uses: ./.github/workflows/build-and-publish-docker.yaml with: artifact_run_id: ${{ github.run_id }} diff --git a/.github/workflows/pr-deploy.yaml b/.github/workflows/pr-deploy.yaml index fe934162..62b37aca 100644 --- a/.github/workflows/pr-deploy.yaml +++ b/.github/workflows/pr-deploy.yaml @@ -60,6 +60,7 @@ jobs: permissions: contents: write packages: write + id-token: write uses: ./.github/workflows/build-and-publish-docker.yaml with: artifact_run_id: ${{ github.event.workflow_run.id || github.run_id }} diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 7f2c58fe..ade91019 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -55,6 +55,7 @@ jobs: permissions: contents: write packages: write + id-token: write uses: ./.github/workflows/build-and-publish-docker.yaml with: artifact_run_id: ${{ github.event.workflow_run.id || github.run_id }} diff --git a/docs/url-params.md b/docs/url-params.md index a474daed..e24e9823 100644 --- a/docs/url-params.md +++ b/docs/url-params.md @@ -46,32 +46,32 @@ possible to support encryption. These parameters are relevant to both [widget](./embedded-standalone.md) and [standalone](./embedded-standalone.md) modes: -| Name | Values | Required for widget | Required for SPA | Description | -| ------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `intent` | `start_call`, `join_existing`, `start_call_dm`, `join_existing_dm. | No, defaults to `start_call` | No, defaults to `start_call` | The intent is a special url parameter that defines the defaults for all the other parameters. In most cases it should be enough to only set the intent to setup element-call. | -| `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` (deprecated: use `posthogUserId` instead) | 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. | -| `confineToRoom` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Keeps the user confined to the current call/room. | -| `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...`. | -| `header` | `none`, `standard` or `app_bar` | No, defaults to `standard` | No, defaults to `standard` | 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`. | -| `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. | -| `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. | -| `controlledAudioDevices` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Whether the [global JS controls for audio devices](./controls.md#audio-devices) should be enabled, allowing the list of audio devices to be controlled by the app hosting Element Call. | -| `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. | -| `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. | -| `sendNotificationType` | `ring` or `notification` | No | No | Will send a "ring" or "notification" `m.rtc.notification` event if the user is the first one in the call. | -| `autoLeaveWhenOthersLeft` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Whether the app should automatically leave the call when there is no one left in the call. | -| `waitForCallPickup` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | When sending a notification, show UI that the app is awaiting an answer, play a dial tone, and (in widget mode) auto-close the widget once the notification expires. | +| Name | Values | Required for widget | Required for SPA | Description | +| ---------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `intent` | `start_call`, `join_existing`, `start_call_dm`, `join_existing_dm. | No, defaults to `start_call` | No, defaults to `start_call` | The intent is a special url parameter that defines the defaults for all the other parameters. In most cases it should be enough to only set the intent to setup element-call. | +| `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. | +| `posthogUserId` | 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. | +| `confineToRoom` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Keeps the user confined to the current call/room. | +| `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...`. | +| `header` | `none`, `standard` or `app_bar` | No, defaults to `standard` | No, defaults to `standard` | 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`. | +| `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. | +| `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. | +| `controlledAudioDevices` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Whether the [global JS controls for audio devices](./controls.md#audio-devices) should be enabled, allowing the list of audio devices to be controlled by the app hosting Element Call. | +| `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. | +| `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. | +| `sendNotificationType` | `ring` or `notification` | No | No | Will send a "ring" or "notification" `m.rtc.notification` event if the user is the first one in the call. | +| `autoLeaveWhenOthersLeft` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | Whether the app should automatically leave the call when there is no one left in the call. | +| `waitForCallPickup` | `true` or `false` | No, defaults to `false` | No, defaults to `false` | When sending a notification, show UI that the app is awaiting an answer, play a dial tone, and (in widget mode) auto-close the widget once the notification expires. | ### Widget-only parameters diff --git a/locales/en/app.json b/locales/en/app.json index 9a85478f..9b1a5675 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -254,6 +254,7 @@ "expand": "Expand", "mute_for_me": "Mute for me", "muted_for_me": "Muted for me", + "screen_share_volume": "Screen share volume", "volume": "Volume", "waiting_for_media": "Waiting for media..." } diff --git a/src/UrlParams.ts b/src/UrlParams.ts index f8ee22fb..31101197 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -473,8 +473,7 @@ export const computeUrlParams = (search = "", hash = ""): UrlParams => { homeserver: !isWidget ? parser.getParam("homeserver") : null, posthogApiHost: parser.getParam("posthogApiHost"), posthogApiKey: parser.getParam("posthogApiKey"), - posthogUserId: - parser.getParam("posthogUserId") ?? parser.getParam("analyticsID"), + posthogUserId: parser.getParam("posthogUserId"), rageshakeSubmitUrl: parser.getParam("rageshakeSubmitUrl"), sentryDsn: parser.getParam("sentryDsn"), sentryEnvironment: parser.getParam("sentryEnvironment"), diff --git a/src/room/InCallView.module.css b/src/room/InCallView.module.css index 96b8a368..55724932 100644 --- a/src/room/InCallView.module.css +++ b/src/room/InCallView.module.css @@ -65,6 +65,7 @@ Please see LICENSE in the repository root for full details. .footer.overlay.hidden { display: grid; opacity: 0; + pointer-events: none; } .footer.overlay:has(:focus-visible) { diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 135745eb..aceb07cf 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -9,8 +9,8 @@ import { IconButton, Text, Tooltip } from "@vector-im/compound-web"; import { type MatrixClient, type Room as MatrixRoom } from "matrix-js-sdk"; import { type FC, - type PointerEvent, - type TouchEvent, + type MouseEvent as ReactMouseEvent, + type PointerEvent as ReactPointerEvent, useCallback, useEffect, useMemo, @@ -110,8 +110,6 @@ import { ObservableScope } from "../state/ObservableScope.ts"; const logger = rootLogger.getChild("[InCallView]"); -const maxTapDurationMs = 400; - export interface ActiveCallProps extends Omit< InCallViewProps, "vm" | "livekitRoom" | "connState" @@ -334,40 +332,20 @@ export const InCallView: FC = ({ ) : null; }, [ringOverlay]); - // Ideally we could detect taps by listening for click events and checking - // that the pointerType of the event is "touch", but this isn't yet supported - // in Safari: https://developer.mozilla.org/en-US/docs/Web/API/Element/click_event#browser_compatibility - // Instead we have to watch for sufficiently fast touch events. - const touchStart = useRef(null); - const onTouchStart = useCallback(() => (touchStart.current = Date.now()), []); - const onTouchEnd = useCallback(() => { - const start = touchStart.current; - if (start !== null && Date.now() - start <= maxTapDurationMs) - vm.tapScreen(); - touchStart.current = null; - }, [vm]); - const onTouchCancel = useCallback(() => (touchStart.current = null), []); - - // We also need to tell the footer controls to prevent touch events from - // bubbling up, or else the footer will be dismissed before a click/change - // event can be registered on the control - const onControlsTouchEnd = useCallback( - (e: TouchEvent) => { - // Somehow applying pointer-events: none to the controls when the footer - // is hidden is not enough to stop clicks from happening as the footer - // becomes visible, so we check manually whether the footer is shown - if (showFooter) { - e.stopPropagation(); - vm.tapControls(); - } else { - e.preventDefault(); - } + const onViewClick = useCallback( + (e: ReactMouseEvent) => { + if ( + (e.nativeEvent as PointerEvent).pointerType === "touch" && + // If an interactive element was tapped, don't count this as a tap on the screen + (e.target as Element).closest?.("button, input") === null + ) + vm.tapScreen(); }, - [vm, showFooter], + [vm], ); const onPointerMove = useCallback( - (e: PointerEvent) => { + (e: ReactPointerEvent) => { if (e.pointerType === "mouse") vm.hoverScreen(); }, [vm], @@ -667,7 +645,6 @@ export const InCallView: FC = ({ key="audio" muted={!audioEnabled} onClick={toggleAudio ?? undefined} - onTouchEnd={onControlsTouchEnd} disabled={toggleAudio === null} data-testid="incall_mute" />, @@ -675,7 +652,6 @@ export const InCallView: FC = ({ key="video" muted={!videoEnabled} onClick={toggleVideo ?? undefined} - onTouchEnd={onControlsTouchEnd} disabled={toggleVideo === null} data-testid="incall_videomute" />, @@ -687,7 +663,6 @@ export const InCallView: FC = ({ className={styles.shareScreen} enabled={sharingScreen} onClick={vm.toggleScreenSharing} - onTouchEnd={onControlsTouchEnd} data-testid="incall_screenshare" />, ); @@ -699,18 +674,11 @@ export const InCallView: FC = ({ key="raise_hand" className={styles.raiseHand} identifier={`${client.getUserId()}:${client.getDeviceId()}`} - onTouchEnd={onControlsTouchEnd} />, ); } if (layout.type !== "pip") - buttons.push( - , - ); + buttons.push(); buttons.push( = ({ onClick={function (): void { vm.hangup(); }} - onTouchEnd={onControlsTouchEnd} data-testid="incall_leave" />, ); @@ -751,7 +718,6 @@ export const InCallView: FC = ({ className={styles.layout} layout={gridMode} setLayout={setGridMode} - onTouchEnd={onControlsTouchEnd} /> )} @@ -760,12 +726,13 @@ export const InCallView: FC = ({ const allConnections = useBehavior(vm.allConnections$); return ( + // The onClick handler here exists to control the visibility of the footer, + // and the footer is also viewable by moving focus into it, so this is fine. + // eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events
diff --git a/src/room/LayoutToggle.tsx b/src/room/LayoutToggle.tsx index 6cddc95f..ca6aa467 100644 --- a/src/room/LayoutToggle.tsx +++ b/src/room/LayoutToggle.tsx @@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { type ChangeEvent, type FC, type TouchEvent, useCallback } from "react"; +import { type ChangeEvent, type FC, useCallback } from "react"; import { useTranslation } from "react-i18next"; import { Tooltip } from "@vector-im/compound-web"; import { @@ -22,15 +22,9 @@ interface Props { layout: Layout; setLayout: (layout: Layout) => void; className?: string; - onTouchEnd?: (e: TouchEvent) => void; } -export const LayoutToggle: FC = ({ - layout, - setLayout, - className, - onTouchEnd, -}) => { +export const LayoutToggle: FC = ({ layout, setLayout, className }) => { const { t } = useTranslation(); const onChange = useCallback( @@ -47,7 +41,6 @@ export const LayoutToggle: FC = ({ value="spotlight" checked={layout === "spotlight"} onChange={onChange} - onTouchEnd={onTouchEnd} /> @@ -58,7 +51,6 @@ export const LayoutToggle: FC = ({ value="grid" checked={layout === "grid"} onChange={onChange} - onTouchEnd={onTouchEnd} /> diff --git a/src/state/media/MediaViewModel.test.ts b/src/state/media/MediaViewModel.test.ts index 4cb137db..f64dd3ee 100644 --- a/src/state/media/MediaViewModel.test.ts +++ b/src/state/media/MediaViewModel.test.ts @@ -9,6 +9,7 @@ import { expect, onTestFinished, test, vi } from "vitest"; import { type LocalTrackPublication, LocalVideoTrack, + Track, TrackEvent, } from "livekit-client"; import { waitFor } from "@testing-library/dom"; @@ -21,6 +22,7 @@ import { mockRemoteMedia, withTestScheduler, mockRemoteParticipant, + mockRemoteScreenShare, } from "../../utils/test"; import { constant } from "../Behavior"; @@ -91,6 +93,88 @@ test("control a participant's volume", () => { }); }); +test("control a participant's screen share volume", () => { + const setVolumeSpy = vi.fn(); + const vm = mockRemoteScreenShare( + rtcMembership, + {}, + mockRemoteParticipant({ setVolume: setVolumeSpy }), + ); + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-ab---c---d|", { + a() { + // Try muting by toggling + vm.togglePlaybackMuted(); + expect(setVolumeSpy).toHaveBeenLastCalledWith( + 0, + Track.Source.ScreenShareAudio, + ); + }, + b() { + // Try unmuting by dragging the slider back up + vm.adjustPlaybackVolume(0.6); + vm.adjustPlaybackVolume(0.8); + vm.commitPlaybackVolume(); + expect(setVolumeSpy).toHaveBeenCalledWith( + 0.6, + Track.Source.ScreenShareAudio, + ); + expect(setVolumeSpy).toHaveBeenLastCalledWith( + 0.8, + Track.Source.ScreenShareAudio, + ); + }, + c() { + // Try muting by dragging the slider back down + vm.adjustPlaybackVolume(0.2); + vm.adjustPlaybackVolume(0); + vm.commitPlaybackVolume(); + expect(setVolumeSpy).toHaveBeenCalledWith( + 0.2, + Track.Source.ScreenShareAudio, + ); + expect(setVolumeSpy).toHaveBeenLastCalledWith( + 0, + Track.Source.ScreenShareAudio, + ); + }, + d() { + // Try unmuting by toggling + vm.togglePlaybackMuted(); + // The volume should return to the last non-zero committed volume + expect(setVolumeSpy).toHaveBeenLastCalledWith( + 0.8, + Track.Source.ScreenShareAudio, + ); + }, + }); + expectObservable(vm.playbackVolume$).toBe("ab(cd)(ef)g", { + a: 1, + b: 0, + c: 0.6, + d: 0.8, + e: 0.2, + f: 0, + g: 0.8, + }); + }); +}); + +test("toggle fit/contain for a participant's video", () => { + const vm = mockRemoteMedia(rtcMembership, {}, mockRemoteParticipant({})); + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-ab|", { + a: () => vm.toggleCropVideo(), + b: () => vm.toggleCropVideo(), + }); + expectObservable(vm.cropVideo$).toBe("abc", { + a: true, + b: false, + c: true, + }); + }); +}); + test("local media remembers whether it should always be shown", () => { const vm1 = mockLocalMedia( rtcMembership, diff --git a/src/state/media/RemoteScreenShareViewModel.ts b/src/state/media/RemoteScreenShareViewModel.ts index eff6d9c1..8c46aeb3 100644 --- a/src/state/media/RemoteScreenShareViewModel.ts +++ b/src/state/media/RemoteScreenShareViewModel.ts @@ -6,8 +6,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { type RemoteParticipant } from "livekit-client"; -import { map } from "rxjs"; +import { Track, type RemoteParticipant } from "livekit-client"; +import { map, of, switchMap } from "rxjs"; import { type Behavior } from "../Behavior"; import { @@ -16,13 +16,20 @@ import { createBaseScreenShare, } from "./ScreenShareViewModel"; import { type ObservableScope } from "../ObservableScope"; +import { createVolumeControls, type VolumeControls } from "../VolumeControls"; +import { observeTrackReference$ } from "../observeTrackReference"; -export interface RemoteScreenShareViewModel extends BaseScreenShareViewModel { +export interface RemoteScreenShareViewModel + extends BaseScreenShareViewModel, VolumeControls { local: false; /** * Whether this screen share's video should be displayed. */ videoEnabled$: Behavior; + /** + * Whether this screen share should be considered to have an audio track. + */ + audioEnabled$: Behavior; } export interface RemoteScreenShareInputs extends BaseScreenShareInputs { @@ -36,9 +43,30 @@ export function createRemoteScreenShare( ): RemoteScreenShareViewModel { return { ...createBaseScreenShare(scope, inputs), + ...createVolumeControls(scope, { + pretendToBeDisconnected$, + sink$: scope.behavior( + inputs.participant$.pipe( + map( + (p) => (volume) => + p?.setVolume(volume, Track.Source.ScreenShareAudio), + ), + ), + ), + }), local: false, videoEnabled$: scope.behavior( pretendToBeDisconnected$.pipe(map((disconnected) => !disconnected)), ), + audioEnabled$: scope.behavior( + inputs.participant$.pipe( + switchMap((p) => + p + ? observeTrackReference$(p, Track.Source.ScreenShareAudio) + : of(null), + ), + map(Boolean), + ), + ), }; } diff --git a/src/tile/SpotlightTile.module.css b/src/tile/SpotlightTile.module.css index 622496d2..af0e0add 100644 --- a/src/tile/SpotlightTile.module.css +++ b/src/tile/SpotlightTile.module.css @@ -84,7 +84,6 @@ Please see LICENSE in the repository root for full details. .expand { appearance: none; cursor: pointer; - opacity: 0; padding: var(--cpd-space-2x); border: none; border-radius: var(--cpd-radius-pill-effect); @@ -108,6 +107,35 @@ Please see LICENSE in the repository root for full details. z-index: 1; } +.volumeSlider { + width: 100%; + min-width: 172px; +} + +/* Disable the hover effect for the screen share volume menu button */ +.volumeMenuItem:hover { + background: transparent; + cursor: default; +} + +.volumeMenuItem { + gap: var(--cpd-space-3x); +} + +.menuMuteButton { + appearance: none; + background: none; + border: none; + padding: 0; + cursor: pointer; + display: flex; +} + +/* Make icons change color with the theme */ +.menuMuteButton > svg { + color: var(--cpd-color-icon-primary); +} + .expand > svg { display: block; color: var(--cpd-color-icon-primary); @@ -119,17 +147,22 @@ Please see LICENSE in the repository root for full details. } } -.expand:active { +.expand:active, +.expand[data-state="open"] { background: var(--cpd-color-gray-100); } @media (hover) { + .tile > div > button { + opacity: 0; + } .tile:hover > div > button { opacity: 1; } } -.tile:has(:focus-visible) > div > button { +.tile:has(:focus-visible) > div > button, +.tile > div:has([data-state="open"]) > button { opacity: 1; } diff --git a/src/tile/SpotlightTile.test.tsx b/src/tile/SpotlightTile.test.tsx index a5332194..aac81b9c 100644 --- a/src/tile/SpotlightTile.test.tsx +++ b/src/tile/SpotlightTile.test.tsx @@ -9,6 +9,7 @@ import { test, expect, vi } from "vitest"; import { isInaccessible, render, screen } from "@testing-library/react"; import { axe } from "vitest-axe"; import userEvent from "@testing-library/user-event"; +import { TooltipProvider } from "@vector-im/compound-web"; import { SpotlightTile } from "./SpotlightTile"; import { @@ -18,6 +19,7 @@ import { mockLocalMedia, mockRemoteMedia, mockRemoteParticipant, + mockRemoteScreenShare, } from "../utils/test"; import { SpotlightTileViewModel } from "../state/TileViewModel"; import { constant } from "../state/Behavior"; @@ -78,3 +80,63 @@ test("SpotlightTile is accessible", async () => { await user.click(screen.getByRole("button", { name: "Expand" })); expect(toggleExpanded).toHaveBeenCalled(); }); + +test("Screen share volume UI is shown when screen share has audio", async () => { + const vm = mockRemoteScreenShare( + mockRtcMembership("@alice:example.org", "AAAA"), + {}, + mockRemoteParticipant({}), + ); + + vi.spyOn(vm, "audioEnabled$", "get").mockReturnValue(constant(true)); + + const toggleExpanded = vi.fn(); + const { container } = render( + + + , + ); + + expect(await axe(container)).toHaveNoViolations(); + + // Volume menu button should exist + expect(screen.queryByRole("button", { name: /volume/i })).toBeInTheDocument(); +}); + +test("Screen share volume UI is hidden when screen share has no audio", async () => { + const vm = mockRemoteScreenShare( + mockRtcMembership("@alice:example.org", "AAAA"), + {}, + mockRemoteParticipant({}), + ); + + vi.spyOn(vm, "audioEnabled$", "get").mockReturnValue(constant(false)); + + const toggleExpanded = vi.fn(); + const { container } = render( + , + ); + + expect(await axe(container)).toHaveNoViolations(); + + // Volume menu button should not exist + expect( + screen.queryByRole("button", { name: /volume/i }), + ).not.toBeInTheDocument(); +}); diff --git a/src/tile/SpotlightTile.tsx b/src/tile/SpotlightTile.tsx index 16a8679d..aa66d6b6 100644 --- a/src/tile/SpotlightTile.tsx +++ b/src/tile/SpotlightTile.tsx @@ -20,6 +20,10 @@ import { CollapseIcon, ChevronLeftIcon, ChevronRightIcon, + VolumeOffIcon, + VolumeOnIcon, + VolumeOffSolidIcon, + VolumeOnSolidIcon, } from "@vector-im/compound-design-tokens/assets/web/icons"; import { animated } from "@react-spring/web"; import { type Observable, map } from "rxjs"; @@ -27,6 +31,7 @@ import { useObservableRef } from "observable-hooks"; import { useTranslation } from "react-i18next"; import classNames from "classnames"; import { type TrackReferenceOrPlaceholder } from "@livekit/components-core"; +import { Menu, MenuItem } from "@vector-im/compound-web"; import FullScreenMaximiseIcon from "../icons/FullScreenMaximise.svg?react"; import FullScreenMinimiseIcon from "../icons/FullScreenMinimise.svg?react"; @@ -45,6 +50,8 @@ import { type UserMediaViewModel } from "../state/media/UserMediaViewModel"; import { type ScreenShareViewModel } from "../state/media/ScreenShareViewModel"; import { type RemoteScreenShareViewModel } from "../state/media/RemoteScreenShareViewModel"; import { type MediaViewModel } from "../state/media/MediaViewModel"; +import { Slider } from "../Slider"; +import { platform } from "../Platform"; interface SpotlightItemBaseProps { ref?: Ref; @@ -240,6 +247,73 @@ const SpotlightItem: FC = ({ SpotlightItem.displayName = "SpotlightItem"; +interface ScreenShareVolumeButtonProps { + vm: RemoteScreenShareViewModel; +} + +const ScreenShareVolumeButton: FC = ({ vm }) => { + const { t } = useTranslation(); + + const audioEnabled = useBehavior(vm.audioEnabled$); + const playbackMuted = useBehavior(vm.playbackMuted$); + const playbackVolume = useBehavior(vm.playbackVolume$); + + const VolumeIcon = playbackMuted ? VolumeOffIcon : VolumeOnIcon; + const VolumeSolidIcon = playbackMuted + ? VolumeOffSolidIcon + : VolumeOnSolidIcon; + + const [volumeMenuOpen, setVolumeMenuOpen] = useState(false); + const onMuteButtonClick = useCallback(() => vm.togglePlaybackMuted(), [vm]); + const onVolumeChange = useCallback( + (v: number) => vm.adjustPlaybackVolume(v), + [vm], + ); + const onVolumeCommit = useCallback(() => vm.commitPlaybackVolume(), [vm]); + + return ( + audioEnabled && ( + + + + } + > + + + + + + ) + ); +}; + interface Props { ref?: Ref; vm: SpotlightTileViewModel; @@ -274,6 +348,7 @@ export const SpotlightTile: FC = ({ const latestMedia = useLatest(media); const latestVisibleId = useLatest(visibleId); const visibleIndex = media.findIndex((vm) => vm.id === visibleId); + const visibleMedia = media.at(visibleIndex); const canGoBack = visibleIndex > 0; const canGoToNext = visibleIndex !== -1 && visibleIndex < media.length - 1; @@ -381,16 +456,21 @@ export const SpotlightTile: FC = ({ /> ))}
-
- +
+ {visibleMedia?.type === "screen share" && !visibleMedia.local && ( + + )} + {platform === "desktop" && ( + + )} {onToggleExpanded && (