Merge pull request #3747 from JakeTripplJ/screenshare-volume

Add volume control to screen shares

locales/en/app.json

@@ -255,6 +255,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..."
   }

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) {

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<InCallViewProps> = ({
     ) : 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<number | null>(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<InCallViewProps> = ({
       key="audio"
       muted={!audioEnabled}
       onClick={toggleAudio ?? undefined}
-      onTouchEnd={onControlsTouchEnd}
       disabled={toggleAudio === null}
       data-testid="incall_mute"
     />,
@@ -675,7 +652,6 @@ export const InCallView: FC<InCallViewProps> = ({
       key="video"
       muted={!videoEnabled}
       onClick={toggleVideo ?? undefined}
-      onTouchEnd={onControlsTouchEnd}
       disabled={toggleVideo === null}
       data-testid="incall_videomute"
     />,
@@ -687,7 +663,6 @@ export const InCallView: FC<InCallViewProps> = ({
         className={styles.shareScreen}
         enabled={sharingScreen}
         onClick={vm.toggleScreenSharing}
-        onTouchEnd={onControlsTouchEnd}
         data-testid="incall_screenshare"
       />,
     );
@@ -699,18 +674,11 @@ export const InCallView: FC<InCallViewProps> = ({
         key="raise_hand"
         className={styles.raiseHand}
         identifier={`${client.getUserId()}:${client.getDeviceId()}`}
-        onTouchEnd={onControlsTouchEnd}
       />,
     );
   }
   if (layout.type !== "pip")
-    buttons.push(
-      <SettingsButton
-        key="settings"
-        onClick={openSettings}
-        onTouchEnd={onControlsTouchEnd}
-      />,
-    );
+    buttons.push(<SettingsButton key="settings" onClick={openSettings} />);
 
   buttons.push(
     <EndCallButton
@@ -718,7 +686,6 @@ export const InCallView: FC<InCallViewProps> = ({
       onClick={function (): void {
         vm.hangup();
       }}
-      onTouchEnd={onControlsTouchEnd}
       data-testid="incall_leave"
     />,
   );
@@ -751,7 +718,6 @@ export const InCallView: FC<InCallViewProps> = ({
           className={styles.layout}
           layout={gridMode}
           setLayout={setGridMode}
-          onTouchEnd={onControlsTouchEnd}
         />
       )}
     </div>
@@ -760,12 +726,13 @@ export const InCallView: FC<InCallViewProps> = ({
   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
     <div
       className={styles.inRoom}
       ref={containerRef}
-      onTouchStart={onTouchStart}
-      onTouchEnd={onTouchEnd}
-      onTouchCancel={onTouchCancel}
+      onClick={onViewClick}
       onPointerMove={onPointerMove}
       onPointerOut={onPointerOut}
     >

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<Props> = ({
-  layout,
-  setLayout,
-  className,
-  onTouchEnd,
-}) => {
+export const LayoutToggle: FC<Props> = ({ layout, setLayout, className }) => {
   const { t } = useTranslation();
 
   const onChange = useCallback(
@@ -47,7 +41,6 @@ export const LayoutToggle: FC<Props> = ({
           value="spotlight"
           checked={layout === "spotlight"}
           onChange={onChange}
-          onTouchEnd={onTouchEnd}
         />
       </Tooltip>
       <SpotlightIcon aria-hidden width={24} height={24} />
@@ -58,7 +51,6 @@ export const LayoutToggle: FC<Props> = ({
           value="grid"
           checked={layout === "grid"}
           onChange={onChange}
-          onTouchEnd={onTouchEnd}
         />
       </Tooltip>
       <GridIcon aria-hidden width={24} height={24} />

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,73 @@ 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 }) => {

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<boolean>;
+  /**
+   * Whether this screen share should be considered to have an audio track.
+   */
+  audioEnabled$: Behavior<boolean>;
 }
 
 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),
+      ),
+    ),
   };
 }

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;
 }
 

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(
+    <TooltipProvider>
+      <SpotlightTile
+        vm={new SpotlightTileViewModel(constant([vm]), constant(false))}
+        targetWidth={300}
+        targetHeight={200}
+        expanded={false}
+        onToggleExpanded={toggleExpanded}
+        showIndicators
+        focusable
+      />
+    </TooltipProvider>,
+  );
+
+  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(
+    <SpotlightTile
+      vm={new SpotlightTileViewModel(constant([vm]), constant(false))}
+      targetWidth={300}
+      targetHeight={200}
+      expanded={false}
+      onToggleExpanded={toggleExpanded}
+      showIndicators
+      focusable
+    />,
+  );
+
+  expect(await axe(container)).toHaveNoViolations();
+
+  // Volume menu button should not exist
+  expect(
+    screen.queryByRole("button", { name: /volume/i }),
+  ).not.toBeInTheDocument();
+});

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<HTMLDivElement>;
@@ -224,6 +231,73 @@ const SpotlightItem: FC<SpotlightItemProps> = ({
 
 SpotlightItem.displayName = "SpotlightItem";
 
+interface ScreenShareVolumeButtonProps {
+  vm: RemoteScreenShareViewModel;
+}
+
+const ScreenShareVolumeButton: FC<ScreenShareVolumeButtonProps> = ({ 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 && (
+      <Menu
+        open={volumeMenuOpen}
+        onOpenChange={setVolumeMenuOpen}
+        title={t("video_tile.screen_share_volume")}
+        side="top"
+        align="end"
+        trigger={
+          <button
+            className={styles.expand}
+            aria-label={t("video_tile.screen_share_volume")}
+          >
+            <VolumeSolidIcon aria-hidden width={20} height={20} />
+          </button>
+        }
+      >
+        <MenuItem
+          as="div"
+          className={styles.volumeMenuItem}
+          onSelect={null}
+          label={null}
+          hideChevron={true}
+        >
+          <button className={styles.menuMuteButton} onClick={onMuteButtonClick}>
+            <VolumeIcon aria-hidden width={24} height={24} />
+          </button>
+          <Slider
+            className={styles.volumeSlider}
+            label={t("video_tile.volume")}
+            value={playbackVolume}
+            min={0}
+            max={1}
+            step={0.01}
+            onValueChange={onVolumeChange}
+            onValueCommit={onVolumeCommit}
+          />
+        </MenuItem>
+      </Menu>
+    )
+  );
+};
+
 interface Props {
   ref?: Ref<HTMLDivElement>;
   vm: SpotlightTileViewModel;
@@ -258,6 +332,7 @@ export const SpotlightTile: FC<Props> = ({
   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;
 
@@ -365,16 +440,21 @@ export const SpotlightTile: FC<Props> = ({
           />
         ))}
       </div>
-      <div className={styles.bottomRightButtons}>
-        <button
-          className={classNames(styles.expand)}
-          aria-label={"maximise"}
-          onClick={onToggleFullscreen}
-          tabIndex={focusable ? undefined : -1}
-        >
-          <FullScreenIcon aria-hidden width={20} height={20} />
-        </button>
 
+      <div className={styles.bottomRightButtons}>
+        {visibleMedia?.type === "screen share" && !visibleMedia.local && (
+          <ScreenShareVolumeButton vm={visibleMedia} />
+        )}
+        {platform === "desktop" && (
+          <button
+            className={classNames(styles.expand)}
+            aria-label={"maximise"}
+            onClick={onToggleFullscreen}
+            tabIndex={focusable ? undefined : -1}
+          >
+            <FullScreenIcon aria-hidden width={20} height={20} />
+          </button>
+        )}
         {onToggleExpanded && (
           <button
             className={classNames(styles.expand)}

src/utils/test.ts

@@ -70,6 +70,10 @@ import {
   createRemoteUserMedia,
   type RemoteUserMediaViewModel,
 } from "../state/media/RemoteUserMediaViewModel";
+import {
+  createRemoteScreenShare,
+  type RemoteScreenShareViewModel,
+} from "../state/media/RemoteScreenShareViewModel";
 
 export function withFakeTimers(continuation: () => void): void {
   vi.useFakeTimers();
@@ -393,6 +397,31 @@ export function mockRemoteMedia(
   });
 }
 
+export function mockRemoteScreenShare(
+  rtcMember: CallMembership,
+  roomMember: Partial<RoomMember>,
+  participant: RemoteParticipant | null,
+  livekitRoom: LivekitRoom | undefined = mockLivekitRoom(
+    {},
+    {
+      remoteParticipants$: of(participant ? [participant] : []),
+    },
+  ),
+): RemoteScreenShareViewModel {
+  const member = mockMatrixRoomMember(rtcMember, roomMember);
+  return createRemoteScreenShare(testScope(), {
+    id: "screenshare",
+    userId: member.userId,
+    participant$: constant(participant),
+    encryptionSystem: { kind: E2eeType.PER_PARTICIPANT },
+    livekitRoom$: constant(livekitRoom),
+    focusUrl$: constant("https://rtc-example.org"),
+    pretendToBeDisconnected$: constant(false),
+    displayName$: constant(member.rawDisplayName ?? "nodisplayname"),
+    mxcAvatarUrl$: constant(member.getMxcAvatarUrl()),
+  });
+}
+
 export function mockConfig(
   config: Partial<ResolvedConfigOptions> = {},
 ): MockInstance<() => ResolvedConfigOptions> {