diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index 9dda265e3..8c0f38609 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -109,14 +109,19 @@ Please see LICENSE in the repository root for full details. /* 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 { + it is suppressed and replaced by one that can. + + Every kind of item the menu can focus, not only the device rows: the camera + menu's blur toggle is a checkbox item and a child of the menu rather than of + the list, and left out it kept the browser's own ring and followed the + pointer with it. */ +.menu [role^="menuitem"]:focus, +.menu [role^="menuitem"]:focus-visible { outline: none; } /* Shown only when the keyboard is what moved the focus. */ -.deviceList[data-focus-modality="keyboard"] [role="menuitemradio"]:focus { +.menu[data-focus-modality="keyboard"] [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.stories.tsx b/src/components/MediaMuteAndSwitchButton.stories.tsx index 26eda4a9a..af78c418d 100644 --- a/src/components/MediaMuteAndSwitchButton.stories.tsx +++ b/src/components/MediaMuteAndSwitchButton.stories.tsx @@ -374,3 +374,89 @@ function overlapping(element: Element, overlays: Element[]): number { return Math.max(worst, shared); }, 0); } + +/** + * The focus ring belongs to the keyboard. Radix focuses whatever the pointer is + * over, so a ring that followed focus alone would trail the mouse. + * + * Asserted on the painted outline rather than on `data-focus-modality`: the + * attribute is what the stylesheet keys off, so asserting it would pass even + * with the rule deleted. + */ +export const KeyboardFocusRing: 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", + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Microphone" })); + const menu = within(document.body); + const first = await menu.findByRole("menuitemradio", { + name: "Microphone 1", + }); + + // Opened by pointer: no ring, even though Radix has moved focus. + await expect(outlineWidth(first)).toBe(0); + + await userEvent.keyboard("{ArrowDown}"); + const focused = document.activeElement as HTMLElement; + await expect(focused).toHaveRole("menuitemradio"); + await expect(outlineWidth(focused)).toBeGreaterThan(0); + + // And the pointer takes it away again. + await userEvent.hover(first); + await expect(outlineWidth(document.activeElement as HTMLElement)).toBe(0); + }, +}; + +/** The painted outline width, in pixels, however the stylesheet spells it. */ +function outlineWidth(element: HTMLElement): number { + const { outlineStyle, outlineWidth } = getComputedStyle(element); + if (outlineStyle === "none") return 0; + return Number.parseFloat(outlineWidth) || 0; +} + +/** + * The camera menu's blur toggle, which the keyboard reaches after the cameras. + * + * It is a checkbox item and a child of the menu rather than of the device list, + * so a focus ring hung on the list alone left it with the browser's own — + * which follows the pointer, and is what the ring exists to replace. + */ +export const FocusRingCoversTheBlurToggle: Story = { + args: { + ...VideoUnmute.args, + iconsAndLabels: "video", + videoBlurEnabled: false, + videoBlurToggleClick: fn(), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Camera" })); + const toggle = await within(document.body).findByRole("menuitemcheckbox", { + name: /Blur background/, + }); + + // Arrowed down past the cameras to the toggle, which is the last thing in + // the menu. + for (let i = 0; i < 6 && document.activeElement !== toggle; i++) + await userEvent.keyboard("{ArrowDown}"); + await expect(document.activeElement).toBe(toggle); + // The same ring the device rows get. + await expect(outlineWidth(toggle)).toBeGreaterThan(0); + + // And the pointer takes it away again, with the toggle still focused — so + // there is something to light up and it is not lit. + await userEvent.hover(toggle); + await expect(document.activeElement).toBe(toggle); + await expect(outlineWidth(toggle)).toBe(0); + }, +}; diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 44111c194..a01a879bf 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -139,39 +139,57 @@ export const MediaMuteAndSwitchButton: FC = ({ const isBusy = busy ?? false; const { t } = useTranslation(); const devices = useMediaDevices(); - // Only while the menu is open, so nothing holds a second capture of the - // microphone for the length of a call. - // The menu is portalled outside the call root, so nothing in the stylesheets - // 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 - // 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", + /** + * Tracks which modality moved the focus, for as long as the list is mounted. + * + * A ref rather than an effect on `menuOpen`: that state is ours, the open + * menu is Radix's, and the two do not commit together — an effect keyed on + * ours can run before Radix has mounted the content, with nothing to attach + * to. The list existing is the honest signal that the menu is open. + * + * Recorded on the menu rather than held in state, because every item the menu + * can focus has to answer to it — the device rows and the camera menu's blur + * toggle, which is the menu's child and not the list's — and because which + * modality someone is using changes nothing that has to be rendered again. + */ + const trackFocusModality = useCallback( + (list: HTMLDivElement | null): (() => void) | undefined => { + // Watched on the menu, not on the document. Element Call can be mounted + // more than once in a host's page, and the menu is portalled out of the + // call root, so a document listener would also answer for a key pressed + // in the other instance, or in the host's own page. The menu rather than + // the list, because the first arrow key arrives while the menu itself + // holds focus, above anything we render. + const menu = list?.closest('[role="menu"]'); + if (menu === null || menu === undefined) return; + // Each opening starts over: the modality belongs to whoever is using this + // menu now, not to whoever last used it. + const record = (modality: "keyboard" | "pointer"): void => { + menu.dataset.focusModality = modality; + }; + record("pointer"); + const usedKeyboard = (): void => record("keyboard"); + const usedPointer = (): void => record("pointer"); + menu.addEventListener("keydown", usedKeyboard, true); + menu.addEventListener("pointermove", usedPointer, true); + return (): void => { + menu.removeEventListener("keydown", usedKeyboard, true); + menu.removeEventListener("pointermove", usedPointer, true); + }; + }, + [], ); - 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]); + + // The menu is portalled outside the call root, so nothing in the stylesheets + // 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. const rootElement = useRootElement(); const [listMaxHeight, setListMaxHeight] = useState(); - useEffect(() => { - if (menuOpen) setFocusModality("pointer"); - }, [menuOpen]); useEffect(() => { if (!menuOpen) return; // Followed rather than measured once: a host can resize the space Element @@ -440,9 +458,9 @@ export const MediaMuteAndSwitchButton: FC = ({