From 8fb367380fbd94090447d97a09a0b20263aa8fbc Mon Sep 17 00:00:00 2001 From: fkwp Date: Thu, 10 Sep 2026 14:44:37 +0200 Subject: [PATCH] Add the sound-effect volume to the audio menu and scroll the device lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The menu gains a third group with the sound-effect volume slider, reading and writing the same stored value as the slider in settings, so the next effect plays at the new level. The menu is bounded to the height Radix reports for it and only the device lists scroll; the heading and the slider stay in place. Every control is now reachable by keyboard alone. Radix swallows Tab inside its menus so that the arrow keys walk the items; the audio menu keeps Tab from it, letting the browser move focus from the device rows to the meter and on to the slider, while the menu's focus trap keeps that order inside the menu. The slider's own keys stop at the slider so the menu does not treat them as navigation. The meter keeps its live-region text on blur: removing it handed the focus trap an empty active element mid-Tab and pulled focus back into the menu. Spec: FEATURES_SPEC/2026-09_Audio_Quick_Menu.md, slice 4 — AC4, AC16, AC17, AC20, AC21 (manual), AC22, AC27. Co-Authored-By: Claude Fable 5.1 --- playwright/audio-menu.spec.ts | 82 +++++++++++ src/components/AudioLevelMeter.test.tsx | 8 +- src/components/AudioLevelMeter.tsx | 11 +- src/components/CallFooter.stories.tsx | 54 ++++++++ src/components/CallFooter.test.tsx | 59 +++++++- src/components/CallFooter.tsx | 13 +- src/components/CallFooterViewModel.test.ts | 18 ++- src/components/CallFooterViewModel.tsx | 22 ++- .../MediaMuteAndSwitchButton.module.css | 47 +++++++ .../MediaMuteAndSwitchButton.test.tsx | 88 +++++++++++- src/components/MediaMuteAndSwitchButton.tsx | 131 ++++++++++++++++-- 11 files changed, 499 insertions(+), 34 deletions(-) diff --git a/playwright/audio-menu.spec.ts b/playwright/audio-menu.spec.ts index 6c83fc1c6..1b5d43a7e 100644 --- a/playwright/audio-menu.spec.ts +++ b/playwright/audio-menu.spec.ts @@ -90,6 +90,88 @@ test("level indicator moves with microphone input", async ({ .toBeGreaterThan(0); }); +test("audio menu is keyboard operable in a real browser", async ({ + browser, + browserName, +}) => { + test.skip( + browserName === "firefox", + 'Firefox headless drives page.keyboard.press("Tab") unreliably, as reconnect.spec.ts records.', + ); + const context = await browser.newContext({ reducedMotion: "reduce" }); + const page = await context.newPage(); + await page.goto("/"); + await SpaHelpers.createCall(page, "Keys", "Keyboard menu", true); + await expect(page.getByTestId("videoTile")).toHaveCount(1); + + // Open from the chevron; the first device row takes focus. + await page.getByRole("button", { name: "Microphone" }).focus(); + await page.keyboard.press("Enter"); + const menu = page.getByRole("menu"); + await expect(menu).toBeVisible(); + const rows = menu.getByRole("menuitemradio"); + await expect(rows.first()).toBeFocused(); + + // Arrow keys walk the device rows, where the browser lists more than one. + if ((await rows.count()) > 1) { + await page.keyboard.press("ArrowDown"); + await expect(rows.nth(1)).toBeFocused(); + } + + // Tab reaches the meter and then the slider; arrows adjust the slider and + // leave the menu open. + await page.keyboard.press("Tab"); + await expect(menu.getByRole("meter")).toBeFocused(); + await page.keyboard.press("Tab"); + const slider = menu.getByRole("slider"); + await expect(slider).toBeFocused(); + const before = Number(await slider.getAttribute("aria-valuenow")); + await page.keyboard.press("ArrowRight"); + await expect + .poll(async () => Number(await slider.getAttribute("aria-valuenow"))) + .toBeGreaterThan(before); + await expect(menu).toBeVisible(); + + // Shift+Tab goes back; Escape closes. + await page.keyboard.press("Shift+Tab"); + await expect(menu.getByRole("meter")).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(menu).not.toBeVisible(); +}); + +test("audio menu stays inside a short window", async ({ browser }) => { + const context = await browser.newContext({ + reducedMotion: "reduce", + viewport: { width: 1280, height: 560 }, + }); + const page = await context.newPage(); + await page.goto("/"); + await SpaHelpers.createCall(page, "Devices", "Long lists", true); + await page.getByTestId("videoTile").first().waitFor(); + + await page.getByRole("button", { name: "Microphone" }).focus(); + await page.keyboard.press("Enter"); + const menu = page.getByRole("menu"); + await expect(menu).toBeVisible(); + + // However many devices the browser reports, the menu fits the window and + // the heading and the slider are on screen with it. How many devices that + // is differs between browsers, so the case where the lists actually + // overflow is covered by the CallFooter "With Many Devices" story, which + // fixes the device count. + await expect(menu).toBeInViewport({ ratio: 1 }); + await expect( + menu.getByRole("heading", { name: "Audio controls" }), + ).toBeInViewport({ ratio: 1 }); + await expect(menu.getByRole("slider")).toBeInViewport({ ratio: 1 }); + + // The menu itself never becomes the scroller: that would carry the heading + // out of view, which is what the scroll area exists to prevent. + await expect + .poll(async () => menu.evaluate((el) => el.scrollHeight <= el.clientHeight)) + .toBe(true); +}); + async function firstUncheckedIndex( rows: Locator, count: number, diff --git a/src/components/AudioLevelMeter.test.tsx b/src/components/AudioLevelMeter.test.tsx index ee3fde33a..954e92a86 100644 --- a/src/components/AudioLevelMeter.test.tsx +++ b/src/components/AudioLevelMeter.test.tsx @@ -50,9 +50,13 @@ describe("AudioLevelMeter", () => { // Nothing is announced until the user puts the meter in focus, so the // level does not talk over the rest of the menu. - expect(meter.textContent).not.toContain("Picking up sound"); + const announcer = meter.querySelector("[aria-live]"); + expect(announcer).toHaveAttribute("aria-live", "off"); await user.tab(); expect(meter).toHaveFocus(); - expect(meter.textContent).toContain("Picking up sound"); + expect(announcer).toHaveAttribute("aria-live", "polite"); + expect(announcer).toHaveTextContent("Picking up sound"); + await user.tab(); + expect(announcer).toHaveAttribute("aria-live", "off"); }); }); diff --git a/src/components/AudioLevelMeter.tsx b/src/components/AudioLevelMeter.tsx index 3a2607673..0bb67c0ee 100644 --- a/src/components/AudioLevelMeter.tsx +++ b/src/components/AudioLevelMeter.tsx @@ -88,10 +88,13 @@ export const AudioLevelMeter: FC = ({ state }) => { /> ))} - {/* Only announces while the meter holds focus, so the level does not - interrupt a screen reader reading the rest of the menu. */} - - {focused ? stateText : ""} + {/* Announces changes only while the meter holds focus, so the level does + not talk over a screen reader reading the rest of the menu. The text + stays in the DOM either way: removing it on blur would hand the + menu's focus trap an empty active element mid-Tab, and it would pull + focus back into the menu instead of letting it reach the slider. */} + + {stateText} ); diff --git a/src/components/CallFooter.stories.tsx b/src/components/CallFooter.stories.tsx index a0375d562..f16b079e5 100644 --- a/src/components/CallFooter.stories.tsx +++ b/src/components/CallFooter.stories.tsx @@ -104,6 +104,7 @@ const meta = { toggleVideo: fnArgType, hangup: fnArgType, selectAudioOutputOption: fnArgType, + setSoundEffectVolume: fnArgType, }, } satisfies Meta; @@ -146,6 +147,8 @@ export const Default: Story = { audioOutputOptions: [], selectedAudioOutput: undefined, selectAudioOutputOption: undefined, + soundEffectVolume: 0.5, + setSoundEffectVolume: undefined, }, parameters: { layout: "fullscreen", @@ -438,6 +441,7 @@ export const WithAudioMenu: Story = { ], selectedAudioOutput: "", selectAudioOutputOption: fn(), + setSoundEffectVolume: fn(), }, play: async ({ args, canvasElement }) => { const canvas = within(canvasElement); @@ -458,6 +462,9 @@ export const WithAudioMenu: Story = { await expect(args.selectAudioOutputOption).toHaveBeenCalledWith("2"); // The menu stays open after a selection. await expect(screen.getByRole("menu")).toBeInTheDocument(); + await expect( + menu.getByRole("slider", { name: /Sound effect volume/ }), + ).toBeInTheDocument(); }, }; @@ -482,3 +489,50 @@ export const WithSingleAudioOutput: Story = { ).toBe(null); }, }; + +/** + * More devices than fit on screen: the heading and the slider stay put while + * the device lists scroll between them. + */ +export const WithManyDevices: Story = { + ...WithAudioMenu, + args: { + ...WithAudioMenu.args, + audioOptions: Array.from({ length: 12 }, (_, i) => ({ + label: { type: "name", name: `Microphone ${i + 1} (USB Audio Device)` }, + id: `mic-${i}`, + })), + selectedAudio: "mic-0", + audioOutputOptions: Array.from({ length: 8 }, (_, i) => ({ + label: { type: "name", name: `Speaker ${i + 1} (DisplayPort)` }, + id: `out-${i}`, + })), + selectedAudioOutput: "out-0", + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Microphone" })); + const menu = await screen.findByRole("menu"); + const heading = within(menu).getByRole("heading", { + name: "Audio controls", + }); + const slider = within(menu).getByRole("slider"); + const scroll = within(menu).getByTestId("audio_menu_scroll"); + + // The menu slides in; measure once it has settled. + await Promise.all( + menu.getAnimations({ subtree: true }).map(async (a) => a.finished), + ); + + // The whole menu is on screen, so the heading and the slider are too. + const menuBox = menu.getBoundingClientRect(); + await expect(menuBox.top).toBeGreaterThanOrEqual(0); + await expect(menuBox.bottom).toBeLessThanOrEqual(window.innerHeight); + await expect(heading.getBoundingClientRect().top).toBeGreaterThanOrEqual(0); + await expect(slider.getBoundingClientRect().bottom).toBeLessThanOrEqual( + window.innerHeight, + ); + // Only the device lists scroll. + await expect(scroll.scrollHeight).toBeGreaterThan(scroll.clientHeight); + }, +}; diff --git a/src/components/CallFooter.test.tsx b/src/components/CallFooter.test.tsx index d7be700db..40199509f 100644 --- a/src/components/CallFooter.test.tsx +++ b/src/components/CallFooter.test.tsx @@ -5,12 +5,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { describe, expect, test, vi } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { BehaviorSubject } from "rxjs"; import { Root as Form, TooltipProvider } from "@vector-im/compound-web"; -import { type ReactNode } from "react"; +import { type JSX, type ReactNode } from "react"; import { CallFooter } from "./CallFooter"; import { DeviceSelection } from "../settings/DeviceSelection"; @@ -23,10 +23,54 @@ import { } from "../state/MediaDevices"; import { constant } from "../state/Behavior"; import { mockMediaDevices } from "../utils/test"; +import { + soundEffectVolume as soundEffectVolumeSetting, + useSetting, +} from "../settings/settings"; import { getBasicCallViewModelEnvironment } from "../utils/test-viewmodel"; import { alice, local } from "../utils/test-fixtures"; describe("CallFooter", () => { + afterEach(() => soundEffectVolumeSetting.setValue(0.5)); + + test("sound effects volume from the menu applies to the next effect", async () => { + const user = userEvent.setup(); + renderFooter(mediaDevicesWithOutputs()); + + await user.click(screen.getByRole("button", { name: "Microphone" })); + await screen.findByRole("menu"); + await user.keyboard("[ArrowDown]"); + await user.tab(); + await user.tab(); + await user.keyboard("[ArrowRight]"); + + // The committed value lands in the setting the sound-effect player reads, + // so the next effect plays at the new level. + expect(soundEffectVolumeSetting.value$.value).toBe(0.51); + }); + + test("sound effects volume is shared between menu and settings", async () => { + const user = userEvent.setup(); + renderFooter(mediaDevicesWithOutputs(), ); + + // Set from the menu, shown in settings. + await user.click(screen.getByRole("button", { name: "Microphone" })); + await screen.findByRole("menu"); + await user.keyboard("[ArrowDown]"); + await user.tab(); + await user.tab(); + await user.keyboard("[ArrowRight]"); + await user.keyboard("[Escape]"); + expect(screen.getByRole("status")).toHaveTextContent("51%"); + + // Set from settings, shown in the menu. + await user.click(screen.getByRole("button", { name: "Set to 20%" })); + await user.click(screen.getByRole("button", { name: "Microphone" })); + expect( + await screen.findByRole("slider", { name: /Sound effect volume/ }), + ).toHaveAttribute("aria-valuenow", "0.2"); + }); + test("audio menu is headed Audio controls", async () => { const user = userEvent.setup(); renderFooter(mediaDevicesWithOutputs()); @@ -101,6 +145,17 @@ describe("CallFooter", () => { ); } + /** Reads and writes the volume the way the settings dialog does. */ + function SettingsVolume(): JSX.Element { + const [volume, setVolume] = useSetting(soundEffectVolumeSetting); + return ( + <> + {Math.round(volume * 100)}% + + + ); + } + /** Two outputs whose selection is one shared value, as in MediaDevices. */ function mediaDevicesWithOutputs(): MediaDevices { const selected$ = new BehaviorSubject< diff --git a/src/components/CallFooter.tsx b/src/components/CallFooter.tsx index bfcee0317..8b394a34f 100644 --- a/src/components/CallFooter.tsx +++ b/src/components/CallFooter.tsx @@ -116,6 +116,9 @@ export interface FooterState { selectedAudioOutput: string | undefined; /** Also controls whether the microphone chevron opens the audio menu */ selectAudioOutputOption: ((deviceId: string) => void) | undefined; + soundEffectVolume: number; + /** Also controls whether the microphone chevron opens the audio menu */ + setSoundEffectVolume: ((volume: number) => void) | undefined; } export interface FooterProps { @@ -159,6 +162,8 @@ export const CallFooter: FC = ({ const audioOutputOptions = useBehavior(vm.audioOutputOptions$); const selectedAudioOutput = useBehavior(vm.selectedAudioOutput$); const selectAudioOutputOption = useBehavior(vm.selectAudioOutputOption$); + const soundEffectVolume = useBehavior(vm.soundEffectVolume$); + const setSoundEffectVolume = useBehavior(vm.setSoundEffectVolume$); const toggleBlur = useBehavior(vm.toggleBlur$); const videoBlurEnabled = useBehavior(vm.videoBlurEnabled$); const buttonSize = useBehavior(vm.buttonSize$); @@ -180,16 +185,18 @@ export const CallFooter: FC = ({ ); } - // The audio menu exists wherever an output can be selected. It names the - // output in use even when there is no microphone to list. + // The audio menu exists wherever its actions do. It names the output in use + // even when there is no microphone to list. const audioControls = - selectAudioOutputOption === undefined + selectAudioOutputOption === undefined || setSoundEffectVolume === undefined ? undefined : { outputOptions: audioOutputOptions ?? [], selectedOutput: selectedAudioOutput, onSelectOutput: selectAudioOutputOption, micDeviceId: selectedAudio, + soundEffectVolume: soundEffectVolume ?? 0, + onSoundEffectVolumeCommit: setSoundEffectVolume, }; if ((audioOptions?.length ?? 0) > 0 || audioControls !== undefined) { diff --git a/src/components/CallFooterViewModel.test.ts b/src/components/CallFooterViewModel.test.ts index 42ac5dbc8..e568eac3c 100644 --- a/src/components/CallFooterViewModel.test.ts +++ b/src/components/CallFooterViewModel.test.ts @@ -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 { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { BehaviorSubject } from "rxjs"; import { testScope, mockMuteStates, mockMediaDevices } from "../utils/test"; @@ -21,6 +21,7 @@ import { createCallFooterViewModel } from "./CallFooterViewModel"; import { HeaderStyle } from "../UrlParams"; import { type FooterSnapshot } from "./CallFooter"; import { type ViewModel } from "../state/ViewModel"; +import { soundEffectVolume } from "../settings/settings"; const platformMock = vi.hoisted(() => vi.fn(() => "desktop")); vi.mock("../Platform", () => ({ @@ -115,7 +116,9 @@ const twoOutputsMediaDevices = mockMediaDevices({ }); describe("createCallFooterViewModel", () => { - describe("audio output", () => { + describe("audio menu", () => { + afterEach(() => soundEffectVolume.setValue(0.5)); + function createVm( platform: string, layout: Layout, @@ -135,12 +138,23 @@ describe("createCallFooterViewModel", () => { const vm = createVm("ios", gridLayout); expect(vm.audioOutputOptions$.value).toEqual([]); expect(vm.selectAudioOutputOption$.value).toBeUndefined(); + expect(vm.setSoundEffectVolume$.value).toBeUndefined(); }); it("offers no audio menu when the layout is pip", () => { const vm = createVm("desktop", pipLayout); expect(vm.audioOutputOptions$.value).toEqual([]); expect(vm.selectAudioOutputOption$.value).toBeUndefined(); + expect(vm.setSoundEffectVolume$.value).toBeUndefined(); + }); + + it("reads and writes the sound-effect volume setting on desktop", () => { + const vm = createVm("desktop", gridLayout); + expect(vm.soundEffectVolume$.value).toBe(0.5); + + vm.setSoundEffectVolume$.value?.(0.2); + expect(soundEffectVolume.value$.value).toBe(0.2); + expect(vm.soundEffectVolume$.value).toBe(0.2); }); it("lists the outputs and the selection on desktop", () => { diff --git a/src/components/CallFooterViewModel.tsx b/src/components/CallFooterViewModel.tsx index c85a24b15..c1bfc6cfa 100644 --- a/src/components/CallFooterViewModel.tsx +++ b/src/components/CallFooterViewModel.tsx @@ -17,6 +17,7 @@ import { type MediaDevices } from "../state/MediaDevices"; import { backgroundBlur as backgroundBlurSettings, debugTileLayout as debugTileLayoutSetting, + soundEffectVolume as soundEffectVolumeSetting, } from "../settings/settings"; import { type Behavior, constant } from "../state/Behavior"; import type { ObservableScope } from "../state/ObservableScope"; @@ -57,10 +58,10 @@ function buildMuteBehaviors( } /** - * Shared helper: maps MediaDevices into every device behavior FooterSnapshot - * needs, for the switcher menus and the audio menu alike. The output list is - * empty wherever the browser cannot switch output; the menu then names the - * default rather than hiding the group. + * Shared helper: maps MediaDevices and the sound-effect setting into every + * device behavior FooterSnapshot needs, for the switcher menus and the audio + * menu alike. The output list is empty wherever the browser cannot switch + * output; the menu then names the default rather than hiding the group. */ function buildDeviceBehaviors( scope: ObservableScope, @@ -80,6 +81,8 @@ function buildDeviceBehaviors( | "audioOutputOptions$" | "selectedAudioOutput$" | "selectAudioOutputOption$" + | "soundEffectVolume$" + | "setSoundEffectVolume$" > { return { audioOptions$: scope.behavior( @@ -162,6 +165,17 @@ function buildDeviceBehaviors( ), ), ), + soundEffectVolume$: soundEffectVolumeSetting.value$, + setSoundEffectVolume$: scope.behavior( + disableSwitcher$.pipe( + map((disable) => + disable + ? undefined + : (volume: number): void => + soundEffectVolumeSetting.setValue(volume), + ), + ), + ), }; } diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index 2d2c39d67..d073033dc 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -66,6 +66,53 @@ Please see LICENSE in the repository root for full details. margin-inline: var(--cpd-border-width-1); } +/* Bound the whole menu to the space Radix reports for it, so that the scroll + area below can shrink while the heading and the slider, its flex siblings, + keep their size and stay on screen. The bound has to cover the menu's own + padding, or the padding lands outside it and carries the heading off the + top of the viewport; the 2x keeps a small gap to the viewport edge. */ +.menu { + box-sizing: border-box; + max-block-size: calc( + var(--radix-dropdown-menu-content-available-height) - var(--cpd-space-2x) + ); +} + +.scrollArea { + flex: 1 1 auto; + min-block-size: 0; + overflow-y: auto; + overscroll-behavior: contain; + scrollbar-gutter: stable; + /* The menu lays its children out as a flex column; the scroll area has to + repeat that for the children it now owns. */ + display: flex; + flex-direction: column; + gap: var(--cpd-space-1x); +} + +.volumeRow { + display: flex; + flex-direction: column; + gap: var(--cpd-space-2x); + padding: var(--cpd-space-3x) var(--cpd-space-4x); +} + +.volumeLabel { + font: var(--cpd-font-body-md-medium); + color: var(--cpd-color-text-primary); +} + +.volumeControl { + display: flex; + align-items: center; + gap: var(--cpd-space-3x); +} + +.volumeSlider { + flex: 1; +} + /* Radix moves DOM focus onto whichever item the pointer is over, so the browser's own focus ring shows up during mouse use. Keyboard navigation gets the border; the pointer gets the hover background and nothing else. */ diff --git a/src/components/MediaMuteAndSwitchButton.test.tsx b/src/components/MediaMuteAndSwitchButton.test.tsx index 29cdf1dec..5ed38271c 100644 --- a/src/components/MediaMuteAndSwitchButton.test.tsx +++ b/src/components/MediaMuteAndSwitchButton.test.tsx @@ -484,7 +484,7 @@ describe("audio menu", () => { expect(screen.getByRole("menu")).toBeInTheDocument(); }); - test("audio menu marks the active output below the microphones", async () => { + test("audio menu marks the active output", async () => { await openAudioMenu(); expect( @@ -493,8 +493,88 @@ describe("audio menu", () => { expect( screen.getByRole("menuitemradio", { name: "Headset" }), ).toHaveAttribute("aria-checked", "false"); - // One rule between the microphone group and the speaker group. - expect(screen.getByRole("menu").querySelectorAll("hr")).toHaveLength(1); + }); + + test("audio menu separates microphone speaker and sound effects groups", async () => { + await openAudioMenu(); + + // One rule after the microphone group, one after the speaker group. + expect(screen.getByRole("menu").querySelectorAll("hr")).toHaveLength(2); + expect(screen.getByTestId("mic_level_meter")).toBeInTheDocument(); + expect(screen.getByTestId("sound_effect_volume")).toBeInTheDocument(); + }); + + test("audio menu keeps its title and volume slider out of the scrolling area", async () => { + await openAudioMenu(); + + // Machines with many inputs and outputs produce a device list taller than + // the menu can be. Only that list scrolls; the heading above it and the + // sound-effect slider below it stay put. + const scroll = screen.getByTestId("audio_menu_scroll"); + expect(scroll).toContainElement( + screen.getByRole("menuitemradio", { name: "Headset Microphone" }), + ); + expect(scroll).toContainElement( + screen.getByRole("menuitemradio", { name: "Headset" }), + ); + expect(scroll).not.toContainElement( + screen.getByTestId("sound_effect_volume"), + ); + expect(scroll).not.toContainElement( + screen.getByRole("heading", { name: "Audio controls" }), + ); + }); + + test("audio menu is fully operable from the keyboard", async () => { + const user = userEvent.setup(); + const onSelect = vi.fn(); + const controls = audioControls(); + renderAudioMenu({ onSelect, audioControls: controls }); + + // Open from the chevron; the first device row takes focus. + await user.tab(); + await user.tab(); + expect(screen.getByRole("button", { name: "Microphone" })).toHaveFocus(); + await user.keyboard("[Enter]"); + await screen.findByRole("menu"); + expect( + screen.getByRole("menuitemradio", { name: "Built-in Microphone" }), + ).toHaveFocus(); + + // Arrow keys walk the microphone rows; Enter selects and keeps the menu. + await user.keyboard("[ArrowDown]"); + const headsetMic = screen.getByRole("menuitemradio", { + name: /Headset Microphone/, + }); + expect(headsetMic).toHaveFocus(); + await user.keyboard("[Enter]"); + expect(onSelect).toHaveBeenCalledWith("mic-2"); + expect(screen.getByRole("menu")).toBeInTheDocument(); + + // Tab reaches the meter below the microphones, then the slider; arrow + // keys adjust the slider without moving the menu's focus. + await user.tab(); + expect(screen.getByRole("meter")).toHaveFocus(); + await user.tab(); + const slider = screen.getByRole("slider", { name: /Sound effect volume/ }); + expect(slider).toHaveFocus(); + await user.keyboard("[ArrowRight]"); + expect(controls.onSoundEffectVolumeCommit).toHaveBeenCalledWith(0.51); + expect(slider).toHaveFocus(); + + // Shift+Tab walks back to the meter and the current row; from there the + // arrow keys carry on into the speaker rows. + await user.tab({ shift: true }); + expect(screen.getByRole("meter")).toHaveFocus(); + await user.tab({ shift: true }); + expect(headsetMic).toHaveFocus(); + await user.keyboard("[ArrowDown][ArrowDown]"); + expect( + screen.getByRole("menuitemradio", { name: "Headset" }), + ).toHaveFocus(); + await user.keyboard("[Enter]"); + expect(controls.onSelectOutput).toHaveBeenCalledWith("out-2"); + expect(screen.getByRole("menu")).toBeInTheDocument(); }); test("audio menu does not reselect the active output", async () => { @@ -620,6 +700,8 @@ describe("audio menu", () => { selectedOutput: "out-1", onSelectOutput: vi.fn(), micDeviceId: "mic-1", + soundEffectVolume: 0.5, + onSoundEffectVolumeCommit: vi.fn(), ...over, }; } diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 74fb96010..e72f2055f 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -11,6 +11,7 @@ import { type FC, useEffect, type JSX, + type KeyboardEvent, } from "react"; import { Button, @@ -39,6 +40,7 @@ import { import { useMediaDevices } from "../MediaDevicesContext"; import { AudioLevelMeter } from "./AudioLevelMeter"; import { useMicrophoneLevel } from "./useMicrophoneLevel"; +import { Slider } from "../Slider"; export interface MenuOptions { label: DeviceLabel; @@ -52,7 +54,8 @@ export interface OutputMenuOptions { /** * The controls that turn the microphone chevron menu into the audio menu: a - * speaker group below the microphone list. Absent for the camera menu. + * speaker group and the sound-effect volume below the microphone list. Absent + * for the camera menu. */ export interface AudioControls { /** @@ -64,6 +67,8 @@ export interface AudioControls { onSelectOutput: (id: string) => void; /** The microphone the level meter follows. */ micDeviceId: string | undefined; + soundEffectVolume: number; + onSoundEffectVolumeCommit: (volume: number) => void; } export interface MediaMuteAndSwitchButtonProps { @@ -92,6 +97,17 @@ export interface MediaMuteAndSwitchButtonProps { } const BLUR_ID = "blur"; +/** The keys a slider reads; inside the menu they must not move focus. */ +const SLIDER_KEYS = new Set([ + "ArrowLeft", + "ArrowRight", + "ArrowUp", + "ArrowDown", + "Home", + "End", + "PageUp", + "PageDown", +]); export const MediaMuteAndSwitchButton: FC = ({ title, @@ -269,18 +285,34 @@ export const MediaMuteAndSwitchButton: FC = ({ } > {audioControls ? ( + // Only the device lists scroll; the title above and the slider below + // stay put. Tab moves between the device rows, the meter and the + // slider; see keepTabInsideMenu. + // eslint-disable-next-line jsx-a11y/no-static-element-interactions
- {deviceItems} - {/* The meter reads the microphone, so it travels with the - microphone list rather than sitting among the output controls; - pinned to the foot of the list, it stays on screen for as long - as any microphone is. */} -
- +
+ {deviceItems} + {/* The meter reads the microphone, so it travels with the + microphone list rather than sitting among the output + controls; pinned to the foot of the scroll port, it stays on + screen for as long as any microphone is. */} +
+ +
+
+
) : ( deviceItems @@ -288,10 +320,9 @@ export const MediaMuteAndSwitchButton: FC = ({ {audioControls && ( <>
- )} @@ -390,3 +421,75 @@ function SpeakerSection({ ); } + +interface SoundEffectVolumeProps { + volume: number; + onCommit: (volume: number) => void; +} + +/** The sound-effect volume slider: the same stored value as in settings. */ +function SoundEffectVolume({ + volume, + onCommit, +}: SoundEffectVolumeProps): JSX.Element { + const { t } = useTranslation(); + const label = t("settings.audio_tab.effect_volume_label"); + // Tracked locally so dragging is smooth; only the committed value is + // stored. A change made in settings while the menu is open resets it. + const [raw, setRaw] = useState(volume); + const [committed, setCommitted] = useState(volume); + if (volume !== committed) { + setCommitted(volume); + setRaw(volume); + } + + return ( + // The menu treats the slider's keys as navigation between its items, which + // would otherwise stop the slider from ever receiving them. The handler has + // to sit on the wrapper because Slider takes no key handler of its own. + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions +
{ + if (SLIDER_KEYS.has(e.key)) e.stopPropagation(); + else keepTabInsideMenu(e); + }} + > + {label} +
+ + `${label}: ${Math.round(v * 100)}%`} + /> +
+
+ ); +} + +/** + * Lets Tab move focus between the device rows, the level meter and the + * sound-effect slider. The menu swallows Tab so that its items are walked + * with the arrow keys; kept from it, the browser's own focus order takes over, + * and the menu's focus trap keeps that order inside the menu. + */ +function keepTabInsideMenu(e: KeyboardEvent): void { + if (e.key === "Tab") e.stopPropagation(); +}