diff --git a/app/src/components/projects/AddProjectDialog.test.tsx b/app/src/components/projects/AddProjectDialog.test.tsx new file mode 100644 index 0000000..4179f43 --- /dev/null +++ b/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(); + 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(); + 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(); + 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(); + 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(); + expect(submitButton()).not.toHaveAttribute("aria-disabled"); + expect(submitButton()).toHaveAccessibleDescription(""); + }); +}); diff --git a/app/src/components/projects/AddProjectDialog.tsx b/app/src/components/projects/AddProjectDialog.tsx index 72149a2..0187fd3 100644 --- a/app/src/components/projects/AddProjectDialog.tsx +++ b/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) { Cancel - + {loading ? "Adding…" : "Add Project"} > 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 ( 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" > + {terminal.reasonNode} ); 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( + + Save + , + ); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); + }); + + it("stays in the accessibility tree when unavailable, and says why", () => { + render( + + Save + , + ); + 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( + + Save + , + ); + 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( + + Save + , + ); + fireEvent.keyDown(screen.getByRole("button", { name: "Save" }), { + key: "Escape", + }); + expect(onKeyDown).toHaveBeenCalled(); + }); + + it("behaves like an ordinary button when available", () => { + render( + + Save + , + ); + 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 ( - - {children} - + <> + + {children} + + {/* 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 `` turns into a click. */ +const ACTIVATION_KEYS = new Set([" ", "Spacebar", "Enter"]); + +export interface UnavailableControlProps { + "aria-disabled"?: true; + "aria-describedby"?: string; + onClick?: MouseEventHandler; + onKeyDown?: KeyboardEventHandler; +} + +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; + onKeyDown?: KeyboardEventHandler; +}): 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: ( + + {reason} + + ), + }; +}