diff --git a/FEATURES_SPEC/2026-09_Audio_Quick_Menu.md b/FEATURES_SPEC/2026-09_Audio_Quick_Menu.md index 1fc6459aa..33235a625 100644 --- a/FEATURES_SPEC/2026-09_Audio_Quick_Menu.md +++ b/FEATURES_SPEC/2026-09_Audio_Quick_Menu.md @@ -112,7 +112,10 @@ Test names are the anchor; tests are created with exactly these names. - check: `pnpm vitest run --project=unit -t "audio menu hints when microphone permission is denied"` - AC15 [D7] — When the selected microphone cannot be opened because another application holds it, the indicator renders in a greyed-out state. - - check: `pnpm vitest run --project=unit -t "level indicator greys out when the microphone is unavailable"` + - check: `pnpm vitest run --project=storybook -t "Microphone Unavailable"`, which reads + the greying a browser applies. The unit test + `-t "level indicator greys out when the microphone is unavailable"` covers the state + the stylesheet keys off, and cannot fail on the greying itself. - AC16 [FR-012] — Moving the sound-effects slider changes the level at which the next sound effect plays. - check: `pnpm vitest run --project=unit -t "sound effects volume from the menu applies to the next effect"` @@ -132,15 +135,17 @@ Test names are the anchor; tests are created with exactly these names. confirm the announced state changes with the signal. - AC22 [D11] — With more devices than fit on screen, the heading and the sound-effect slider stay visible while the device lists scroll. - - check: `pnpm vitest run --project=unit -t "audio menu keeps its title and volume slider out of the scrolling area"` + - check: `pnpm vitest run --project=storybook -t "With Many Devices"`, which measures the + layout in a browser with the device count fixed. The unit test + `-t "audio menu keeps its title and volume slider out of the scrolling area"` and the + e2e `-g "audio menu stays inside a short window"` cover the structure and the window + bound; neither can fail on layout alone. - AC23 [D12] — The level indicator stays with the microphone list and never sits among the output controls. - check: `pnpm vitest run --project=unit -t "level indicator stays with the microphone list rather than the speakers"` - AC24 [D13] — Moving through the menu by keyboard marks the current item with a border; moving over it with a pointer marks it with a background and no border. - - check: manual, open the menu and tab through the device rows, confirming a border marks - the focused row; then move the pointer across the rows, confirming a background appears - and no border does. + - check: `pnpm test:playwright --project=chromium --project=firefox -g "the focus border follows the keyboard and not the pointer"` - AC25 [SC-001] — Selecting an output from the menu during a call leaves the other participants visible throughout. - check: `pnpm test:playwright --project=chromium -g "audio menu leaves participants visible while switching output"` @@ -310,6 +315,33 @@ is what holds it there. - Added as AC28 and the product spec's edge case reworded to match, both with the owner's authorisation, since acceptance criteria and the product spec are human-owned. +### 2026-09-11 — finding: `:focus-visible` cannot express D13 (#4254) +- The menu moves focus onto whichever item the pointer is over, so the browser decides the + modality for a focus it did not see the user cause. Chromium calls every focus after any + key press keyboard-driven, so one Escape left the border following the mouse for the rest + of the session; Firefox never calls a programmatic focus keyboard-driven, so the border + never appeared there at all. Neither matches D13. +- The menu now tracks which way each item was reached — a key press on the trigger or on an + item, against pointer movement over one — and the border follows that. Verified in both + browsers: the border appears only after a key press and leaves again on the next pointer + move. AC24's manual check could now name a unit test; owner's call. + +### 2026-09-11 — acceptance checks that could not fail (#4254) +- Raised in review: AC22's check ran in jsdom, which lays nothing out, so it passed whatever + the layout did — the defect it exists for went unseen until a screenshot. Its check now + names the `CallFooter` "With Many Devices" story, which measures the geometry in a browser + with the device count fixed. The unit test and the short-window e2e stay as they are; the + criterion names the one that would fail. +- AC24 was a manual check, and the border it describes turned out to be wrong in both + browsers for different reasons. Its check now names an e2e test that reads the painted + outline as the pointer and the keyboard take turns. +- The same audit found AC15: its check asserted the attribute the stylesheet keys off, not + the greying, so losing the CSS rule would not have failed it. Re-anchored to the meter's + own story, which reads the applied opacity. +- Every edit to `## Acceptance criteria` was made with the owner's authorisation. The + remaining manual checks are AC6, which needs Safari, and AC21, which needs a screen + reader. + ## PRs - #4254 — draft, one commit per slice — AC1–AC27 (AC6, AC21, AC24 manual by the reviewer; diff --git a/playwright/audio-menu.spec.ts b/playwright/audio-menu.spec.ts index 1b5d43a7e..c0c40d7a2 100644 --- a/playwright/audio-menu.spec.ts +++ b/playwright/audio-menu.spec.ts @@ -172,6 +172,40 @@ test("audio menu stays inside a short window", async ({ browser }) => { .toBe(true); }); +test("the focus border follows the keyboard and not the pointer", async ({ + browser, +}) => { + const context = await browser.newContext({ reducedMotion: "reduce" }); + const page = await context.newPage(); + await page.goto("/"); + await SpaHelpers.createCall(page, "Focus", "Focus border", true); + await page.getByTestId("videoTile").first().waitFor(); + + const chevron = page.getByRole("button", { name: "Microphone" }); + const rows = page.getByRole("menu").getByRole("menuitemradio"); + const borderOfFocused = async (): Promise => + page.evaluate(() => { + const el = document.activeElement; + return el === null ? "none" : getComputedStyle(el).outlineStyle; + }); + + // Reached with the pointer: the hover background carries it, no border. + await chevron.click(); + await page.getByRole("menu").waitFor(); + await rows.nth(1).hover(); + expect(await borderOfFocused()).toBe("none"); + + // Reached with the keyboard: a border marks where the keyboard is. The menu + // moves focus to whatever the pointer is over, so the browser cannot tell + // these two apart on its own. + await page.keyboard.press("ArrowDown"); + expect(await borderOfFocused()).toBe("solid"); + + // And back, on the next movement of the pointer. + await rows.nth(0).hover(); + expect(await borderOfFocused()).toBe("none"); +}); + async function firstUncheckedIndex( rows: Locator, count: number, diff --git a/src/components/AudioLevelMeter.stories.tsx b/src/components/AudioLevelMeter.stories.tsx index 38f9f13c5..988394da5 100644 --- a/src/components/AudioLevelMeter.stories.tsx +++ b/src/components/AudioLevelMeter.stories.tsx @@ -47,11 +47,16 @@ export const Loud: Story = { args: { state: { type: "active", level: 0.95 } }, }; -export const Unavailable: Story = { +/** Named apart from the footer's own "Unavailable Media Devices" story. */ +export const MicrophoneUnavailable: Story = { args: { state: { type: "unavailable" } }, play: async ({ canvasElement }) => { const meter = within(canvasElement).getByRole("meter"); await expect(meter).toHaveAttribute("data-unavailable", "true"); + // The greying is the point, and it is done in CSS: an attribute alone + // would still be there with the rule gone. + const bar = meter.querySelector("span"); + await expect(Number(getComputedStyle(bar!).opacity)).toBeLessThan(1); }, }; diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index d073033dc..fb43886d2 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -113,14 +113,14 @@ Please see LICENSE in the repository root for full details. flex: 1; } -/* Radix moves DOM focus onto whichever item the pointer is over, so the +/* The menu moves 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. */ .menu [role^="menuitem"]:focus { outline: none; } -.menu [role^="menuitem"]:focus-visible { +.keyboardNav [role^="menuitem"]:focus { outline: var(--cpd-border-width-2) solid var(--cpd-color-border-focused); outline-offset: calc(-1 * var(--cpd-border-width-2)); } diff --git a/src/components/MediaMuteAndSwitchButton.test.tsx b/src/components/MediaMuteAndSwitchButton.test.tsx index 207b64621..4365654fe 100644 --- a/src/components/MediaMuteAndSwitchButton.test.tsx +++ b/src/components/MediaMuteAndSwitchButton.test.tsx @@ -555,6 +555,26 @@ describe("audio menu", () => { ); }); + test("the focus border marks keyboard use and never the pointer", async () => { + const user = await openAudioMenu(); + const menu = screen.getByRole("menu"); + const rows = screen.getAllByRole("menuitemradio"); + + // Opened and driven by pointer: the hover background alone. + await user.hover(rows[1]); + expect(menu.className).not.toMatch(/keyboardNav/); + + // One arrow key, and the border marks where the keyboard is. + await user.keyboard("[ArrowDown]"); + expect(menu.className).toMatch(/keyboardNav/); + + // Back to the pointer, and the border goes with it: the browser calls + // every focus after a key press keyboard-driven, so this cannot be left + // to :focus-visible. + await user.hover(rows[0]); + expect(menu.className).not.toMatch(/keyboardNav/); + }); + test("audio menu is fully operable from the keyboard", async () => { const user = userEvent.setup(); const onSelect = vi.fn(); diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 226c3d02b..bde2dec10 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -123,6 +123,13 @@ export const MediaMuteAndSwitchButton: FC = ({ }) => { const [plannedSelection, setPlannedSelection] = useState(null); const [menuOpen, setMenuOpen] = useState(false); + // Which of the two reached the current item, since `:focus-visible` cannot + // tell: the menu focuses whatever the pointer is over. + const [keyboardNav, setKeyboardNav] = useState(false); + const modality = { + onKeyDown: (): void => setKeyboardNav(true), + onPointerMove: (): void => setKeyboardNav(false), + }; const isBusy = busy ?? false; const { t } = useTranslation(); const devices = useMediaDevices(); @@ -226,6 +233,7 @@ export const MediaMuteAndSwitchButton: FC = ({ key={id} role="menuitemradio" aria-checked={selectedOption === id} + {...modality} > {selectedOption === id && ( = ({ {/* The mute button lives inside */} {button} = ({ kind={"tertiary"} size="lg" aria-label={optionsButtonLabel} + onKeyDown={() => setKeyboardNav(true)} + onPointerDown={() => setKeyboardNav(false)} /> } > @@ -295,7 +307,7 @@ export const MediaMuteAndSwitchButton: FC = ({ 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. */} -
+
{/* The capture is bound to the menu being open, so the microphone is only ever held while the user is looking at the level. */} @@ -310,6 +322,7 @@ export const MediaMuteAndSwitchButton: FC = ({ options={audioControls.outputOptions} selected={audioControls.selectedOutput} onSelect={audioControls.onSelectOutput} + modality={modality} />
) : ( @@ -321,12 +334,14 @@ export const MediaMuteAndSwitchButton: FC = ({ )} {(toggles?.length ?? 0) > 0 &&
} {toggles?.map((toggle) => ( { videoBlurToggleClick?.(); @@ -345,6 +360,11 @@ interface SpeakerSectionProps { options: OutputMenuOptions[]; selected: string | undefined; onSelect: (id: string) => void; + /** Reports whether a row was reached by pointer or by keyboard. */ + modality: { + onKeyDown: () => void; + onPointerMove: () => void; + }; } /** @@ -359,6 +379,7 @@ function SpeakerSection({ options, selected, onSelect, + modality, }: SpeakerSectionProps): JSX.Element { const { t } = useTranslation(); const labelText = (label: AudioOutputDeviceLabel): string => { @@ -412,6 +433,7 @@ function SpeakerSection({ }} role="menuitemradio" aria-checked={selected === id} + {...modality} > {selected === id && } @@ -423,12 +445,14 @@ function SpeakerSection({ interface SoundEffectVolumeProps { volume: number; onCommit: (volume: number) => void; + onPointerMove: () => void; } /** The sound-effect volume slider: the same stored value as in settings. */ function SoundEffectVolume({ volume, onCommit, + onPointerMove, }: SoundEffectVolumeProps): JSX.Element { const { t } = useTranslation(); const label = t("settings.audio_tab.effect_volume_label"); @@ -451,6 +475,7 @@ function SoundEffectVolume({ data-testid="sound_effect_volume" role="group" aria-label={label} + onPointerMove={onPointerMove} onKeyDown={(e) => { if (SLIDER_KEYS.has(e.key)) e.stopPropagation(); else keepTabInsideMenu(e);