Make WinampButton keyboard and screen reader operable (#1365)

When a WinampButton has an onClick handler, render it with role="button" and
tabIndex=0, and activate it with Enter or Space. WinampButtons used as plain
containers (no onClick) are unchanged.

ClickedDiv now also sets its "clicked" state on click, since keyboard
activation does not fire pointerdown.

The Milkdrop window binds Space to "next preset" via a native listener that
runs before React's, so ignore Space there when it targets a button.

packages/webamp/js/components/ClickedDiv.tsx

@@ -21,11 +21,22 @@ function ClickedDiv(props: Props) {
       props.onPointerDown(e);
     }
   }
+  const { onClick } = props;
+  // Keyboard activation does not fire `pointerdown`, so mark the button as
+  // clicked here too.
+  const handleClick =
+    onClick == null
+      ? undefined
+      : (e: React.MouseEvent<HTMLDivElement>) => {
+          setClicked(true);
+          onClick(e);
+        };
   return (
     <WinampButton
       {...props}
       className={classnames(props.className, { clicked })}
       onPointerDown={handlePointerDown}
+      onClick={handleClick}
     />
   );
 }

packages/webamp/js/components/MilkdropWindow/index.tsx

@@ -42,6 +42,13 @@ function useKeyHandler() {
     (e: KeyboardEvent) => {
       switch (e.keyCode) {
         case 32: // spacebar
+          // Let Space activate focused buttons, like the close button.
+          if (
+            e.target instanceof Element &&
+            e.target.getAttribute("role") === "button"
+          ) {
+            break;
+          }
           selectNextPreset();
           break;
         case 8: // backspace

packages/webamp/js/components/WinampButton.test.tsx

@@ -0,0 +1,110 @@
+// @types/react is still on v18, which does not declare React 19's `act`.
+// @ts-ignore
+import { act } from "react";
+import { createRoot, Root } from "react-dom/client";
+import WinampButton from "./WinampButton";
+import ClickedDiv from "./ClickedDiv";
+
+// Tell React that this environment supports `act`.
+// @ts-ignore
+globalThis.IS_REACT_ACT_ENVIRONMENT = true;
+
+let container: HTMLDivElement;
+let root: Root;
+
+beforeEach(() => {
+  container = document.createElement("div");
+  document.body.appendChild(container);
+  root = createRoot(container);
+});
+
+afterEach(() => {
+  act(() => root.unmount());
+  container.remove();
+});
+
+function render(element: JSX.Element): HTMLElement {
+  act(() => root.render(element));
+  return container.firstElementChild as HTMLElement;
+}
+
+function keyDown(element: HTMLElement, key: string, init?: KeyboardEventInit) {
+  const event = new KeyboardEvent("keydown", {
+    key,
+    bubbles: true,
+    cancelable: true,
+    ...init,
+  });
+  act(() => {
+    element.dispatchEvent(event);
+  });
+  return event;
+}
+
+describe("WinampButton", () => {
+  it("is a focusable button when it has an onClick handler", () => {
+    const button = render(<WinampButton title="Play" onClick={() => {}} />);
+    expect(button.getAttribute("role")).toBe("button");
+    expect(button.tabIndex).toBe(0);
+  });
+
+  it("is a plain div without an onClick handler", () => {
+    const div = render(<WinampButton className="band" />);
+    expect(div.hasAttribute("role")).toBe(false);
+    expect(div.hasAttribute("tabindex")).toBe(false);
+  });
+
+  it("is activated by Enter and Space", () => {
+    const onClick = vi.fn();
+    const button = render(<WinampButton title="Play" onClick={onClick} />);
+    expect(keyDown(button, "Enter").defaultPrevented).toBe(true);
+    expect(keyDown(button, " ").defaultPrevented).toBe(true);
+    expect(onClick).toHaveBeenCalledTimes(2);
+  });
+
+  it("ignores other keys and key repeat", () => {
+    const onClick = vi.fn();
+    const button = render(<WinampButton title="Play" onClick={onClick} />);
+    expect(keyDown(button, "a").defaultPrevented).toBe(false);
+    keyDown(button, "Enter", { repeat: true });
+    expect(onClick).not.toHaveBeenCalled();
+  });
+
+  it("ignores keys from focusable descendants", () => {
+    const onClick = vi.fn();
+    render(
+      <WinampButton title="Play" onClick={onClick}>
+        <input />
+      </WinampButton>
+    );
+    keyDown(container.querySelector("input")!, "Enter");
+    expect(onClick).not.toHaveBeenCalled();
+  });
+
+  it("still calls a provided onKeyDown handler", () => {
+    const onKeyDown = vi.fn();
+    const button = render(
+      <WinampButton title="Play" onClick={() => {}} onKeyDown={onKeyDown} />
+    );
+    keyDown(button, "a");
+    expect(onKeyDown).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe("ClickedDiv", () => {
+  it("is marked as clicked when activated from the keyboard", () => {
+    const onClick = vi.fn();
+    const button = render(
+      <ClickedDiv id="close" title="Close" onClick={onClick} />
+    );
+    expect(button.classList.contains("clicked")).toBe(false);
+    keyDown(button, "Enter");
+    expect(onClick).toHaveBeenCalledTimes(1);
+    expect(button.classList.contains("clicked")).toBe(true);
+  });
+
+  it("is not a button without an onClick handler", () => {
+    const div = render(<ClickedDiv id="option" title="Winamp Menu" />);
+    expect(div.hasAttribute("role")).toBe(false);
+  });
+});

packages/webamp/js/components/WinampButton.tsx

@@ -5,6 +5,7 @@ import {
   HTMLAttributes,
   useState,
   PointerEvent as ReactPointerEvent,
+  KeyboardEvent as ReactKeyboardEvent,
 } from "react";
 
 const ACTIVE_CLASSNAME = "winamp-active";
@@ -31,10 +32,15 @@ type Props = DetailedHTMLPropsAndMore;
  * > "down" state (e.g. for a mouse, between the time the mouse button is pressed
  * > and the time it is depressed; for a finger in a multitouch environment, while
  * > the finger is touching the display surface).
+ *
+ * If an `onClick` handler is provided, the `div` is also exposed as a button to
+ * keyboard and screen reader users: it can be focused with Tab and activated
+ * with Enter or Space.
  */
 export default function WinampButton({
   requireClicksOriginateLocally = true,
   onPointerDown: originalOnPointerDown,
+  onKeyDown: originalOnKeyDown,
   className,
   ...htmlProps
 }: Props): JSX.Element {
@@ -87,8 +93,29 @@ export default function WinampButton({
     }
   };
 
+  const onKeyDown = (e: ReactKeyboardEvent<HTMLDivElement>) => {
+    if (originalOnKeyDown != null) {
+      originalOnKeyDown(e);
+    }
+    // Ignore key events bubbling up from focusable descendants.
+    if (e.target !== e.currentTarget || (e.key !== "Enter" && e.key !== " ")) {
+      return;
+    }
+    // Prevent Space from scrolling the page.
+    e.preventDefault();
+    if (!e.repeat) {
+      e.currentTarget.click();
+    }
+  };
+
+  const buttonProps =
+    htmlProps.onClick == null
+      ? { onKeyDown: originalOnKeyDown }
+      : { role: "button", tabIndex: 0, onKeyDown };
+
   return (
     <div
+      {...buttonProps}
       {...htmlProps}
       className={classnames(className, { [ACTIVE_CLASSNAME]: active })}
       onPointerDown={onPointerDown}