Show the focus ring for the keyboard alone, and fix the radio styling

- Show the focus ring only when the keyboard moved the focus. Radix focuses
  whatever the pointer is over, so the browser cannot answer that question
  here: Chromium treats every focus after a key press as keyboard-driven,
  Firefox treats no programmatic focus as keyboard-driven. The menu records
  which modality arrived, watched at the document while it is open, since the
  first arrow key lands on the menu itself rather than on anything we render.
- Suppress the browser's own ring on menu items, so there is one answer to
  that question rather than two.
- Drop readOnly from the device radios. Compound paints a read-only control
  muted, and that rule comes after the checked rule, so it overrode the accent
  fill marking the selection and the menu stopped matching settings.
- Make the decorative radio inert rather than aria-hidden. A negative tabindex
  inside an interactive control stays reachable to assistive technology even
  when hidden, which axe rejects.
- Give each section a labelled group and mark its heading decorative. A menu
  may contain only items, separators and groups, and the headings were direct
  children of it. The list wrappers are role="none" for the same reason.
- Run axe over the open menu, which is what found both of the above.

Spec: FEATURES_SPEC/2026-09_Quick_Audio_Menu.md — AC23, AC24, AC25
This commit is contained in:
fkwp
2026-09-16 17:44:55 +02:00
parent a76ef6215b
commit 0802649375
4 changed files with 262 additions and 36 deletions
@@ -83,3 +83,17 @@ Please see LICENSE in the repository root for full details.
margin-inline: var(--cpd-border-width-1); margin-inline: var(--cpd-border-width-1);
margin-block-end: var(--cpd-border-width-1); margin-block-end: var(--cpd-border-width-1);
} }
/* Radix focuses whatever the pointer is over, so the browser's own ring marks
the item under the mouse. It cannot distinguish the two modalities here, so
it is suppressed and replaced by one that can. */
.deviceList [role="menuitemradio"]:focus,
.deviceList [role="menuitemradio"]:focus-visible {
outline: none;
}
/* Shown only when the keyboard is what moved the focus. */
.deviceList[data-focus-modality="keyboard"] [role="menuitemradio"]:focus {
outline: var(--cpd-border-width-2) solid var(--cpd-color-border-focused);
outline-offset: calc(-1 * var(--cpd-border-width-2));
}
@@ -6,6 +6,7 @@ Please see LICENSE in the repository root for full details.
*/ */
import { describe, expect, test, vi } from "vitest"; import { describe, expect, test, vi } from "vitest";
import { axe } from "vitest-axe";
import { act, render, screen, type RenderResult } from "@testing-library/react"; import { act, render, screen, type RenderResult } from "@testing-library/react";
import userEvent from "@testing-library/user-event"; import userEvent from "@testing-library/user-event";
import { type JSX, useState, type ReactNode } from "react"; import { type JSX, useState, type ReactNode } from "react";
@@ -433,6 +434,98 @@ describe("MediaMuteAndSwitchButton", () => {
).toBeInTheDocument(); ).toBeInTheDocument();
}); });
test("marks focus as keyboard-driven only when the keyboard moved it", 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()}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
const list = screen
.getByRole("menuitemradio", { name: "Microphone 1" })
.closest("[data-focus-modality]");
// The menu focuses whatever the pointer is over, so focus alone says
// nothing about how someone is navigating.
expect(list).toHaveAttribute("data-focus-modality", "pointer");
await user.keyboard("{ArrowDown}");
expect(list).toHaveAttribute("data-focus-modality", "keyboard");
await user.pointer({
target: screen.getByRole("menuitemradio", { name: "Microphone 2" }),
coords: { clientX: 10, clientY: 10 },
});
expect(list).toHaveAttribute("data-focus-modality", "pointer");
});
test("marks the selected device with the accent fill", 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="mic2"
onSelect={vi.fn()}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
const selected = screen
.getByRole("menuitemradio", { name: "Microphone 2" })
.querySelector("input[type=radio]");
expect(selected).toBeChecked();
// A read-only control is painted muted, which loses the accent fill that
// marks the selection and makes the menu differ from settings.
expect(selected).not.toHaveAttribute("readonly");
});
test("the open menu has no accessibility violations", 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"
onSelectOutput={vi.fn()}
/>,
);
await user.click(getByRole("button", { name: "Microphone" }));
// Includes the menu's own structure: wrappers between the menu and its
// items break the relationship the roles describe.
const menu = document.querySelector('[role="menu"]');
expect(await axe(menu as HTMLElement)).toHaveNoViolations();
});
test("lists speaker and microphone sections", async () => { test("lists speaker and microphone sections", async () => {
const user = userEvent.setup(); const user = userEvent.setup();
const { getByRole } = renderComponent( const { getByRole } = renderComponent(
+80 -33
View File
@@ -117,8 +117,33 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
// can size it against the call. Measure the call area rather than the window, // can size it against the call. Measure the call area rather than the window,
// or the menu is wrong wherever Element Call is not the whole page. Measured // or the menu is wrong wherever Element Call is not the whole page. Measured
// when the menu opens: it is short-lived enough not to need watching. // when the menu opens: it is short-lived enough not to need watching.
// Radix focuses whatever the pointer is over, so the browser's own
// :focus-visible cannot tell us whether a person is navigating by keyboard:
// Chromium answers yes to everything after any key press, Firefox answers no
// to programmatic focus. Track it ourselves and let the styling follow.
const [focusModality, setFocusModality] = useState<"keyboard" | "pointer">(
"pointer",
);
useEffect(() => {
if (!menuOpen) return;
// Watched at the document, and only while the menu is open. Which modality
// someone is using is not a property of any one element: the first arrow
// key arrives while the menu itself holds focus, above anything we render,
// and Radix moves focus around as the pointer travels.
const usedKeyboard = (): void => setFocusModality("keyboard");
const usedPointer = (): void => setFocusModality("pointer");
document.addEventListener("keydown", usedKeyboard, true);
document.addEventListener("pointermove", usedPointer, true);
return (): void => {
document.removeEventListener("keydown", usedKeyboard, true);
document.removeEventListener("pointermove", usedPointer, true);
};
}, [menuOpen]);
const rootElement = useRootElement(); const rootElement = useRootElement();
const [listMaxHeight, setListMaxHeight] = useState<number>(); const [listMaxHeight, setListMaxHeight] = useState<number>();
useEffect(() => {
if (menuOpen) setFocusModality("pointer");
}, [menuOpen]);
useEffect(() => { useEffect(() => {
if (menuOpen) if (menuOpen)
setListMaxHeight( setListMaxHeight(
@@ -247,15 +272,20 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
disabled={disabled} disabled={disabled}
label={labelText(label, numbered)} label={labelText(label, numbered)}
Icon={ Icon={
<RadioInput // Inert, not aria-hidden: a form control inside a menu item must be
// Decoration: aria-checked on the menu item is what conveys the // out of the focus order and out of the accessibility tree, and
// selection, and Radix owns focus within the menu. // aria-hidden alone leaves it focusable. The item's aria-checked is
aria-hidden // what conveys the selection.
tabIndex={-1} <span inert>
checked={selected === id} <RadioInput
disabled={disabled} checked={selected === id}
readOnly disabled={disabled}
/> // Not readOnly: that styles the control as muted, losing the
// accent fill that marks the selection. The menu item owns the
// interaction, so the change handler has nothing to do.
onChange={(): void => {}}
/>
</span>
} }
onSelect={(e) => { onSelect={(e) => {
e.preventDefault(); e.preventDefault();
@@ -314,7 +344,11 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
} }
> >
<div <div
// Transparent to assistive technology, so the menu still sees its
// items as its own children.
role="none"
className={styles.deviceList} className={styles.deviceList}
data-focus-modality={focusModality}
style={ style={
{ {
"--device-list-max-height": "--device-list-max-height":
@@ -324,34 +358,47 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
> >
{iconsAndLabels === "audio" && outputOptions && ( {iconsAndLabels === "audio" && outputOptions && (
<> <>
<MenuTitle title={t("settings.devices.speaker")} /> {/* A menu may only contain items, separators and groups, so each
{deviceItems( heading belongs to a group rather than sitting beside the
"output", items it names. */}
outputOptions, <div role="group" aria-label={t("settings.devices.speaker")}>
selectedOutputOption, {/* The heading is decoration: the group carries the name, and
onSelectOutput, a menu may only contain items, separators and groups. */}
(n) => t("settings.devices.speaker_numbered", { n }), <div aria-hidden>
)} <MenuTitle title={t("settings.devices.speaker")} />
</div>
{deviceItems(
"output",
outputOptions,
selectedOutputOption,
onSelectOutput,
(n) => t("settings.devices.speaker_numbered", { n }),
)}
</div>
<Separator /> <Separator />
</> </>
)} )}
<MenuTitle title={optionsButtonLabel} /> <div role="group" aria-label={optionsButtonLabel}>
{/* The heading sits outside, so the meter can never ride up over it: <div aria-hidden>
<MenuTitle title={optionsButtonLabel} />
</div>
{/* The heading sits outside, so the meter can never ride up over it:
sticky only holds while this block is in view. */} sticky only holds while this block is in view. */}
<div> <div role="none">
{deviceItems( {deviceItems(
"input", "input",
options, options,
selectedOption, selectedOption,
onSelect, onSelect,
numberedLabel, numberedLabel,
)} )}
{iconsAndLabels === "audio" && ( {iconsAndLabels === "audio" && (
<MicrophoneLevelMeter <MicrophoneLevelMeter
state={microphoneState} state={microphoneState}
className={styles.stickyMeter} className={styles.stickyMeter}
/> />
)} )}
</div>
</div> </div>
</div> </div>
{(toggles?.length ?? 0) > 0 && <hr />} {(toggles?.length ?? 0) > 0 && <hr />}
+75 -3
View File
@@ -5,8 +5,10 @@ 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 { describe, expect, it, vi } from "vitest"; import { afterEach, describe, expect, it, vi } from "vitest";
import { render } from "@testing-library/react"; import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { of } from "rxjs";
import { LeaveToHomeProvider } from "../LeaveToHomeContext"; import { LeaveToHomeProvider } from "../LeaveToHomeContext";
import { TooltipProvider } from "@vector-im/compound-web"; import { TooltipProvider } from "@vector-im/compound-web";
import { type MatrixClient } from "matrix-js-sdk"; import { type MatrixClient } from "matrix-js-sdk";
@@ -19,6 +21,7 @@ import {
import { LobbyView } from "./LobbyView"; import { LobbyView } from "./LobbyView";
import { E2eeType } from "../e2ee/e2eeType"; import { E2eeType } from "../e2ee/e2eeType";
import { mockMediaDevices, mockMuteStates } from "../utils/test"; import { mockMediaDevices, mockMuteStates } from "../utils/test";
import { type MediaDevices } from "../state/MediaDevices";
import { MediaDevicesContext } from "../MediaDevicesContext"; import { MediaDevicesContext } from "../MediaDevicesContext";
import { type ProcessorState } from "../livekit/TrackProcessorContext"; import { type ProcessorState } from "../livekit/TrackProcessorContext";
import { type EncryptionSystem } from "../e2ee/sharedKeyManagement"; import { type EncryptionSystem } from "../e2ee/sharedKeyManagement";
@@ -77,9 +80,10 @@ function renderLobbyView(
props: Partial<Parameters<typeof LobbyView>[0]> = {}, props: Partial<Parameters<typeof LobbyView>[0]> = {},
withAppBar = false, withAppBar = false,
platform = "android", platform = "android",
devices: Partial<MediaDevices> = {},
): ReturnType<typeof render> { ): ReturnType<typeof render> {
platformMock.mockReturnValue(platform); platformMock.mockReturnValue(platform);
const mediaDevices = mockMediaDevices({}); const mediaDevices = mockMediaDevices(devices);
const muteStates = mockMuteStates(); const muteStates = mockMuteStates();
const hideHeader = withAppBar ? true : false; const hideHeader = withAppBar ? true : false;
const lobbyView = ( const lobbyView = (
@@ -178,3 +182,71 @@ describe("LobbyView", () => {
expect(await axe(container)).toHaveNoViolations(); expect(await axe(container)).toHaveNoViolations();
}); });
}); });
describe("LobbyView microphone level", () => {
const realMediaDevices = Object.getOwnPropertyDescriptor(
navigator,
"mediaDevices",
);
afterEach(() => {
vi.unstubAllGlobals();
// Put navigator back, or every later test in the run inherits the stub.
if (realMediaDevices === undefined) {
Reflect.deleteProperty(navigator, "mediaDevices");
} else {
Object.defineProperty(navigator, "mediaDevices", realMediaDevices);
}
});
/** Just enough of the Web Audio and capture APIs for the meter to run. */
function stubAudioCapture(): void {
vi.stubGlobal(
"AudioContext",
class {
public readonly state = "running";
public createAnalyser(): object {
return {
fftSize: 1024,
getByteTimeDomainData: (): void => {},
};
}
public createMediaStreamSource(): object {
return { connect: (): void => {} };
}
public close(): void {}
},
);
// Only this property: replacing navigator wholesale drops the getters on
// its prototype, such as userAgent.
Object.defineProperty(navigator, "mediaDevices", {
configurable: true,
value: {
getUserMedia: vi.fn().mockResolvedValue({ getTracks: () => [] }),
},
});
}
it("shows the microphone level meter", async () => {
stubAudioCapture();
const user = userEvent.setup();
const { getByRole } = renderLobbyView({}, false, "desktop", {
requestDeviceNames: (): void => {},
audioInput: {
available$: of(
new Map([["mic1", { type: "name", name: "Microphone 1" }]]),
),
selected$: of({ id: "mic1" }),
select: (): void => {},
},
} as unknown as Partial<MediaDevices>);
// The meter lives with the microphone picker, which the pre-join screen
// reaches through the same chevron as a call in progress.
await user.click(getByRole("button", { name: "Microphone" }));
expect(
await screen.findByRole("meter", { name: "Microphone level" }),
).toBeInTheDocument();
});
});