Use radio controls to mark the selected device in the quick menu

- Mark the selection with a radio control instead of a trailing check icon
  beside a device glyph, matching the design and the settings modal, so both
  device pickers read the same way.
- Take the visual from Compound's RadioInput rather than restyling a span of
  our own. RadioControl is wrapped in a Radix form control and needs a Form
  ancestor, which a dropdown menu has no business providing; RadioInput does
  not.
- Keep rows as menuitemradio with aria-checked, rendered through
  MenuItem as="div" so an input is never nested inside a button.
- Render the radio aria-hidden and not focusable: Radix owns focus inside the
  menu, and aria-checked carries the state.
- Leave the activating spinner unchanged.
- Keep every scope.behavior call inside the function that owns the scope,
  which no-observablescope-leak requires.
- Cover the speaker section, an output that cannot be chosen, and a lone
  device shown disabled, with unit tests and stories for each.

Spec: FEATURES_SPEC/2026-09_Quick_Audio_Menu.md — AC1, AC7
This commit is contained in:
fkwp
2026-09-16 14:02:01 +02:00
parent ef2cbbbaea
commit 664fac104f
4 changed files with 252 additions and 107 deletions
+18 -46
View File
@@ -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. Please see LICENSE in the repository root for full details.
*/ */
import { combineLatest, map, switchMap } from "rxjs"; import { combineLatest, map, type Observable, switchMap } from "rxjs";
import { supportsBackgroundProcessors } from "@livekit/track-processors"; import { supportsBackgroundProcessors } from "@livekit/track-processors";
import { type CallViewModel } from "../state/CallViewModel/CallViewModel"; import { type CallViewModel } from "../state/CallViewModel/CallViewModel";
@@ -76,63 +76,35 @@ function buildDeviceBehaviors(
| "toggleBlur$" | "toggleBlur$"
| "videoBlurEnabled$" | "videoBlurEnabled$"
> { > {
return { const options$ = (
audioOptions$: scope.behavior( available$: Behavior<Map<string, MenuOptions["label"]>>,
disableSwitcher$.pipe( ): Observable<MenuOptions[]> =>
switchMap((disable) => disableSwitcher$.pipe(
disable switchMap((disable) =>
? constant([] as MenuOptions[]) disable
: mediaDevices.audioInput.available$.pipe( ? constant([] as MenuOptions[])
map((available) => : available$.pipe(
[...available.entries()].map(([id, label]) => ({ map((available) =>
id, [...available.entries()].map(([id, label]) => ({ id, label })),
label,
})),
),
), ),
), ),
), ),
), );
return {
audioOptions$: scope.behavior(options$(mediaDevices.audioInput.available$)),
selectedAudio$: scope.behavior( selectedAudio$: scope.behavior(
mediaDevices.audioInput.selected$.pipe(map((s) => s?.id)), mediaDevices.audioInput.selected$.pipe(map((s) => s?.id)),
), ),
selectAudioButtonOption$: constant(mediaDevices.audioInput.select), selectAudioButtonOption$: constant(mediaDevices.audioInput.select),
audioOutputOptions$: scope.behavior( audioOutputOptions$: scope.behavior(
disableSwitcher$.pipe( options$(mediaDevices.audioOutput.available$),
switchMap((disable) =>
disable
? constant([] as MenuOptions[])
: mediaDevices.audioOutput.available$.pipe(
map((available) =>
[...available.entries()].map(([id, label]) => ({
id,
label,
})),
),
),
),
),
), ),
selectedAudioOutput$: scope.behavior( selectedAudioOutput$: scope.behavior(
mediaDevices.audioOutput.selected$.pipe(map((s) => s?.id)), mediaDevices.audioOutput.selected$.pipe(map((s) => s?.id)),
), ),
selectAudioOutputOption$: constant(mediaDevices.audioOutput.select), selectAudioOutputOption$: constant(mediaDevices.audioOutput.select),
videoOptions$: scope.behavior( videoOptions$: scope.behavior(options$(mediaDevices.videoInput.available$)),
disableSwitcher$.pipe(
switchMap((disable) =>
disable
? constant([] as MenuOptions[])
: mediaDevices.videoInput.available$.pipe(
map((available) =>
[...available.entries()].map(([id, label]) => ({
id,
label,
})),
),
),
),
),
),
selectedVideo$: scope.behavior( selectedVideo$: scope.behavior(
mediaDevices.videoInput.selected$.pipe(map((s) => s?.id)), mediaDevices.videoInput.selected$.pipe(map((s) => s?.id)),
), ),
@@ -113,3 +113,87 @@ export const VideoUnmute: Story = {
selectedOption: "2", selectedOption: "2",
}, },
}; };
export const SpeakerAndMicrophoneSections: Story = {
args: {
...Default.args,
title: "Microphone",
iconsAndLabels: "audio",
enabled: true,
options: [
{ label: { type: "name", name: "Microphone 1" }, id: "mic1" },
{ label: { type: "name", name: "Microphone 2" }, id: "mic2" },
],
selectedOption: "mic1",
outputOptions: [
{ label: { type: "default", name: "Built-in Output" }, id: "default" },
{ label: { type: "name", name: "Headset" }, id: "spk2" },
],
selectedOutputOption: "default",
onSelectOutput: fn(),
},
play: async ({ args, canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(canvas.getByRole("button", { name: "Microphone" }));
const headset = await within(document.body).findByRole("menuitemradio", {
name: "Headset",
});
await userEvent.click(headset);
await expect(args.onSelectOutput).toHaveBeenCalledWith("spk2");
},
};
export const OutputCannotBeChosen: Story = {
args: {
...Default.args,
title: "Microphone",
iconsAndLabels: "audio",
enabled: true,
options: [
{ label: { type: "name", name: "Microphone 1" }, id: "mic1" },
{ label: { type: "name", name: "Microphone 2" }, id: "mic2" },
],
selectedOption: "mic1",
outputOptions: [
{ label: { type: "name", name: "Speakers" }, id: "spk1" },
{ label: { type: "name", name: "Headset" }, id: "spk2" },
],
selectedOutputOption: "spk1",
// No callback: the speakers are listed, but none can be picked.
onSelectOutput: undefined,
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(canvas.getByRole("button", { name: "Microphone" }));
const speakers = await within(document.body).findByRole("menuitemradio", {
name: "Speakers",
});
await expect(speakers).toHaveAttribute("aria-disabled", "true");
},
};
export const OnlyOneDevice: Story = {
args: {
...Default.args,
title: "Microphone",
iconsAndLabels: "audio",
enabled: true,
options: [{ label: { type: "name", name: "Microphone 1" }, id: "mic1" }],
selectedOption: "mic1",
outputOptions: [{ label: { type: "name", name: "Speakers" }, id: "spk1" }],
selectedOutputOption: "spk1",
onSelectOutput: fn(),
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(canvas.getByRole("button", { name: "Microphone" }));
// Shown rather than hidden, so the menu keeps its shape everywhere.
const only = await within(document.body).findByRole("menuitemradio", {
name: "Microphone 1",
});
await expect(only).toHaveAttribute("aria-disabled", "true");
},
};
+125 -11
View File
@@ -327,7 +327,7 @@ describe("MediaMuteAndSwitchButton", () => {
expect(onVideoBlurToggle).toHaveBeenCalled(); expect(onVideoBlurToggle).toHaveBeenCalled();
}); });
test("renders check icon to mark the selected menu item", async () => { test("marks the selected menu item as checked", async () => {
const user = userEvent.setup(); const user = userEvent.setup();
const { getByRole } = renderComponent( const { getByRole } = renderComponent(
<MediaMuteAndSwitchButton <MediaMuteAndSwitchButton
@@ -342,19 +342,133 @@ describe("MediaMuteAndSwitchButton", () => {
/>, />,
); );
// open menu
await user.click(getByRole("button", { name: "Microphone" })); await user.click(getByRole("button", { name: "Microphone" }));
// The selected item (mic2) renders both an IconOptions SVG and a CheckIcon SVG screen.getByRole("menuitemradio", { name: "Microphone 2", checked: true });
const mic1Item = screen.getByRole("menuitemradio", { screen.getByRole("menuitemradio", { name: "Microphone 1", checked: false });
name: "Microphone 2", });
});
expect(mic1Item.querySelectorAll("svg").length).toBe(2);
// The unselected item (mic1) only renders its IconOptions SVG test("lists speaker and microphone sections", async () => {
const mic2Item = screen.getByRole("menuitemradio", { const user = userEvent.setup();
name: "Microphone 1", 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={vi.fn()}
outputOptions={[
{
label: { type: "default", name: "Built-in Output" },
id: "default",
},
{ label: { type: "name", name: "Headset" }, id: "spk2" },
]}
selectedOutputOption="default"
onSelectOutput={vi.fn()}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
screen.getByRole("menuitemradio", {
name: "Default (Built-in Output)",
checked: true,
}); });
expect(mic2Item.querySelectorAll("svg").length).toBe(1); screen.getByRole("menuitemradio", { name: "Headset", checked: false });
screen.getByRole("menuitemradio", { name: "Microphone 1", checked: true });
screen.getByRole("menuitemradio", { name: "Microphone 2", checked: false });
});
test("calls the output select callback on speaker click", async () => {
const user = userEvent.setup();
const onSelectOutput = 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"
outputOptions={[
{ label: { type: "name", name: "Speakers" }, id: "spk1" },
{ label: { type: "name", name: "Headset" }, id: "spk2" },
]}
selectedOutputOption="spk1"
onSelectOutput={onSelectOutput}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
await user.click(screen.getByRole("menuitemradio", { name: "Headset" }));
expect(onSelectOutput).toHaveBeenCalledWith("spk2");
});
test("shows a single device entry disabled", 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" },
]}
selectedOption="mic1"
onSelect={onSelect}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
// Shown rather than hidden, so the menu keeps its shape, but not choosable.
const only = screen.getByRole("menuitemradio", { name: "Microphone 1" });
expect(only).toHaveAttribute("aria-disabled", "true");
});
test("shows the speaker section disabled when no output can be chosen", async () => {
const user = userEvent.setup();
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={vi.fn()}
outputOptions={[
{ label: { type: "name", name: "Speakers" }, id: "spk1" },
{ label: { type: "name", name: "Headset" }, id: "spk2" },
]}
selectedOutputOption="spk1"
// No callback: nothing can be picked here.
onSelectOutput={undefined}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
expect(
screen.getByRole("menuitemradio", { name: "Speakers" }),
).toHaveAttribute("aria-disabled", "true");
expect(
screen.getByRole("menuitemradio", { name: "Headset" }),
).toHaveAttribute("aria-disabled", "true");
// The microphone section is unaffected.
expect(
screen.getByRole("menuitemradio", { name: "Microphone 2" }),
).not.toHaveAttribute("aria-disabled", "true");
}); });
}); });
+25 -50
View File
@@ -5,29 +5,20 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE in the repository root for full details. Please see LICENSE in the repository root for full details.
*/ */
import { import { useState, type FC, useEffect, type ReactElement } from "react";
type ComponentType,
useState,
type FC,
useEffect,
type ReactElement,
} from "react";
import { import {
Button, Button,
Menu, Menu,
MenuItem, MenuItem,
MenuTitle, MenuTitle,
RadioInput,
Separator, Separator,
ToggleMenuItem, ToggleMenuItem,
} from "@vector-im/compound-web"; } from "@vector-im/compound-web";
import { import {
CheckIcon,
ChevronUpIcon, ChevronUpIcon,
ChevronDownIcon, ChevronDownIcon,
MicOnIcon,
SpinnerIcon, SpinnerIcon,
VideoCallIcon,
VolumeOnIcon,
} from "@vector-im/compound-design-tokens/assets/web/icons"; } from "@vector-im/compound-design-tokens/assets/web/icons";
import classNames from "classnames"; import classNames from "classnames";
import { useTranslation } from "react-i18next"; import { useTranslation } from "react-i18next";
@@ -48,8 +39,7 @@ export interface MenuOptions {
export interface MediaMuteAndSwitchButtonProps { export interface MediaMuteAndSwitchButtonProps {
/** /**
* The accessible name of the menu. Defaults to a translated name for the * The accessible name of the menu. Defaults to a translated name for the
* media kind; the menu's own title is not shown, since each section carries * media kind. Never shown: each section carries its own heading.
* its own heading.
*/ */
title?: string; title?: string;
/** If the Mute button is enabled */ /** If the Mute button is enabled */
@@ -71,8 +61,8 @@ export interface MediaMuteAndSwitchButtonProps {
/** The output option currently rendered as selected */ /** The output option currently rendered as selected */
selectedOutputOption?: string; selectedOutputOption?: string;
/** /**
* Called when an output device is picked. Undefined means the platform does * Called when an output device is picked. Undefined means no output can be
* not permit choosing an output, and the section renders disabled. * chosen here, and the section renders disabled.
*/ */
onSelectOutput?: (id: string) => void; onSelectOutput?: (id: string) => void;
videoBlurToggleClick?: () => void; videoBlurToggleClick?: () => void;
@@ -155,20 +145,17 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
break; break;
} }
let IconOptions: ComponentType<React.SVGAttributes<SVGElement>> | undefined;
let optionsButtonLabel: string; let optionsButtonLabel: string;
let defaultMenuTitle: string; let defaultMenuTitle: string;
let numberedLabel: (number: number) => string; let numberedLabel: (number: number) => string;
switch (iconsAndLabels) { switch (iconsAndLabels) {
case "video": case "video":
IconOptions = VideoCallIcon;
optionsButtonLabel = t("settings.devices.camera"); optionsButtonLabel = t("settings.devices.camera");
defaultMenuTitle = t("settings.devices.camera_source"); defaultMenuTitle = t("settings.devices.camera_source");
numberedLabel = (n): string => numberedLabel = (n): string =>
t("settings.devices.camera_numbered", { n }); t("settings.devices.camera_numbered", { n });
break; break;
case "audio": case "audio":
IconOptions = MicOnIcon;
optionsButtonLabel = t("settings.devices.microphone"); optionsButtonLabel = t("settings.devices.microphone");
defaultMenuTitle = t("settings.devices.mic_source"); defaultMenuTitle = t("settings.devices.mic_source");
numberedLabel = (n): string => numberedLabel = (n): string =>
@@ -176,7 +163,8 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
break; break;
} }
const labelToText = ( /** The text shown for a device, whichever kind of label it carries. */
const labelText = (
label: MenuOptions["label"], label: MenuOptions["label"],
numbered: (n: number) => string, numbered: (n: number) => string,
): string => { ): string => {
@@ -201,27 +189,30 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
selected: string | undefined, selected: string | undefined,
select: ((id: string) => void) | undefined, select: ((id: string) => void) | undefined,
numbered: (n: number) => string, numbered: (n: number) => string,
Icon: ComponentType<React.SVGAttributes<SVGElement>> | undefined,
): ReactElement[] => { ): ReactElement[] => {
const list = items ?? []; const list = items ?? [];
// Shown but not choosable when the platform will not switch this kind of // Shown but not choosable when nothing can be picked here, or when there is
// device, or when there is only one of them. The entry stays visible so the // only one device. The entry stays visible so the menu keeps the same shape
// menu keeps the same shape everywhere. // on every platform.
const disabled = select === undefined || list.length <= 1; const disabled = select === undefined || list.length <= 1;
return list.map(({ label, id }) => ( return list.map(({ label, id }) => (
<MenuItem <MenuItem
// A radio input inside a button is invalid, and the menu needs an
// element it can give menuitemradio semantics to.
as="div"
hideChevron hideChevron
disabled={disabled} disabled={disabled}
label={labelToText(label, numbered)} label={labelText(label, numbered)}
Icon={ Icon={
Icon && ( <RadioInput
<Icon // Decoration: aria-checked on the menu item is what conveys the
width={24} // selection, and Radix owns focus within the menu.
height={24} aria-hidden
className={styles.itemIcon} tabIndex={-1}
aria-hidden checked={selected === id}
/> disabled={disabled}
) readOnly
/>
} }
onSelect={(e) => { onSelect={(e) => {
e.preventDefault(); e.preventDefault();
@@ -233,13 +224,6 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
role="menuitemradio" role="menuitemradio"
aria-checked={selected === id} aria-checked={selected === id}
> >
{selected === id && (
<CheckIcon
width={24}
height={24}
aria-hidden // A label would be redundant to aria-checked above
/>
)}
{selected !== id && plannedSelection === id && ( {selected !== id && plannedSelection === id && (
<SpinnerIcon <SpinnerIcon
width={24} width={24}
@@ -252,8 +236,6 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
)); ));
}; };
const showOutputSection = iconsAndLabels === "audio" && outputOptions;
return ( return (
<div <div
className={classNames({ className={classNames({
@@ -285,7 +267,7 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
/> />
} }
> >
{showOutputSection && ( {iconsAndLabels === "audio" && outputOptions && (
<> <>
<MenuTitle title={t("settings.devices.speaker")} /> <MenuTitle title={t("settings.devices.speaker")} />
{deviceItems( {deviceItems(
@@ -293,19 +275,12 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
selectedOutputOption, selectedOutputOption,
onSelectOutput, onSelectOutput,
(n) => t("settings.devices.speaker_numbered", { n }), (n) => t("settings.devices.speaker_numbered", { n }),
VolumeOnIcon,
)} )}
<Separator /> <Separator />
</> </>
)} )}
<MenuTitle title={optionsButtonLabel} /> <MenuTitle title={optionsButtonLabel} />
{deviceItems( {deviceItems(options, selectedOption, onSelect, numberedLabel)}
options,
selectedOption,
onSelect,
numberedLabel,
IconOptions,
)}
{(toggles?.length ?? 0) > 0 && <hr />} {(toggles?.length ?? 0) > 0 && <hr />}
{toggles?.map((toggle) => ( {toggles?.map((toggle) => (
<ToggleMenuItem <ToggleMenuItem