From aa0a574091d5b62d7ec4f2579bd7bffd425e2a41 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 2 Sep 2026 09:08:50 -0700 Subject: [PATCH 1/2] Announce unavailable controls instead of hiding them behind `disabled` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Native `disabled` removes an element from the tab order and from the accessibility tree, so any explanation of why a control cannot be used is delivered only to a sighted user with a mouse. `useUnavailable` is the way out: `aria-disabled` keeps the control focusable and announced, `aria-describedby` carries the reason, and — because `aria-disabled` is advisory and blocks nothing — the hook hands back the click and Enter/Space guards along with the attributes, so a call site cannot take the announcement without the guard. `Button` gets it as an opt-in `unavailable` / `unavailableReason` pair. Opt-in matters: 37 files render this button and none of them change. The `aria-disabled:` class mirrors exist because Tailwind's `disabled:` variant only matches the native attribute, which this pattern deliberately omits. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm --- app/src/components/ui/Button.test.tsx | 77 ++++++++++++++++++++++++ app/src/components/ui/Button.tsx | 52 ++++++++++++---- app/src/components/ui/unavailable.tsx | 87 +++++++++++++++++++++++++++ 3 files changed, 205 insertions(+), 11 deletions(-) create mode 100644 app/src/components/ui/Button.test.tsx create mode 100644 app/src/components/ui/unavailable.tsx diff --git a/app/src/components/ui/Button.test.tsx b/app/src/components/ui/Button.test.tsx new file mode 100644 index 0000000..0a85c43 --- /dev/null +++ b/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( + , + ); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + }); + + it("stays in the accessibility tree when unavailable, and says why", () => { + render( + , + ); + 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( + , + ); + 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( + , + ); + fireEvent.keyDown(screen.getByRole("button", { name: "Save" }), { + key: "Escape", + }); + expect(onKeyDown).toHaveBeenCalled(); + }); + + it("behaves like an ordinary button when available", () => { + render( + , + ); + const button = screen.getByRole("button", { name: "Save" }); + expect(button).not.toHaveAttribute("aria-disabled"); + expect(button).toHaveAccessibleDescription(""); + fireEvent.click(button); + expect(onClick).toHaveBeenCalledTimes(1); + }); +}); diff --git a/app/src/components/ui/Button.tsx b/app/src/components/ui/Button.tsx index 79b8655..be41f79 100644 --- a/app/src/components/ui/Button.tsx +++ b/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 { 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 = { 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 = { @@ -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 ( - + <> + + {/* Outside the button: inside, the reason would join its accessible name. */} + {reasonNode} + ); } diff --git a/app/src/components/ui/unavailable.tsx b/app/src/components/ui/unavailable.tsx new file mode 100644 index 0000000..b25e7bc --- /dev/null +++ b/app/src/components/ui/unavailable.tsx @@ -0,0 +1,87 @@ +import { + useId, + type KeyboardEventHandler, + type MouseEventHandler, + type ReactNode, +} from "react"; + +/** Keys a native ` - diff --git a/app/src/components/projects/ProjectRow.test.tsx b/app/src/components/projects/ProjectRow.test.tsx index d22064c..ffd6b6e 100644 --- a/app/src/components/projects/ProjectRow.test.tsx +++ b/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(); - expect( - screen.getByRole("button", { - name: "Open a Claude terminal for Test Project", - }), - ).toBeDisabled(); - unmount(); - render(); 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(); + 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(); + 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(); + 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(); diff --git a/app/src/components/projects/ProjectRow.tsx b/app/src/components/projects/ProjectRow.tsx index aa14fba..87e981e 100644 --- a/app/src/components/projects/ProjectRow.tsx +++ b/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 (
+ {terminal.reasonNode}
);