Merge pull request #4003 from element-hq/device-switch-fixes

Show the right labels in device switcher menus

src/components/CallFooter.stories.tsx

@@ -17,6 +17,9 @@ import { useStaticViewModel } from "../state/ViewModel";
 import { ReactionsSenderContext } from "../reactions/useReactionsSender";
 import { type ReactionOption } from "../reactions";
 import { type GridMode } from "../state/CallViewModel/CallViewModel";
+import { MediaDevicesContext } from "../MediaDevicesContext";
+import { MediaDevices } from "../state/MediaDevices";
+import { globalScope } from "../state/ObservableScope";
 // consts for tests
 const reactionIdentifier = "@user:example.com:DEVICE";
 const reactionData = {
@@ -24,6 +27,8 @@ const reactionData = {
   reactions$: new BehaviorSubject({}),
 };
 
+const mediaDevices = new MediaDevices(globalScope);
+
 /**
  * A wrapper component that is used for:
  *  - exposing the snapshot via props so the storybook documents the snapshot properties (basically unpack them form the vm)
@@ -41,17 +46,19 @@ function CallFooterStoryWrapper({
 }): ReactNode {
   const vm = useStaticViewModel(vmSnapshot);
   return (
-    <div className={inCallViewStyles.inRoom}>
-      <ReactionsSenderContext
-        value={{
-          supportsReactions: false,
-          toggleRaisedHand: async () => Promise.resolve(),
-          sendReaction: async (reaction: ReactionOption) => Promise.resolve(),
-        }}
-      >
-        <CallFooter vm={vm} />
-      </ReactionsSenderContext>
-    </div>
+    <MediaDevicesContext value={mediaDevices}>
+      <div className={inCallViewStyles.inRoom}>
+        <ReactionsSenderContext
+          value={{
+            supportsReactions: false,
+            toggleRaisedHand: async () => Promise.resolve(),
+            sendReaction: async (reaction: ReactionOption) => Promise.resolve(),
+          }}
+        >
+          <CallFooter vm={vm} />
+        </ReactionsSenderContext>
+      </div>
+    </MediaDevicesContext>
   );
 }
 

src/components/MediaMuteAndSwitchButton.stories.tsx

@@ -6,12 +6,25 @@ Please see LICENSE in the repository root for full details.
 */
 
 import { fn, userEvent, within, expect } from "storybook/test";
+import { type JSX } from "react";
 
 import type { Meta, StoryObj } from "@storybook/react-vite";
 import { MediaMuteAndSwitchButton } from "./MediaMuteAndSwitchButton";
+import { MediaDevicesContext } from "../MediaDevicesContext";
+import { MediaDevices } from "../state/MediaDevices";
+import { globalScope } from "../state/ObservableScope";
+
+const mediaDevices = new MediaDevices(globalScope);
 
 const meta = {
   component: MediaMuteAndSwitchButton,
+  decorators: [
+    (Story): JSX.Element => (
+      <MediaDevicesContext value={mediaDevices}>
+        <Story />
+      </MediaDevicesContext>
+    ),
+  ],
 } satisfies Meta<typeof MediaMuteAndSwitchButton>;
 
 export default meta;

src/components/MediaMuteAndSwitchButton.test.tsx

@@ -8,14 +8,35 @@ Please see LICENSE in the repository root for full details.
 import { describe, expect, test, vi } from "vitest";
 import { act, render, screen, type RenderResult } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { type JSX, useState } from "react";
+import { type JSX, useState, type ReactNode } from "react";
 import { TooltipProvider } from "@vector-im/compound-web";
 
 import { MediaMuteAndSwitchButton } from "./MediaMuteAndSwitchButton";
+import { MediaDevicesContext } from "../MediaDevicesContext";
+import { type MediaDevices } from "../state/MediaDevices";
+
+interface RenderOptions {
+  requestDeviceNames: () => void;
+}
+
+function renderComponent(
+  component: ReactNode,
+  { requestDeviceNames = (): void => {} }: Partial<RenderOptions> = {},
+): RenderResult {
+  return render(
+    <TooltipProvider>
+      <MediaDevicesContext
+        value={{ requestDeviceNames } as unknown as MediaDevices}
+      >
+        {component}
+      </MediaDevicesContext>
+    </TooltipProvider>,
+  );
+}
 
 describe("MediaMuteAndSwitchButton", () => {
   test("renders", () => {
-    const { container } = render(
+    const { container } = renderComponent(
       <TooltipProvider>
         <MediaMuteAndSwitchButton title={"Switcher"} iconsAndLabels={"audio"} />
       </TooltipProvider>,
@@ -28,14 +49,12 @@ describe("MediaMuteAndSwitchButton", () => {
       type: "video" | "audio",
       enabled: boolean,
     ): RenderResult => {
-      return render(
-        <TooltipProvider>
-          <MediaMuteAndSwitchButton
-            title={"Switcher"}
-            iconsAndLabels={type}
-            enabled={enabled}
-          />
-        </TooltipProvider>,
+      return renderComponent(
+        <MediaMuteAndSwitchButton
+          title={"Switcher"}
+          iconsAndLabels={type}
+          enabled={enabled}
+        />,
       );
     };
     const renderAudioEndabled = renderLabels("audio", true);
@@ -60,15 +79,13 @@ describe("MediaMuteAndSwitchButton", () => {
   test("calls mute on mute press", async () => {
     const user = userEvent.setup();
     const onMute = vi.fn();
-    const { getByRole } = render(
-      <TooltipProvider>
-        <MediaMuteAndSwitchButton
-          title={"Switcher"}
-          onMuteClick={onMute}
-          iconsAndLabels="audio"
-          enabled={true}
-        />
-      </TooltipProvider>,
+    const { getByRole } = renderComponent(
+      <MediaMuteAndSwitchButton
+        title={"Switcher"}
+        onMuteClick={onMute}
+        iconsAndLabels="audio"
+        enabled={true}
+      />,
     );
 
     await user.click(getByRole("switch", { name: "Mute microphone" }));
@@ -76,23 +93,74 @@ describe("MediaMuteAndSwitchButton", () => {
     expect(onMute).toHaveBeenCalled();
   });
 
-  test("calls select callback on menu click", async () => {
+  test("requests device names when opened", async () => {
     const user = userEvent.setup();
-    const onSelect = vi.fn();
-    const { getByRole } = render(
-      <TooltipProvider>
+    const requestDeviceNames = vi.fn();
+    renderComponent(
+      <MediaMuteAndSwitchButton
+        title="Switcher"
+        iconsAndLabels="audio"
+        enabled
+      />,
+      { requestDeviceNames },
+    );
+
+    expect(requestDeviceNames).not.toHaveBeenCalled();
+    await user.click(screen.getByRole("button", { name: "Microphone" }));
+    expect(requestDeviceNames).toHaveBeenCalled();
+  });
+
+  test("shows numbered devices correctly", async () => {
+    const user = userEvent.setup();
+    renderComponent(
+      <>
         <MediaMuteAndSwitchButton
           title="Switcher"
           iconsAndLabels="audio"
-          enabled={true}
+          enabled
           options={[
-            { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
-            { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
+            { label: { type: "number", number: 1 }, id: "mic1" },
+            { label: { type: "number", number: 2 }, id: "mic2" },
           ]}
           selectedOption="mic1"
-          onSelect={onSelect}
         />
-      </TooltipProvider>,
+        <MediaMuteAndSwitchButton
+          title="Switcher"
+          iconsAndLabels="video"
+          enabled
+          options={[
+            { label: { type: "number", number: 1 }, id: "cam1" },
+            { label: { type: "number", number: 2 }, id: "cam2" },
+          ]}
+          selectedOption="cam1"
+        />
+      </>,
+    );
+
+    await user.click(screen.getByRole("button", { name: "Microphone" }));
+    screen.getByRole("menuitem", { name: "Microphone 1" });
+    screen.getByRole("menuitem", { name: "Microphone 2" });
+    await user.keyboard("[Escape]");
+    await user.click(screen.getByRole("button", { name: "Camera" }));
+    screen.getByRole("menuitem", { name: "Camera 1" });
+    screen.getByRole("menuitem", { name: "Camera 2" });
+  });
+
+  test("calls select callback on menu click", async () => {
+    const user = userEvent.setup();
+    const onSelect = vi.fn();
+    const { getByRole } = renderComponent(
+      <MediaMuteAndSwitchButton
+        title="Switcher"
+        iconsAndLabels="audio"
+        enabled={true}
+        options={[
+          { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
+          { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
+        ]}
+        selectedOption="mic1"
+        onSelect={onSelect}
+      />,
     );
 
     await user.click(getByRole("button", { name: "Microphone" }));
@@ -103,20 +171,18 @@ describe("MediaMuteAndSwitchButton", () => {
   test("does not call select callback on already selected menu click", async () => {
     const user = userEvent.setup();
     const onSelect = vi.fn();
-    const { getByRole } = render(
-      <TooltipProvider>
-        <MediaMuteAndSwitchButton
-          title="Switcher"
-          iconsAndLabels="audio"
-          enabled={true}
-          options={[
-            { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
-            { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
-          ]}
-          selectedOption="mic1"
-          onSelect={onSelect}
-        />
-      </TooltipProvider>,
+    const { getByRole } = renderComponent(
+      <MediaMuteAndSwitchButton
+        title="Switcher"
+        iconsAndLabels="audio"
+        enabled={true}
+        options={[
+          { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
+          { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
+        ]}
+        selectedOption="mic1"
+        onSelect={onSelect}
+      />,
     );
 
     await user.click(getByRole("button", { name: "Microphone" }));
@@ -133,29 +199,27 @@ describe("MediaMuteAndSwitchButton", () => {
     function Wrapper(): JSX.Element {
       const [selectedOption, setSelectedOption] = useState("mic1");
       return (
-        <TooltipProvider>
-          <MediaMuteAndSwitchButton
-            title="Switcher"
-            iconsAndLabels="audio"
-            enabled={true}
-            options={[
-              { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
-              { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
-            ]}
-            selectedOption={selectedOption}
-            onSelect={(id) => {
-              onSelectPressed();
-              void promise.then(() => {
-                setSelectedOption(id);
-                onOptionUpdated();
-              });
-            }}
-          />
-        </TooltipProvider>
+        <MediaMuteAndSwitchButton
+          title="Switcher"
+          iconsAndLabels="audio"
+          enabled={true}
+          options={[
+            { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
+            { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
+          ]}
+          selectedOption={selectedOption}
+          onSelect={(id) => {
+            onSelectPressed();
+            void promise.then(() => {
+              setSelectedOption(id);
+              onOptionUpdated();
+            });
+          }}
+        />
       );
     }
 
-    const { getByRole } = render(<Wrapper />);
+    const { getByRole } = renderComponent(<Wrapper />);
 
     await user.click(getByRole("button", { name: "Microphone" }));
     await user.click(screen.getByRole("menuitem", { name: "Microphone 2" }));
@@ -188,16 +252,14 @@ describe("MediaMuteAndSwitchButton", () => {
     const user = userEvent.setup();
     const onSelect = vi.fn();
     const onVideoBlurToggle = vi.fn();
-    const { getByRole } = render(
-      <TooltipProvider>
-        <MediaMuteAndSwitchButton
-          title="Switcher"
-          iconsAndLabels="video"
-          enabled={true}
-          videoBlurToggleClick={onVideoBlurToggle}
-          onSelect={onSelect}
-        />
-      </TooltipProvider>,
+    const { getByRole } = renderComponent(
+      <MediaMuteAndSwitchButton
+        title="Switcher"
+        iconsAndLabels="video"
+        enabled={true}
+        videoBlurToggleClick={onVideoBlurToggle}
+        onSelect={onSelect}
+      />,
     );
 
     await user.click(getByRole("button", { name: "Camera" }));
@@ -215,19 +277,17 @@ describe("MediaMuteAndSwitchButton", () => {
 
   test("renders check icon to mark the selected menu item", async () => {
     const user = userEvent.setup();
-    const { getByRole } = render(
-      <TooltipProvider>
-        <MediaMuteAndSwitchButton
-          title="Switcher"
-          iconsAndLabels="audio"
-          enabled={true}
-          options={[
-            { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
-            { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
-          ]}
-          selectedOption="mic2"
-        />
-      </TooltipProvider>,
+    const { getByRole } = renderComponent(
+      <MediaMuteAndSwitchButton
+        title="Switcher"
+        iconsAndLabels="audio"
+        enabled={true}
+        options={[
+          { label: { type: "name", name: "Microphone 1" }, id: "mic1" },
+          { label: { type: "name", name: "Microphone 2" }, id: "mic2" },
+        ]}
+        selectedOption="mic2"
+      />,
     );
 
     // open menu

src/components/MediaMuteAndSwitchButton.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 ComponentType, useState, type FC } from "react";
+import { type ComponentType, useState, type FC, useEffect } from "react";
 import {
   Button,
   Menu,
@@ -26,6 +26,7 @@ import { useTranslation } from "react-i18next";
 import styles from "./MediaMuteAndSwitchButton.module.css";
 import { MicButton, VideoButton } from "../button";
 import { type DeviceLabel } from "../state/MediaDevices";
+import { useMediaDevices } from "../MediaDevicesContext";
 
 export interface MenuOptions {
   label: DeviceLabel;
@@ -69,6 +70,12 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
   const [plannedSelection, setPlannedSelection] = useState<string | null>(null);
   const [menuOpen, setMenuOpen] = useState(false);
   const { t } = useTranslation();
+  const devices = useMediaDevices();
+
+  useEffect(() => {
+    if (menuOpen) devices.requestDeviceNames(); // No-op after the first call
+  }, [menuOpen, devices]);
+
   let button;
   let toggles: { label: string; enabled: boolean; id: string }[] = [];
   switch (iconsAndLabels) {
@@ -119,15 +126,16 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
       IconOptions = VideoCallIcon;
       optionsButtonLabel = t("settings.devices.camera");
       numberedLabel = (n): string =>
-        t("settings.devices.microphone_numbered", { n });
+        t("settings.devices.camera_numbered", { n });
       break;
     case "audio":
       IconOptions = MicOnIcon;
       optionsButtonLabel = t("settings.devices.microphone");
       numberedLabel = (n): string =>
-        t("settings.devices.camera_numbered", { n });
+        t("settings.devices.microphone_numbered", { n });
       break;
   }
+
   return (
     <div
       className={classNames({

src/settings/SettingsModal.tsx

@@ -100,7 +100,7 @@ export const SettingsModal: FC<Props> = ({
 
   const devices = useMediaDevices();
   useEffect(() => {
-    if (open) devices.requestDeviceNames();
+    if (open) devices.requestDeviceNames(); // No-op after the first call
   }, [open, devices]);
 
   const [soundVolume, setSoundVolume] = useSetting(soundEffectVolumeSetting);