Merge pull request 'Keep disabled controls in the accessibility tree so their reason is announced' (#49) from fix/disabled-control-accessibility into main

Reviewed-on: #49

app/src/components/projects/AddProjectDialog.test.tsx

@@ -0,0 +1,118 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render, screen, fireEvent, waitFor, act } from "@testing-library/react";
+import AddProjectDialog from "./AddProjectDialog";
+
+const add = vi.fn();
+
+vi.mock("../../hooks/useProjects", () => ({
+  useProjects: () => ({ add }),
+}));
+
+vi.mock("@tauri-apps/plugin-dialog", () => ({
+  open: vi.fn(async () => null),
+}));
+
+/** A promise whose resolution this test controls, so `loading` can be held open. */
+function deferred() {
+  let resolve!: (v: unknown) => void;
+  const promise = new Promise((r) => {
+    resolve = r;
+  });
+  return { promise, resolve };
+}
+
+function fillValidForm() {
+  fireEvent.change(screen.getByLabelText("Project name"), {
+    target: { value: "my-project" },
+  });
+  fireEvent.change(screen.getByLabelText("Folder 1 host path"), {
+    target: { value: "/home/user/my-project" },
+  });
+}
+
+function submitButton() {
+  return screen.getByRole("button", { name: /Add Project|Adding/ });
+}
+
+describe("AddProjectDialog", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("adds the project with the name and folder entered", async () => {
+    add.mockResolvedValue({ id: "p1" });
+    const onClose = vi.fn();
+    render(<AddProjectDialog onClose={onClose} />);
+    fillValidForm();
+    fireEvent.click(submitButton());
+    await waitFor(() =>
+      expect(add).toHaveBeenCalledWith("my-project", [
+        { host_path: "/home/user/my-project", mount_name: "my-project" },
+      ]),
+    );
+    await waitFor(() => expect(onClose).toHaveBeenCalled());
+  });
+
+  it("keeps the submit button announced, and explains why, while adding", async () => {
+    const { promise, resolve } = deferred();
+    add.mockReturnValue(promise);
+    render(<AddProjectDialog onClose={vi.fn()} />);
+    fillValidForm();
+    fireEvent.click(submitButton());
+
+    // Native `disabled` would remove the button from the accessibility tree
+    // exactly when it has something to say.
+    await waitFor(() =>
+      expect(submitButton()).toHaveAttribute("aria-disabled", "true"),
+    );
+    expect(submitButton()).not.toBeDisabled();
+    expect(submitButton()).toHaveAccessibleDescription(/being added/i);
+
+    await act(async () => resolve({ id: "p1" }));
+  });
+
+  it("ignores clicks and Enter/Space on the submit button while adding", async () => {
+    const { promise, resolve } = deferred();
+    add.mockReturnValue(promise);
+    render(<AddProjectDialog onClose={vi.fn()} />);
+    fillValidForm();
+    fireEvent.click(submitButton());
+    await waitFor(() =>
+      expect(submitButton()).toHaveAttribute("aria-disabled", "true"),
+    );
+
+    fireEvent.click(submitButton());
+    fireEvent.keyDown(submitButton(), { key: "Enter" });
+    fireEvent.keyDown(submitButton(), { key: " " });
+    expect(add).toHaveBeenCalledTimes(1);
+
+    await act(async () => resolve({ id: "p1" }));
+  });
+
+  it("ignores a form submit raised from elsewhere while adding", async () => {
+    const { promise, resolve } = deferred();
+    add.mockReturnValue(promise);
+    render(<AddProjectDialog onClose={vi.fn()} />);
+    fillValidForm();
+    fireEvent.click(submitButton());
+    await waitFor(() =>
+      expect(submitButton()).toHaveAttribute("aria-disabled", "true"),
+    );
+
+    // Enter in a text field submits a form regardless of the submit button's
+    // state, so the handler has to guard itself too.
+    // Modal portals to document.body, so the form is not under `container`.
+    const form = document.querySelector("form");
+    expect(form).not.toBeNull();
+    fireEvent.submit(form!);
+    expect(add).toHaveBeenCalledTimes(1);
+
+    await act(async () => resolve({ id: "p1" }));
+  });
+
+  it("leaves the submit button plainly available when idle", () => {
+    render(<AddProjectDialog onClose={vi.fn()} />);
+    expect(submitButton()).not.toHaveAttribute("aria-disabled");
+    expect(submitButton()).toHaveAccessibleDescription("");
+  });
+});

