Merge pull request #2644 from Molecule-AI/a11y/canvas-cookie-consent-not-modal

canvas/CookieConsent: stop claiming aria-modal without a focus trap
This commit is contained in:
Hongming Wang 2026-05-03 22:39:44 +00:00 committed by GitHub
commit 84448d452b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 35 additions and 19 deletions

View File

@ -98,9 +98,17 @@ export function CookieConsent() {
};
return (
<div
role="dialog"
aria-modal="true"
// role="region" + aria-label, NOT role="dialog" + aria-modal. The
// banner is informational — it never blocks the page, never traps
// focus, and the user can keep using the canvas while it's up.
// Claiming aria-modal="true" without a focus trap is genuinely
// harmful for screen-reader users: they get told the rest of the
// page is inert, jump into the banner, and then can't escape.
// Region semantics let assistive tech navigate around it normally.
// (Also: forcing a modal cookie banner would be a dark pattern —
// GDPR explicitly discourages it.)
<section
role="region"
aria-labelledby="cookie-consent-title"
aria-describedby="cookie-consent-body"
className="fixed bottom-0 left-0 right-0 z-[9999] border-t border-line bg-surface/95 backdrop-blur-sm p-4 shadow-[0_-4px_12px_rgba(0,0,0,0.4)]"
@ -117,7 +125,7 @@ export function CookieConsent() {
workspaces). See our{" "}
<a
href="https://moleculesai.app/legal/privacy"
className="text-accent underline hover:text-accent"
className="text-accent underline underline-offset-2 hover:text-accent-strong focus:outline-none focus-visible:ring-2 focus-visible:ring-accent/60 rounded-sm"
target="_blank"
rel="noreferrer"
>
@ -130,20 +138,20 @@ export function CookieConsent() {
<button
type="button"
onClick={() => decide("rejected")}
className="rounded border border-line bg-surface-sunken px-4 py-2 text-sm text-ink hover:bg-surface-card"
className="rounded border border-line bg-surface-sunken px-4 py-2 text-sm text-ink hover:bg-surface-card focus:outline-none focus-visible:ring-2 focus-visible:ring-accent/60 focus-visible:ring-offset-2 focus-visible:ring-offset-surface"
>
Necessary only
</button>
<button
type="button"
onClick={() => decide("accepted")}
className="rounded border border-accent bg-accent-strong px-4 py-2 text-sm font-medium text-white hover:bg-accent"
className="rounded border border-accent bg-accent-strong px-4 py-2 text-sm font-medium text-white hover:bg-accent focus:outline-none focus-visible:ring-2 focus-visible:ring-accent/60 focus-visible:ring-offset-2 focus-visible:ring-offset-surface"
>
Accept all
</button>
</div>
</div>
</div>
</section>
);
}

View File

@ -40,7 +40,7 @@ afterEach(() => {
describe("CookieConsent", () => {
it("renders the banner when no decision is stored", () => {
render(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
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(<CookieConsent />);
expect(screen.queryByRole("dialog")).toBeNull();
expect(screen.queryByRole("region")).toBeNull();
});
});