From 1eb91a35eb8876e5deebb18b8d4fa54a8dfed699 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 2 Sep 2026 09:08:57 -0700 Subject: [PATCH] Give the terminal and Add Project buttons a reason a screen reader can hear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were the defect the new hook exists for. The sidebar's Claude terminal button is disabled whenever the container is not running and never said so — its `title` names the action, so the precondition appeared nowhere in the accessibility tree at all. Add Project's submit button is disabled while an add is in flight, and its only signal is the label swapping to "Adding…" on an element a screen reader can no longer reach. The submit button needs a second guard the hook cannot supply: Enter inside a text field submits a form without touching the submit button, so `handleSubmit` now returns early while loading. Without it, swapping `disabled` for `aria-disabled` would have turned an accessibility fix into a double-submit bug. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm --- .../projects/AddProjectDialog.test.tsx | 118 ++++++++++++++++++ .../components/projects/AddProjectDialog.tsx | 18 ++- .../components/projects/ProjectRow.test.tsx | 40 ++++-- app/src/components/projects/ProjectRow.tsx | 22 +++- 4 files changed, 185 insertions(+), 13 deletions(-) create mode 100644 app/src/components/projects/AddProjectDialog.test.tsx 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) { - 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}
);