app/src/components/projects/AddProjectDialog.tsx

@@ -55,6 +55,10 @@ export default function AddProjectDialog({ onClose }: Props) {
 
   const handleSubmit = async (e?: React.FormEvent) => {
     if (e) e.preventDefault();
+    // The submit button is `aria-disabled` rather than `disabled` while an add
+    // is in flight, and Enter inside a text field submits the form without
+    // touching the button at all. Both routes end here, so the guard does too.
+    if (loading) return;
     if (!name.trim()) {
       setError("Project name is required");
       return;
@@ -97,7 +101,19 @@ export default function AddProjectDialog({ onClose }: Props) {
           <Button size="md" variant="ghost" onClick={onClose}>
             Cancel
           </Button>
-          <Button size="md" variant="primary" type="submit" form={formId} disabled={loading}>
+          <Button
+            size="md"
+            variant="primary"
+            type="submit"
+            form={formId}
+            unavailable={loading}
+            unavailableReason="The project is being added. Wait for it to finish."
+            title={
+              loading
+                ? "The project is being added. Wait for it to finish."
+                : undefined
+            }
+          >
             {loading ? "Adding…" : "Add Project"}
           </Button>
         </>

app/src/components/projects/ProjectRow.test.tsx

@@ -122,14 +122,6 @@ describe("ProjectRow", () => {
   });
 
   it("only allows opening a terminal while the container runs", () => {
-    const { unmount } = render(<ProjectRow project={baseProject} />);
-    expect(
-      screen.getByRole("button", {
-        name: "Open a Claude terminal for Test Project",
-      }),
-    ).toBeDisabled();
-    unmount();
-
     render(<ProjectRow project={{ ...baseProject, status: "running" }} />);
     fireEvent.click(
       screen.getByRole("button", {
@@ -139,6 +131,38 @@ describe("ProjectRow", () => {
     expect(mockOpenClaudeTerminal).toHaveBeenCalled();
   });
 
+  it("keeps the terminal button announced, and explains why, while stopped", () => {
+    render(<ProjectRow project={baseProject} />);
+    const button = screen.getByRole("button", {
+      name: "Open a Claude terminal for Test Project",
+    });
+    // Native `disabled` would drop the button out of the accessibility tree
+    // and out of the tab order, taking the reason with it.
+    expect(button).not.toBeDisabled();
+    expect(button).toHaveAttribute("aria-disabled", "true");
+    expect(button).toHaveAccessibleDescription(/is not running/i);
+  });
+
+  it("ignores clicks and Enter/Space on the terminal button while stopped", () => {
+    render(<ProjectRow project={baseProject} />);
+    const button = screen.getByRole("button", {
+      name: "Open a Claude terminal for Test Project",
+    });
+    fireEvent.click(button);
+    fireEvent.keyDown(button, { key: "Enter" });
+    fireEvent.keyDown(button, { key: " " });
+    expect(mockOpenClaudeTerminal).not.toHaveBeenCalled();
+  });
+
+  it("drops aria-disabled once the container is running", () => {
+    render(<ProjectRow project={{ ...baseProject, status: "running" }} />);
+    const button = screen.getByRole("button", {
+      name: "Open a Claude terminal for Test Project",
+    });
+    expect(button).not.toHaveAttribute("aria-disabled");
+    expect(button).not.toHaveAccessibleDescription(/is not running/i);
+  });
+
   it("shows container progress inline rather than in a blocking modal", () => {
     setStore({ containerProgress: { "test-1": "Pulling image…" } });
     render(<ProjectRow project={{ ...baseProject, status: "starting" }} />);

app/src/components/projects/ProjectRow.tsx

@@ -3,6 +3,7 @@ import type { Project } from "../../lib/types";
 import { useAppState, homeTabKey } from "../../store/appState";
 import { useProjectActions } from "../../hooks/useProjectActions";
 import { ProjectStatusIndicator } from "../ui/StatusIndicator";
+import { useUnavailable } from "../ui/unavailable";
 
 interface Props {
   project: Project;
@@ -31,6 +32,15 @@ export default function ProjectRow({ project }: Props) {
   const isTransitioning =
     project.status === "starting" || project.status === "stopping";
 
+  // A terminal needs a running container. Saying so out loud beats a `disabled`
+  // attribute that hides the button — and the reason — from anyone not using a
+  // mouse and eyes.
+  const terminal = useUnavailable({
+    unavailable: !isRunning,
+    reason: `${project.name} is not running. Start it to open a terminal.`,
+    onClick: () => openClaudeTerminal(),
+  });
+
   return (
     <div
       className={`group relative px-2 py-1.5 rounded-[var(--radius-control)] transition-colors min-w-0 overflow-hidden ${
@@ -113,11 +123,14 @@ export default function ProjectRow({ project }: Props) {
         </button>
         <button
           type="button"
-          disabled={!isRunning}
-          onClick={() => openClaudeTerminal()}
-          title={`Open a Claude terminal for ${project.name}`}
+          {...terminal.controlProps}
+          title={
+            isRunning
+              ? `Open a Claude terminal for ${project.name}`
+              : `${project.name} is not running. Start it to open a terminal.`
+          }
           aria-label={`Open a Claude terminal for ${project.name}`}
-          className="w-6 h-6 flex items-center justify-center rounded-[var(--radius-control)] text-[var(--text-secondary)] hover:text-[var(--text-primary)] hover:bg-[var(--bg-primary)] disabled:text-[var(--text-disabled)] transition-colors"
+          className="w-6 h-6 flex items-center justify-center rounded-[var(--radius-control)] text-[var(--text-secondary)] hover:text-[var(--text-primary)] hover:bg-[var(--bg-primary)] disabled:text-[var(--text-disabled)] aria-disabled:text-[var(--text-disabled)] aria-disabled:hover:text-[var(--text-disabled)] aria-disabled:hover:bg-transparent aria-disabled:cursor-not-allowed transition-colors"
         >
           <svg
             className="w-3.5 h-3.5"
@@ -134,6 +147,7 @@ export default function ProjectRow({ project }: Props) {
             <line x1="13" y1="15" x2="17" y2="15" />
           </svg>
         </button>
+        {terminal.reasonNode}
       </div>
     </div>
   );

app/src/components/ui/Button.test.tsx

@@ -0,0 +1,77 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render, screen, fireEvent } from "@testing-library/react";
+import Button from "./Button";
+
+const onClick = vi.fn();
+const onKeyDown = vi.fn();
+
+describe("Button", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("still supports the native disabled attribute", () => {
+    render(
+      <Button disabled onClick={onClick}>
+        Save
+      </Button>,
+    );
+    expect(screen.getByRole("button", { name: "Save" })).toBeDisabled();
+  });
+
+  it("stays in the accessibility tree when unavailable, and says why", () => {
+    render(
+      <Button unavailable unavailableReason="Stop the container first.">
+        Save
+      </Button>,
+    );
+    const button = screen.getByRole("button", { name: "Save" });
+    expect(button).not.toBeDisabled();
+    expect(button).toHaveAttribute("aria-disabled", "true");
+    expect(button).toHaveAccessibleDescription("Stop the container first.");
+    // The reason is a description, not part of the name.
+    expect(button).toHaveAccessibleName("Save");
+  });
+
+  it("guards clicks and Enter/Space while unavailable", () => {
+    render(
+      <Button unavailable unavailableReason="Stop the container first." onClick={onClick}>
+        Save
+      </Button>,
+    );
+    const button = screen.getByRole("button", { name: "Save" });
+    fireEvent.click(button);
+    fireEvent.keyDown(button, { key: "Enter" });
+    fireEvent.keyDown(button, { key: " " });
+    expect(onClick).not.toHaveBeenCalled();
+  });
+
+  it("still forwards keys that are not activation keys", () => {
+    render(
+      <Button
+        unavailable
+        unavailableReason="Stop the container first."
+        onKeyDown={onKeyDown}
+      >
+        Save
+      </Button>,
+    );
+    fireEvent.keyDown(screen.getByRole("button", { name: "Save" }), {
+      key: "Escape",
+    });
+    expect(onKeyDown).toHaveBeenCalled();
+  });
+
+  it("behaves like an ordinary button when available", () => {
+    render(
+      <Button unavailable={false} unavailableReason="Stop the container first." onClick={onClick}>
+        Save
+      </Button>,
+    );
+    const button = screen.getByRole("button", { name: "Save" });
+    expect(button).not.toHaveAttribute("aria-disabled");
+    expect(button).toHaveAccessibleDescription("");
+    fireEvent.click(button);
+    expect(onClick).toHaveBeenCalledTimes(1);
+  });
+});

app/src/components/ui/Button.tsx

@@ -1,4 +1,5 @@
 import type { ButtonHTMLAttributes, ReactNode } from "react";
+import { useUnavailable } from "./unavailable";
 
 export type ButtonVariant = "primary" | "secondary" | "danger" | "ghost";
 export type ButtonSize = "sm" | "md";
@@ -7,22 +8,37 @@ interface Props extends ButtonHTMLAttributes<HTMLButtonElement> {
   variant?: ButtonVariant;
   size?: ButtonSize;
   children: ReactNode;
+  /**
+   * Unavailable, but still announced. Renders `aria-disabled` and wires
+   * `unavailableReason` to `aria-describedby` instead of using the native
+   * `disabled` attribute, which would take the button out of the tab order and
+   * out of the accessibility tree — reason and all. Clicks and Enter/Space are
+   * guarded for you. Prefer this over `disabled` whenever there is a reason
+   * worth telling the user.
+   */
+  unavailable?: boolean;
+  /** Why the button cannot be used. Required for `unavailable` to say anything. */
+  unavailableReason?: string;
 }
 
 /**
  * Real buttons with visible bounds and a ≥24px hit target.
  * Filled variants use the *-emphasis tokens so white text clears WCAG AA;
  * `--accent` stays reserved for foreground/link use.
+ *
+ * The `aria-disabled:` class mirrors below exist because Tailwind's
+ * `disabled:` variant only matches the native attribute, which `unavailable`
+ * deliberately does not set. Keep the two lists in step.
  */
 const VARIANTS: Record<ButtonVariant, string> = {
   primary:
-    "bg-[var(--accent-emphasis)] text-white border border-transparent hover:bg-[var(--accent-emphasis-hover)] disabled:bg-[var(--bg-tertiary)] disabled:text-[var(--text-disabled)] disabled:border-[var(--border-color)]",
+    "bg-[var(--accent-emphasis)] text-white border border-transparent hover:bg-[var(--accent-emphasis-hover)] disabled:bg-[var(--bg-tertiary)] disabled:text-[var(--text-disabled)] disabled:border-[var(--border-color)] aria-disabled:bg-[var(--bg-tertiary)] aria-disabled:text-[var(--text-disabled)] aria-disabled:border-[var(--border-color)] aria-disabled:hover:bg-[var(--bg-tertiary)]",
   secondary:
-    "bg-[var(--bg-tertiary)] text-[var(--text-primary)] border border-[var(--border-color)] hover:bg-[var(--border-color)] disabled:text-[var(--text-disabled)] disabled:hover:bg-[var(--bg-tertiary)]",
+    "bg-[var(--bg-tertiary)] text-[var(--text-primary)] border border-[var(--border-color)] hover:bg-[var(--border-color)] disabled:text-[var(--text-disabled)] disabled:hover:bg-[var(--bg-tertiary)] aria-disabled:text-[var(--text-disabled)] aria-disabled:hover:bg-[var(--bg-tertiary)]",
   danger:
-    "bg-transparent text-[var(--error)] border border-[var(--error)]/40 hover:bg-[var(--error-muted)] disabled:text-[var(--text-disabled)] disabled:border-[var(--border-color)] disabled:hover:bg-transparent",
+    "bg-transparent text-[var(--error)] border border-[var(--error)]/40 hover:bg-[var(--error-muted)] disabled:text-[var(--text-disabled)] disabled:border-[var(--border-color)] disabled:hover:bg-transparent aria-disabled:text-[var(--text-disabled)] aria-disabled:border-[var(--border-color)] aria-disabled:hover:bg-transparent",
   ghost:
-    "bg-transparent text-[var(--text-secondary)] border border-transparent hover:text-[var(--text-primary)] hover:bg-[var(--bg-tertiary)] disabled:text-[var(--text-disabled)] disabled:hover:bg-transparent",
+    "bg-transparent text-[var(--text-secondary)] border border-transparent hover:text-[var(--text-primary)] hover:bg-[var(--bg-tertiary)] disabled:text-[var(--text-disabled)] disabled:hover:bg-transparent aria-disabled:text-[var(--text-disabled)] aria-disabled:hover:text-[var(--text-disabled)] aria-disabled:hover:bg-transparent",
 };
 
 const SIZES: Record<ButtonSize, string> = {
@@ -35,16 +51,30 @@ export default function Button({
   size = "sm",
   className = "",
   type = "button",
+  unavailable = false,
+  unavailableReason = "",
   children,
   ...rest
 }: Props) {
+  const { controlProps, reasonNode } = useUnavailable({
+    unavailable,
+    reason: unavailableReason,
+    onClick: rest.onClick,
+    onKeyDown: rest.onKeyDown,
+  });
+
   return (
-    <button
-      type={type}
-      {...rest}
-      className={`inline-flex items-center justify-center whitespace-nowrap rounded-[var(--radius-control)] font-medium transition-colors disabled:cursor-not-allowed ${SIZES[size]} ${VARIANTS[variant]} ${className}`}
-    >
-      {children}
-    </button>
+    <>
+      <button
+        type={type}
+        {...rest}
+        {...controlProps}
+        className={`inline-flex items-center justify-center whitespace-nowrap rounded-[var(--radius-control)] font-medium transition-colors disabled:cursor-not-allowed aria-disabled:cursor-not-allowed ${SIZES[size]} ${VARIANTS[variant]} ${className}`}
+      >
+        {children}
+      </button>
+      {/* Outside the button: inside, the reason would join its accessible name. */}
+      {reasonNode}
+    </>
   );
 }

app/src/components/ui/unavailable.tsx

@@ -0,0 +1,87 @@
+import {
+  useId,
+  type KeyboardEventHandler,
+  type MouseEventHandler,
+  type ReactNode,
+} from "react";
+
+/** Keys a native `<button>` turns into a click. */
+const ACTIVATION_KEYS = new Set([" ", "Spacebar", "Enter"]);
+
+export interface UnavailableControlProps {
+  "aria-disabled"?: true;
+  "aria-describedby"?: string;
+  onClick?: MouseEventHandler<HTMLButtonElement>;
+  onKeyDown?: KeyboardEventHandler<HTMLButtonElement>;
+}
+
+export interface UnavailableControl {
+  /** Spread onto the control. Carries the guarded handlers. */
+  controlProps: UnavailableControlProps;
+  /**
+   * Render as a *sibling* of the control — inside it the reason would be
+   * appended to the accessible name instead of the description.
+   */
+  reasonNode: ReactNode;
+}
+
+/**
+ * Makes a control unavailable without hiding it from assistive technology.
+ *
+ * `disabled` takes an element out of the tab order *and* out of the
+ * accessibility tree, so the `title` explaining why it cannot be used is
+ * announced to nobody and shown only to a sighted user with a mouse. That is
+ * backwards: the people who most need the reason are the ones who never get
+ * it. `aria-disabled` keeps the control focusable and announced, and
+ * `aria-describedby` hands over the reason.
+ *
+ * The catch is that `aria-disabled` is advisory — it does not block clicks or
+ * Enter/Space the way `disabled` does. This hook therefore returns the guards
+ * along with the attributes, so a call site cannot take the announcement
+ * without the guard. Handlers that a form can reach without going through the
+ * control (Enter inside a text field submits the form) still have to guard
+ * themselves.
+ */
+export function useUnavailable({
+  unavailable,
+  reason,
+  onClick,
+  onKeyDown,
+}: {
+  unavailable: boolean;
+  reason: string;
+  onClick?: MouseEventHandler<HTMLButtonElement>;
+  onKeyDown?: KeyboardEventHandler<HTMLButtonElement>;
+}): UnavailableControl {
+  const reasonId = `${useId()}unavailable`;
+
+  if (!unavailable) {
+    return { controlProps: { onClick, onKeyDown }, reasonNode: null };
+  }
+
+  return {
+    controlProps: {
+      "aria-disabled": true,
+      "aria-describedby": reasonId,
+      onClick: (e) => {
+        e.preventDefault();
+        e.stopPropagation();
+      },
+      onKeyDown: (e) => {
+        if (!ACTIVATION_KEYS.has(e.key)) {
+          onKeyDown?.(e);
+          return;
+        }
+        // Suppress the default action before it can become a click, submit a
+        // form, or scroll the page.
+        e.preventDefault();
+        e.stopPropagation();
+      },
+    },
+    reasonNode: (
+      <span id={reasonId} className="sr-only">
+        {reason}
+      </span>
+    ),
+  };
+}