Track the focus modality on the menu itself

- Was watched at the document, so a key pressed in another Element Call on the
  same page — or in the host's own page — lit this menu's ring. Element Call
  can be mounted more than once, and the menu is portalled out of the root.
- Watched on the menu instead, which is the element that holds focus.
- A callback ref, not an effect on `menuOpen`: that state is ours and the open
  menu is Radix's, and they do not commit together, so the effect could run
  before there was anything to attach to.
- Recorded on the menu rather than in state, so every item it can focus answers
  to it — the camera menu's blur toggle kept the browser's own ring otherwise,
  which follows the pointer.
- Nothing re-renders when the modality changes now, so the reset folds in with
  the listeners and the second effect on that state goes.
- Comments moved to the code they describe; three unrelated ones had stacked up
  on one declaration.

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));
 }

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);
+  },
+};

src/components/MediaMuteAndSwitchButton.tsx

@@ -139,39 +139,57 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
   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<HTMLElement>('[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<number>();
-  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<MediaMuteAndSwitchButtonProps> = ({
         <div
           // Transparent to assistive technology, so the menu still sees its
           // items as its own children.
+          ref={trackFocusModality}
           role="none"
           className={styles.deviceList}
-          data-focus-modality={focusModality}
           style={
             {
               "--device-list-max-height":