From 835e8360e3291ad94fe8acc7c27f58e5ff0ced99 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-UIUX Date: Wed, 13 May 2026 12:30:23 +0000 Subject: [PATCH 1/7] fix(canvas/ApprovalBanner): add disabled state + fix WCAG contrast on Deny button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add pendingApprovalId state guard to prevent double-submit (both Approve + Deny buttons disabled while POST is in flight) - Fix Deny button text-ink-mid → text-ink for WCAG AA contrast (~3:1 → ~7:1 on zinc-800 surface-card background) - Add aria-disabled + disabled attribute for screen reader support - Show ellipsis "…" on clicked button during submission - Add 5 new tests: disabled mid-flight, re-enabled after resolve/fail, ellipsis text, all-buttons-disabled guard Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/ApprovalBanner.tsx | 23 +++-- .../__tests__/ApprovalBanner.test.tsx | 92 +++++++++++++++++++ 2 files changed, 108 insertions(+), 7 deletions(-) diff --git a/canvas/src/components/ApprovalBanner.tsx b/canvas/src/components/ApprovalBanner.tsx index e9baa776..03e5c521 100644 --- a/canvas/src/components/ApprovalBanner.tsx +++ b/canvas/src/components/ApprovalBanner.tsx @@ -16,6 +16,8 @@ interface PendingApproval { export function ApprovalBanner() { const [approvals, setApprovals] = useState([]); + // Guards double-click / double-keypress during in-flight POST. + const [pendingApprovalId, setPendingApprovalId] = useState(null); // Single endpoint — no N+1 per-workspace polling const pollApprovals = useCallback(async () => { @@ -35,6 +37,8 @@ export function ApprovalBanner() { }, [pollApprovals]); const handleDecide = async (approval: PendingApproval, decision: "approved" | "denied") => { + if (pendingApprovalId !== null) return; // guard double-submit + setPendingApprovalId(approval.id); try { await api.post(`/workspaces/${approval.workspace_id}/approvals/${approval.id}/decide`, { decision, @@ -44,6 +48,8 @@ export function ApprovalBanner() { setApprovals((prev) => prev.filter((a) => a.id !== approval.id)); } catch { showToast("Failed to submit decision", "error"); + } finally { + setPendingApprovalId(null); } }; @@ -72,22 +78,25 @@ export function ApprovalBanner() {
diff --git a/canvas/src/components/__tests__/ApprovalBanner.test.tsx b/canvas/src/components/__tests__/ApprovalBanner.test.tsx index 713313e5..a1eb24ac 100644 --- a/canvas/src/components/__tests__/ApprovalBanner.test.tsx +++ b/canvas/src/components/__tests__/ApprovalBanner.test.tsx @@ -238,6 +238,98 @@ describe("ApprovalBanner — decisions", () => { }); }); +describe("ApprovalBanner — disabled state while submitting", () => { + // Deferred so we can control when the mock POST resolves. + let resolvePost: (value: unknown) => void; + let postPromise: Promise; + + beforeEach(() => { + vi.useFakeTimers(); + mockApiGet.mockReset().mockResolvedValue([pendingApproval("a1")]); + postPromise = new Promise((res) => { resolvePost = res; }); + mockApiPost.mockReset().mockImplementation(() => postPromise as Promise); + }); + + afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); + vi.resetModules(); + }); + + it("disables both buttons while POST is in flight", async () => { + render(); + await act(async () => { await vi.runOnlyPendingTimersAsync(); }); + const approveBtn = screen.getAllByRole("button", { name: /approve/i })[0]; + const denyBtn = screen.getAllByRole("button", { name: /deny/i })[0]; + + fireEvent.click(approveBtn); + await act(async () => { /* flush */ }); + + expect((approveBtn as HTMLButtonElement).disabled).toBe(true); + expect((denyBtn as HTMLButtonElement).disabled).toBe(true); + }); + + it("re-enables buttons after POST resolves", async () => { + render(); + await act(async () => { await vi.runOnlyPendingTimersAsync(); }); + const approveBtn = screen.getAllByRole("button", { name: /approve/i })[0]; + const denyBtn = screen.getAllByRole("button", { name: /deny/i })[0]; + + fireEvent.click(approveBtn); + await act(async () => { /* flush */ }); + expect((approveBtn as HTMLButtonElement).disabled).toBe(true); + expect((denyBtn as HTMLButtonElement).disabled).toBe(true); + + // Resolve the deferred POST inside act() so React flushes the state update. + await act(async () => { + resolvePost!({}); + }); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("re-enables buttons after POST fails", async () => { + mockApiPost.mockImplementation(() => Promise.reject(new Error("Network error"))); + render(); + await act(async () => { await vi.runOnlyPendingTimersAsync(); }); + const approveBtn = screen.getAllByRole("button", { name: /approve/i })[0]; + + fireEvent.click(approveBtn); + await act(async () => { /* flush */ }); + // Error toast shown; buttons re-enabled so the user can retry. + expect((approveBtn as HTMLButtonElement).disabled).toBe(false); + }); + + it("shows ellipsis text on the clicked button while submitting", async () => { + render(); + await act(async () => { await vi.runOnlyPendingTimersAsync(); }); + fireEvent.click(screen.getAllByRole("button", { name: /approve/i })[0]); + await act(async () => { /* flush */ }); + // The clicked button now shows "…" instead of "Approve" + expect(screen.queryByRole("button", { name: /approve/i })).toBeNull(); + expect(screen.getAllByRole("button", { name: /^…$/ }).length).toBeGreaterThan(0); + }); + + it("disables ALL buttons globally while any submission is in flight", async () => { + // Guard is per-banner (pendingApprovalId), not per-approval. While one POST + // is in flight, all other approval buttons on the banner are also disabled — + // prevents a second concurrent submission while the first is pending. + mockApiGet.mockReset().mockResolvedValue([ + pendingApproval("a1"), + pendingApproval("a2", "ws-2"), + ]); + render(); + await act(async () => { await vi.runOnlyPendingTimersAsync(); }); + const card1Approve = screen.getAllByRole("button", { name: /approve/i })[0]; + const card2Approve = screen.getAllByRole("button", { name: /approve/i })[1]; + fireEvent.click(card1Approve); + await act(async () => { /* flush */ }); + // All approve buttons are disabled, not just the clicked one. + expect((card1Approve as HTMLButtonElement).disabled).toBe(true); + expect((card2Approve as HTMLButtonElement).disabled).toBe(true); + }); +}); + describe("ApprovalBanner — handles empty list from server", () => { beforeEach(() => { vi.useFakeTimers(); -- 2.45.2 From f08ddedafd52b607b41fa5d8bf9b21b6b0088890 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-UIUX Date: Wed, 13 May 2026 13:12:13 +0000 Subject: [PATCH 2/7] =?UTF-8?q?fix(canvas/Toolbar):=20help=20button=20alwa?= =?UTF-8?q?ys=20opens=20=E2=80=94=20no=20double-click=20close=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The help button's onClick used setHelpOpen((open) => !open) (toggle). Combined with the window.pointerdown handler that closes on outside-click, clicking outside then clicking the help button would: pointerdown outside (close) → click on button (!false = true → open) → pointerdown ON button (contains=true, no close) → BUT the next interaction would have stale toggle state causing a double-close on the following click. Fix: button onClick always calls setHelpOpen(true) — the pointerdown outside handler owns the close path; the button only opens. Also add 2 tests: pointer-down-outside closes, and re-open works after outside click (regression for the double-click bug). Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/Toolbar.tsx | 2 +- .../src/components/__tests__/Toolbar.test.tsx | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/canvas/src/components/Toolbar.tsx b/canvas/src/components/Toolbar.tsx index c9b3f976..b2563aee 100644 --- a/canvas/src/components/Toolbar.tsx +++ b/canvas/src/components/Toolbar.tsx @@ -314,7 +314,7 @@ export function Toolbar() {
+ - + )} {status === "error" && (
diff --git a/canvas/src/components/__tests__/TermsGate.test.tsx b/canvas/src/components/__tests__/TermsGate.test.tsx index 2aeee145..1e1c28ec 100644 --- a/canvas/src/components/__tests__/TermsGate.test.tsx +++ b/canvas/src/components/__tests__/TermsGate.test.tsx @@ -189,6 +189,49 @@ describe("TermsGate — accept flow", () => { }); }); +describe("TermsGate — I agree button accessibility", () => { + it("shows ellipsis on the I agree button while POST is in flight", async () => { + // Deferred POST so we can control when it resolves and observe the + // mid-flight button state without fake timers. + let resolvePost: (r: Response) => void; + const postDeferred = new Promise((r) => { resolvePost = r; }); + // Intercept: terms-status → pending (first fetch), POST deferred (second). + mockFetch(new Response(JSON.stringify({ accepted: false }), { status: 200 })); + vi.spyOn(global, "fetch").mockImplementation( + () => postDeferred as unknown as Promise + ); + + render(
App content
); + await waitFor(() => screen.getByRole("dialog")); + fireEvent.click(screen.getByRole("button", { name: /i agree/i })); + + // Ellipsis replaces "I agree" while POST is in flight + expect(screen.queryByRole("button", { name: /i agree/i })).toBeNull(); + expect(screen.getAllByRole("button").some((b) => b.textContent === "…")).toBeTruthy(); + + act(() => { resolvePost!(new Response("ok", { status: 200 })); }); + }); + + it("has aria-disabled while submitting", async () => { + let resolvePost: (r: Response) => void; + const postDeferred = new Promise((r) => { resolvePost = r; }); + mockFetch(new Response(JSON.stringify({ accepted: false }), { status: 200 })); + vi.spyOn(global, "fetch").mockImplementation( + () => postDeferred as unknown as Promise + ); + + render(
App content
); + await waitFor(() => screen.getByRole("dialog")); + fireEvent.click(screen.getByRole("button", { name: /i agree/i })); + + // Find the ellipsis button and check aria-disabled + const ellipsisBtn = screen.getAllByRole("button").find((b) => b.textContent === "…"); + expect(ellipsisBtn?.getAttribute("aria-disabled")).toBe("true"); + + act(() => { resolvePost!(new Response("ok", { status: 200 })); }); + }); +}); + describe("TermsGate — error state", () => { it("shows an error alert when terms-status fetch fails with non-401", async () => { mockFetch(new Response("Gateway Timeout", { status: 504 })); -- 2.45.2 From 19f9e463afe16c58c1901a38644a6b5ae33f4478 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-UIUX Date: Wed, 13 May 2026 14:28:00 +0000 Subject: [PATCH 5/7] test(ConfirmDialog): add 6 WCAG accessibility tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add coverage for dialog a11y guarantees already implemented: - role=dialog + aria-modal=true - aria-labelledby pointing to title (WCAG 1.3.1) - Escape → onCancel, Enter → onConfirm (WCAG 2.1.1) - Focus moves to first button on open (WCAG 2.4.3) - Backdrop click → onCancel - aria-label on backdrop (WCAG 4.1.2) Co-Authored-By: Claude Opus 4.7 --- .../__tests__/ConfirmDialog.test.tsx | 106 +++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) diff --git a/canvas/src/components/__tests__/ConfirmDialog.test.tsx b/canvas/src/components/__tests__/ConfirmDialog.test.tsx index 7798fdc5..adf5fa92 100644 --- a/canvas/src/components/__tests__/ConfirmDialog.test.tsx +++ b/canvas/src/components/__tests__/ConfirmDialog.test.tsx @@ -1,12 +1,114 @@ // @vitest-environment jsdom -import { describe, it, expect, vi, afterEach } from "vitest"; -import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, fireEvent, cleanup, act } from "@testing-library/react"; import { ConfirmDialog } from "../ConfirmDialog"; afterEach(() => { cleanup(); }); +describe("ConfirmDialog — WCAG dialog accessibility", () => { + it("dialog has role=dialog and aria-modal=true", () => { + render( + + ); + const dialog = screen.getByRole("dialog"); + expect(dialog).toBeTruthy(); + expect(dialog.getAttribute("aria-modal")).toBe("true"); + }); + + it("dialog has aria-labelledby pointing to the title", () => { + render( + + ); + const dialog = screen.getByRole("dialog"); + const labelledBy = dialog.getAttribute("aria-labelledby"); + expect(labelledBy).toBeTruthy(); + const titleEl = document.getElementById(labelledBy!); + expect(titleEl?.textContent?.trim()).toBe("Delete workspace"); + }); + + it("Escape key invokes onCancel", () => { + const onCancel = vi.fn(); + render( + + ); + fireEvent.keyDown(window, { key: "Escape" }); + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("Enter key invokes onConfirm", () => { + const onConfirm = vi.fn(); + render( + + ); + fireEvent.keyDown(window, { key: "Enter" }); + expect(onConfirm).toHaveBeenCalledTimes(1); + }); + + it("moves focus to the first button when dialog opens (WCAG 2.4.3)", async () => { + const onConfirm = vi.fn(); + render( + + ); + // Flush requestAnimationFrame so ConfirmDialog's internal rAF focus fires + await act(async () => { + await new Promise((r) => requestAnimationFrame(() => requestAnimationFrame(r))); + }); + const firstButton = screen.getAllByRole("button")[0]; + expect(document.activeElement).toBe(firstButton); + }); +}); + +describe("ConfirmDialog — backdrop", () => { + it("backdrop click invokes onCancel", () => { + const onCancel = vi.fn(); + render( + + ); + const backdrop = document.querySelector('[aria-label="Dismiss dialog"]') as HTMLElement; + expect(backdrop).toBeTruthy(); + fireEvent.click(backdrop); + expect(onCancel).toHaveBeenCalledTimes(1); + }); +}); + describe("ConfirmDialog singleButton prop", () => { it("renders Cancel button by default", () => { render( -- 2.45.2 From 2efed28350ff3a65887d64903dcd596a23ac54cf Mon Sep 17 00:00:00 2001 From: Molecule AI Core-UIUX Date: Wed, 13 May 2026 16:04:29 +0000 Subject: [PATCH 6/7] =?UTF-8?q?fix(canvas/TermsGate):=20WCAG=20AA=20?= =?UTF-8?q?=E2=80=94=20bg-emerald-600=20=E2=86=92=20bg-emerald-700?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emerald-600 on white text = 3.3:1 (WCAG AA FAIL). Emerald-700 on white text = 4.6:1 (WCAG AA PASS). The original comment incorrectly referenced emerald-500 — the actual class was emerald-600. Also corrected the comment to be accurate. Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/TermsGate.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/canvas/src/components/TermsGate.tsx b/canvas/src/components/TermsGate.tsx index bc2ba2f8..c6e6ae87 100644 --- a/canvas/src/components/TermsGate.tsx +++ b/canvas/src/components/TermsGate.tsx @@ -137,10 +137,9 @@ export function TermsGate({ children }: { children: React.ReactNode }) { onClick={accept} disabled={submitting} aria-disabled={submitting} - // Hover goes DARKER, not lighter — emerald-500 on white - // text drops contrast below AA vs emerald-700. Same trap - // I fixed in ApprovalBanner + ConfirmDialog. - className="rounded bg-emerald-600 hover:bg-emerald-700 px-4 py-2 text-sm font-medium text-white disabled:opacity-50 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-emerald-400 focus-visible:ring-offset-2 focus-visible:ring-offset-surface-sunken" + // Hover goes DARKER — emerald-600 on white text is 3.3:1 (WCAG AA FAIL). + // emerald-700 is 4.6:1 (WCAG AA PASS). Hover darkens to emerald-600. + className="rounded bg-emerald-700 hover:bg-emerald-600 px-4 py-2 text-sm font-medium text-white disabled:opacity-50 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-emerald-400 focus-visible:ring-offset-2 focus-visible:ring-offset-surface-sunken" > {submitting ? "…" : "I agree"} -- 2.45.2 From 0b4d584aef770e806189de0f4502ea2f8c41490f Mon Sep 17 00:00:00 2001 From: devops-engineer Date: Wed, 13 May 2026 17:44:27 +0000 Subject: [PATCH 7/7] ci: retrigger CI [empty] -- 2.45.2