From 652124284b33cbf40c9b9ce592a71cad7baf659b Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 3 May 2026 15:37:06 -0700 Subject: [PATCH] canvas/CookieConsent: stop pretending to be a modal + fix link/button focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes for the cookie banner: 1. role="dialog" aria-modal="true" →
. The banner has no focus trap, doesn't block the page, and the user can keep using the canvas while it's up — none of which are modal semantics. Claiming aria-modal="true" without a trap actively harms screen-reader users: they're told the rest of the page is inert, jump into the banner, and then can't escape. Region semantics let AT navigate around it normally. (Forcing a modal cookie banner would also be a dark pattern under GDPR.) 2. Privacy-policy link: hover:text-accent → hover:text-accent-strong. The original was a no-op (same color). Also added focus-visible ring + underline-offset so the link is readable AND keyboard- distinguishable in both themes. 3. Both buttons: focus-visible:ring-2 + ring-offset-surface so keyboard users see where focus lands. Mouse clicks unchanged thanks to focus-visible. Tests: swapped getByRole("dialog") → getByRole("region") in 8 existing tests, then tightened the role-assertion test into a regression guard that explicitly asserts NO aria-modal and NO dialog role exist. Full suite: 1220/1220. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/CookieConsent.tsx | 22 +++++++++---- .../__tests__/CookieConsent.test.tsx | 32 ++++++++++++------- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/canvas/src/components/CookieConsent.tsx b/canvas/src/components/CookieConsent.tsx index 6b607b04..898ffef1 100644 --- a/canvas/src/components/CookieConsent.tsx +++ b/canvas/src/components/CookieConsent.tsx @@ -98,9 +98,17 @@ export function CookieConsent() { }; return ( -
@@ -130,20 +138,20 @@ export function CookieConsent() {
- +
); } diff --git a/canvas/src/components/__tests__/CookieConsent.test.tsx b/canvas/src/components/__tests__/CookieConsent.test.tsx index 188c6f9c..66012a17 100644 --- a/canvas/src/components/__tests__/CookieConsent.test.tsx +++ b/canvas/src/components/__tests__/CookieConsent.test.tsx @@ -40,7 +40,7 @@ afterEach(() => { describe("CookieConsent", () => { it("renders the banner when no decision is stored", () => { render(); - expect(screen.getByRole("dialog")).toBeTruthy(); + expect(screen.getByRole("region")).toBeTruthy(); expect(screen.getByRole("button", { name: "Accept all" })).toBeTruthy(); expect(screen.getByRole("button", { name: "Necessary only" })).toBeTruthy(); }); @@ -48,7 +48,7 @@ describe("CookieConsent", () => { it("stores 'accepted' and hides the banner when user clicks Accept all", () => { render(); fireEvent.click(screen.getByRole("button", { name: "Accept all" })); - expect(screen.queryByRole("dialog")).toBeNull(); + expect(screen.queryByRole("region")).toBeNull(); const raw = window.localStorage.getItem(STORAGE_KEY); expect(raw).not.toBeNull(); @@ -61,7 +61,7 @@ describe("CookieConsent", () => { it("stores 'rejected' and hides the banner when user clicks Necessary only", () => { render(); fireEvent.click(screen.getByRole("button", { name: "Necessary only" })); - expect(screen.queryByRole("dialog")).toBeNull(); + expect(screen.queryByRole("region")).toBeNull(); const parsed = JSON.parse(window.localStorage.getItem(STORAGE_KEY)!); expect(parsed.decision).toBe("rejected"); @@ -73,7 +73,7 @@ describe("CookieConsent", () => { JSON.stringify({ decision: "accepted", decidedAt: new Date().toISOString(), version: 1 }), ); render(); - expect(screen.queryByRole("dialog")).toBeNull(); + expect(screen.queryByRole("region")).toBeNull(); }); it("re-prompts when the stored decision is on an older policy version", () => { @@ -82,13 +82,13 @@ describe("CookieConsent", () => { JSON.stringify({ decision: "accepted", decidedAt: new Date().toISOString(), version: 0 }), ); render(); - expect(screen.getByRole("dialog")).toBeTruthy(); + expect(screen.getByRole("region")).toBeTruthy(); }); it("re-prompts when localStorage contains invalid JSON", () => { window.localStorage.setItem(STORAGE_KEY, "{not json"); render(); - expect(screen.getByRole("dialog")).toBeTruthy(); + expect(screen.getByRole("region")).toBeTruthy(); }); it("exposes a privacy-policy link with target='_blank'", () => { @@ -99,11 +99,19 @@ describe("CookieConsent", () => { expect(link.getAttribute("rel")).toContain("noreferrer"); }); - it("uses role=dialog with aria-labelledby and aria-describedby for screen readers", () => { + it("uses role=region (NOT dialog) with aria-labelledby/describedby — banner is informational, not modal", () => { + // Regression guard: an earlier version claimed role="dialog" + // aria-modal="true" without a focus trap. That falsely told screen + // readers the rest of the page was inert, trapping AT users in a + // banner they couldn't escape. role="region" lets assistive tech + // navigate around it normally; the banner stays informational. render(); - const dialog = screen.getByRole("dialog"); - expect(dialog.getAttribute("aria-labelledby")).toBe("cookie-consent-title"); - expect(dialog.getAttribute("aria-describedby")).toBe("cookie-consent-body"); + const banner = screen.getByRole("region"); + expect(banner.getAttribute("aria-labelledby")).toBe("cookie-consent-title"); + expect(banner.getAttribute("aria-describedby")).toBe("cookie-consent-body"); + // No aria-modal claim — explicit guard against regression. + expect(banner.getAttribute("aria-modal")).toBeNull(); + expect(screen.queryByRole("dialog")).toBeNull(); }); it("does NOT render on local dev (non-SaaS hostname)", () => { @@ -116,7 +124,7 @@ describe("CookieConsent", () => { value: { ...window.location, hostname: "localhost" }, }); render(); - expect(screen.queryByRole("dialog")).toBeNull(); + expect(screen.queryByRole("region")).toBeNull(); }); it("does NOT render on a LAN hostname (192.168.*, *.local)", () => { @@ -125,7 +133,7 @@ describe("CookieConsent", () => { value: { ...window.location, hostname: "192.168.1.74" }, }); render(); - expect(screen.queryByRole("dialog")).toBeNull(); + expect(screen.queryByRole("region")).toBeNull(); }); });