From 9504e04dab45d3c6a24564f2be859006e6212060 Mon Sep 17 00:00:00 2001 From: zodyp Date: Mon, 14 Sep 2026 07:56:56 -0300 Subject: [PATCH 01/10] refactor(a11y): share the modal keyboard contract through useDialogFocus Extract Modal's Escape handler, initial focus, Tab trap and focus restore into src/shared/hooks/useDialogFocus.ts so slide-over drawers can reuse the exact same contract. The trap now skips disabled controls: a disabled last button refused focus and let Tab escape the dialog. Tests: tests/unit/shared/components/Modal.a11y.test.tsx (dialog name, aria-modal, named close button, initial focus, Tab wrap skipping disabled, Escape); tests/unit/ui/modal-focus-restoration.test.tsx unchanged and green. Co-Authored-By: Claude Opus 5 --- src/shared/components/Modal.tsx | 72 ++-------------- src/shared/hooks/useDialogFocus.ts | 85 +++++++++++++++++++ .../shared/components/Modal.a11y.test.tsx | 78 +++++++++++++++++ 3 files changed, 168 insertions(+), 67 deletions(-) create mode 100644 src/shared/hooks/useDialogFocus.ts create mode 100644 tests/unit/shared/components/Modal.a11y.test.tsx diff --git a/src/shared/components/Modal.tsx b/src/shared/components/Modal.tsx index 61f361714e4..7e2f849ea71 100644 --- a/src/shared/components/Modal.tsx +++ b/src/shared/components/Modal.tsx @@ -3,6 +3,7 @@ import { useEffect, useRef, useId } from "react"; import { useTranslations } from "next-intl"; import { cn } from "@/shared/utils/cn"; +import { useDialogFocus } from "@/shared/hooks/useDialogFocus"; import Button, { type ButtonVariant } from "./Button"; // #6265 — preset for content-heavy modals: caps height on the OUTERMOST dialog @@ -55,8 +56,7 @@ export default function Modal({ }: ModalProps) { const t = useTranslations("common"); const titleId = useId(); - const dialogRef = useRef(null); - const previouslyFocusedRef = useRef(null); + const dialogRef = useRef(null); const sizes = { sm: "max-w-sm", @@ -78,71 +78,9 @@ export default function Modal({ }; }, [isOpen]); - // Handle escape key - useEffect(() => { - const handleEscape = (e) => { - if (e.key === "Escape" && isOpen) { - onClose(); - } - }; - document.addEventListener("keydown", handleEscape); - return () => document.removeEventListener("keydown", handleEscape); - }, [isOpen, onClose]); - - // Return keyboard users to the control that opened the dialog. - useEffect(() => { - if (!isOpen) return; - - const activeElement = document.activeElement; - previouslyFocusedRef.current = activeElement instanceof HTMLElement ? activeElement : null; - - return () => { - const previouslyFocused = previouslyFocusedRef.current; - previouslyFocusedRef.current = null; - if (previouslyFocused?.isConnected) { - previouslyFocused.focus(); - } - }; - }, [isOpen]); - - // Focus trap - useEffect(() => { - if (!isOpen || !dialogRef.current) return; - - const dialog = dialogRef.current; - const focusableSelector = - 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'; - - // Focus first focusable element - const firstFocusable = dialog.querySelector(focusableSelector); - const focusTimer = firstFocusable - ? window.setTimeout(() => firstFocusable.focus(), 50) - : undefined; - - const handleTab = (e) => { - if (e.key !== "Tab") return; - - const focusable = [...dialog.querySelectorAll(focusableSelector)]; - if (focusable.length === 0) return; - - const first = focusable[0]; - const last = focusable[focusable.length - 1]; - - if (e.shiftKey && document.activeElement === first) { - e.preventDefault(); - last.focus(); - } else if (!e.shiftKey && document.activeElement === last) { - e.preventDefault(); - first.focus(); - } - }; - - dialog.addEventListener("keydown", handleTab); - return () => { - if (focusTimer !== undefined) window.clearTimeout(focusTimer); - dialog.removeEventListener("keydown", handleTab); - }; - }, [isOpen]); + // Escape to close, initial focus, Tab trap (skipping disabled controls) and focus + // restore to the opener — shared with the slide-over drawers. + useDialogFocus(dialogRef, isOpen, onClose); if (!isOpen) return null; diff --git a/src/shared/hooks/useDialogFocus.ts b/src/shared/hooks/useDialogFocus.ts new file mode 100644 index 00000000000..bce9f9042d1 --- /dev/null +++ b/src/shared/hooks/useDialogFocus.ts @@ -0,0 +1,85 @@ +"use client"; + +import { useEffect, type RefObject } from "react"; + +// Disabled controls cannot receive focus, so leaving them in the list made the trap +// "wrap" onto an element that silently refused focus and let Tab escape the dialog. +const FOCUSABLE_SELECTOR = [ + "a[href]", + "button:not([disabled])", + "input:not([disabled]):not([type='hidden'])", + "select:not([disabled])", + "textarea:not([disabled])", + "[tabindex]:not([tabindex='-1'])", +].join(", "); + +const INITIAL_FOCUS_DELAY_MS = 50; + +function getFocusableElements(container: HTMLElement): HTMLElement[] { + return Array.from(container.querySelectorAll(FOCUSABLE_SELECTOR)); +} + +function trapTabKey(dialog: HTMLElement, event: KeyboardEvent) { + if (event.key !== "Tab") return; + const focusable = getFocusableElements(dialog); + if (focusable.length === 0) return; + + const first = focusable[0]; + const last = focusable[focusable.length - 1]; + if (event.shiftKey && document.activeElement === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } +} + +/** + * Keyboard contract shared by every modal surface (centered Modal, slide-over drawer): + * - Escape closes; + * - the first focusable control receives focus when the dialog opens; + * - Tab / Shift+Tab stay inside the dialog; + * - focus returns to the opener on close (only if the opener is still in the DOM). + */ +export function useDialogFocus( + dialogRef: RefObject, + isOpen: boolean, + onClose: () => void +) { + useEffect(() => { + if (!isOpen) return; + const handleEscape = (event: KeyboardEvent) => { + if (event.key === "Escape") onClose(); + }; + document.addEventListener("keydown", handleEscape); + return () => document.removeEventListener("keydown", handleEscape); + }, [isOpen, onClose]); + + // Declared before the focus effect so the opener is captured before focus moves. + useEffect(() => { + if (!isOpen) return; + const activeElement = document.activeElement; + const opener = activeElement instanceof HTMLElement ? activeElement : null; + return () => { + if (opener?.isConnected) opener.focus(); + }; + }, [isOpen]); + + useEffect(() => { + const dialog = dialogRef.current; + if (!isOpen || !dialog) return; + + const firstFocusable = getFocusableElements(dialog)[0]; + const focusTimer = firstFocusable + ? window.setTimeout(() => firstFocusable.focus(), INITIAL_FOCUS_DELAY_MS) + : undefined; + const handleTab = (event: KeyboardEvent) => trapTabKey(dialog, event); + + dialog.addEventListener("keydown", handleTab); + return () => { + if (focusTimer !== undefined) window.clearTimeout(focusTimer); + dialog.removeEventListener("keydown", handleTab); + }; + }, [isOpen, dialogRef]); +} diff --git a/tests/unit/shared/components/Modal.a11y.test.tsx b/tests/unit/shared/components/Modal.a11y.test.tsx new file mode 100644 index 00000000000..e6de684de0e --- /dev/null +++ b/tests/unit/shared/components/Modal.a11y.test.tsx @@ -0,0 +1,78 @@ +// @vitest-environment jsdom +import React from "react"; +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import Modal from "@/shared/components/Modal"; + +function renderModal(onClose = vi.fn()) { + render( + + + + + } + > + + + ); + return onClose; +} + +describe("Modal a11y contract", () => { + it("is a modal dialog named by its title", () => { + renderModal(); + const dialog = screen.getByRole("dialog", { name: "Edit connection" }); + + expect(dialog).toHaveAttribute("aria-modal", "true"); + }); + + it("gives the icon-only close button an accessible name and hides its glyph", () => { + renderModal(); + const close = screen.getByRole("button", { name: "Close" }); + + expect(close.querySelector(".material-symbols-outlined")).toHaveAttribute( + "aria-hidden", + "true" + ); + }); + + it("moves focus into the dialog when it opens", async () => { + renderModal(); + const dialog = screen.getByRole("dialog"); + + await waitFor(() => expect(dialog.contains(document.activeElement)).toBe(true)); + }); + + it("wraps Tab from the last enabled control back to the first, skipping disabled ones", async () => { + const user = userEvent.setup(); + renderModal(); + const save = screen.getByRole("button", { name: "Save" }); + const close = screen.getByRole("button", { name: "Close" }); + // Let the deferred initial focus land first so it cannot race the manual focus below. + await waitFor(() => expect(close).toHaveFocus()); + + save.focus(); + await user.tab(); + expect(close).toHaveFocus(); + + await user.tab({ shift: true }); + expect(save).toHaveFocus(); + }); + + it("closes on Escape", async () => { + const user = userEvent.setup(); + const onClose = renderModal(); + + await user.keyboard("{Escape}"); + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); From 43c965b15ea1a9c4e51149adf9e4725b93051c8a Mon Sep 17 00:00:00 2001 From: zodyp Date: Mon, 14 Sep 2026 07:56:58 -0300 Subject: [PATCH 02/10] fix(a11y): label Checkbox explicitly and describe Toggle switches Checkbox generates an id when none is passed so the visible label keeps an explicit htmlFor association. Toggle links its visible description through aria-describedby when a label or ariaLabel already names the switch (no double announcement when the description is the name). Tests: tests/unit/shared/components/Checkbox.a11y.test.tsx and tests/unit/shared/components/Toggle.a11y.test.tsx (role=switch, aria-checked, Space/Enter, description, disabled). Co-Authored-By: Claude Opus 5 --- src/shared/components/Checkbox.tsx | 10 ++- src/shared/components/Toggle.tsx | 13 +++- .../shared/components/Checkbox.a11y.test.tsx | 65 ++++++++++++++++ .../shared/components/Toggle.a11y.test.tsx | 76 +++++++++++++++++++ 4 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 tests/unit/shared/components/Checkbox.a11y.test.tsx create mode 100644 tests/unit/shared/components/Toggle.a11y.test.tsx diff --git a/src/shared/components/Checkbox.tsx b/src/shared/components/Checkbox.tsx index 56fc423b6c4..c427cf5fe2f 100644 --- a/src/shared/components/Checkbox.tsx +++ b/src/shared/components/Checkbox.tsx @@ -1,5 +1,6 @@ "use client"; +import { useId } from "react"; import { cn } from "@/shared/utils/cn"; interface CheckboxProps extends React.InputHTMLAttributes { @@ -10,12 +11,17 @@ interface CheckboxProps extends React.InputHTMLAttributes { * Checkbox — token-driven native checkbox (brand accent + keyboard focus ring). * Replaces the ad-hoc `` * pattern scattered across the dashboard. Optional `label` wraps it in a clickable row. + * Without a `label`, pass `aria-label` (or `aria-labelledby`) so the box has a name. */ export default function Checkbox({ label, className, id, ...props }: CheckboxProps) { + const generatedId = useId(); + // The label wraps the input, but an explicit id/htmlFor pair keeps the association + // intact for assistive tech that ignores implicit wrapping. + const inputId = id ?? generatedId; const box = ( {box} diff --git a/src/shared/components/Toggle.tsx b/src/shared/components/Toggle.tsx index ffac728de7c..4ef5af765ab 100644 --- a/src/shared/components/Toggle.tsx +++ b/src/shared/components/Toggle.tsx @@ -1,5 +1,6 @@ "use client"; +import { useId } from "react"; import { cn } from "@/shared/utils/cn"; interface ToggleProps { @@ -48,6 +49,11 @@ export default function Toggle({ }, }; + const descriptionId = useId(); + // The description is announced as a description only when something else names the + // switch; with no label it already is the name, so it must not be read twice. + const describedBy = description && (ariaLabel || label) ? descriptionId : undefined; + const handleClick = () => { if (!disabled && onChange) { onChange(!checked); @@ -67,6 +73,7 @@ export default function Toggle({ role="switch" aria-checked={checked} aria-label={ariaLabel || label || description || title || "Toggle"} + aria-describedby={describedBy} title={title} disabled={disabled} onClick={handleClick} @@ -94,7 +101,11 @@ export default function Toggle({ {(label || description) && (
{label && {label}} - {description && {description}} + {description && ( + + {description} + + )}
)} diff --git a/tests/unit/shared/components/Checkbox.a11y.test.tsx b/tests/unit/shared/components/Checkbox.a11y.test.tsx new file mode 100644 index 00000000000..f82c941b636 --- /dev/null +++ b/tests/unit/shared/components/Checkbox.a11y.test.tsx @@ -0,0 +1,65 @@ +// @vitest-environment jsdom +import React, { useState } from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it } from "vitest"; + +import Checkbox from "@/shared/components/Checkbox"; + +function ControlledCheckbox() { + const [checked, setChecked] = useState(false); + return ( + setChecked(event.target.checked)} + /> + ); +} + +describe("Checkbox a11y contract", () => { + it("explicitly associates the visible label with the native checkbox when no id is passed", () => { + render(); + + const box = screen.getByRole("checkbox", { name: "Enable cache" }); + const label = screen.getByText("Enable cache").closest("label"); + + expect(box.id).not.toBe(""); + expect(label?.htmlFor).toBe(box.id); + }); + + it("keeps a caller-provided id for the label association", () => { + render(); + + const box = screen.getByRole("checkbox", { name: "Enable cache" }); + expect(box.id).toBe("cache-toggle"); + expect(screen.getByText("Enable cache").closest("label")?.htmlFor).toBe("cache-toggle"); + }); + + it("uses aria-label as the accessible name when rendered without a visible label", () => { + render(); + + expect(screen.getByRole("checkbox", { name: "Select row 1" })).toBeInTheDocument(); + }); + + it("is reachable with Tab and toggles its checked state with Space", async () => { + const user = userEvent.setup(); + render(); + const box = screen.getByRole("checkbox", { name: "Enable cache" }); + + await user.tab(); + expect(box).toHaveFocus(); + expect(box).not.toBeChecked(); + + await user.keyboard(" "); + expect(box).toBeChecked(); + }); + + it("toggles when the label text is clicked", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByText("Enable cache")); + expect(screen.getByRole("checkbox", { name: "Enable cache" })).toBeChecked(); + }); +}); diff --git a/tests/unit/shared/components/Toggle.a11y.test.tsx b/tests/unit/shared/components/Toggle.a11y.test.tsx new file mode 100644 index 00000000000..cad452e723a --- /dev/null +++ b/tests/unit/shared/components/Toggle.a11y.test.tsx @@ -0,0 +1,76 @@ +// @vitest-environment jsdom +import React, { useState } from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import Toggle from "@/shared/components/Toggle"; + +function ControlledToggle(props: { label?: string; description?: string; ariaLabel?: string }) { + const [checked, setChecked] = useState(false); + return ; +} + +describe("Toggle a11y contract", () => { + it("exposes role=switch with aria-checked reflecting the state", async () => { + const user = userEvent.setup(); + render(); + const toggle = screen.getByRole("switch", { name: "Auto retry" }); + + expect(toggle).toHaveAttribute("aria-checked", "false"); + await user.click(toggle); + expect(toggle).toHaveAttribute("aria-checked", "true"); + }); + + it("is keyboard operable with Space and Enter", async () => { + const user = userEvent.setup(); + render(); + const toggle = screen.getByRole("switch", { name: "Auto retry" }); + + await user.tab(); + expect(toggle).toHaveFocus(); + await user.keyboard(" "); + expect(toggle).toHaveAttribute("aria-checked", "true"); + await user.keyboard("{Enter}"); + expect(toggle).toHaveAttribute("aria-checked", "false"); + }); + + it("names the switch from its visible label and announces the description", () => { + render(); + const toggle = screen.getByRole("switch", { name: "Auto retry" }); + + expect(toggle).toHaveAccessibleDescription("Retry failed requests once"); + }); + + it("keeps an explicit ariaLabel as the name and still announces the description", () => { + render( + + ); + const toggle = screen.getByRole("switch", { name: "Enable provider Acme" }); + + expect(toggle).toHaveAccessibleDescription("Routes traffic to Acme"); + }); + + it("falls back to the description as the name when there is no label", () => { + render(); + + expect(screen.getByRole("switch", { name: "Compact mode" })).not.toHaveAttribute( + "aria-describedby" + ); + }); + + it("does not change state while disabled", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + render(); + const toggle = screen.getByRole("switch", { name: "Locked" }); + + expect(toggle).toBeDisabled(); + await user.click(toggle); + expect(onChange).not.toHaveBeenCalled(); + }); +}); From 44f01b32fd0e38721c1ef7f667177f76e633c8e9 Mon Sep 17 00:00:00 2001 From: zodyp Date: Mon, 14 Sep 2026 07:56:59 -0300 Subject: [PATCH 03/10] fix(a11y): make sidebar section headers and collapse control operable axe serious violations on /dashboard and /dashboard/providers: - nested-interactive: section headers were div[role=button] wrapping the pin button and could not be reached from the keyboard. SidebarSectionHeader renders the expand control and the pin control as sibling buttons (aria-expanded, aria-pressed), same visual layout. - aria-hidden-focus: the collapse button lived inside an aria-hidden container; only the decorative window dots are hidden now. Collapsed (icon-only) nav links get aria-label, restart/shutdown get explicit names, and Material Symbols glyphs are aria-hidden so ligature text is not read. Tests: tests/unit/shared/components/SidebarSectionHeader.a11y.test.tsx and tests/unit/shared/components/Sidebar.a11y.test.tsx. Co-Authored-By: Claude Opus 5 --- src/shared/components/Sidebar.tsx | 77 ++++++----------- .../components/SidebarSectionHeader.tsx | 77 +++++++++++++++++ .../shared/components/Sidebar.a11y.test.tsx | 84 +++++++++++++++++++ .../SidebarSectionHeader.a11y.test.tsx | 75 +++++++++++++++++ 4 files changed, 262 insertions(+), 51 deletions(-) create mode 100644 src/shared/components/SidebarSectionHeader.tsx create mode 100644 tests/unit/shared/components/Sidebar.a11y.test.tsx create mode 100644 tests/unit/shared/components/SidebarSectionHeader.a11y.test.tsx diff --git a/src/shared/components/Sidebar.tsx b/src/shared/components/Sidebar.tsx index 0ab117604e2..7df7a0f83bf 100644 --- a/src/shared/components/Sidebar.tsx +++ b/src/shared/components/Sidebar.tsx @@ -25,6 +25,7 @@ import Button from "./Button"; import Input from "./Input"; import { ConfirmModal } from "./Modal"; import CloudSyncStatus from "./CloudSyncStatus"; +import SidebarSectionHeader from "./SidebarSectionHeader"; import { useTranslations } from "next-intl"; import { HIDDEN_SIDEBAR_GROUP_LABELS_SETTING_KEY, @@ -465,7 +466,7 @@ export default function Sidebar({ ); const content = ( <> - + {!collapsed && ( @@ -481,6 +482,8 @@ export default function Sidebar({ const sharedProps = { onMouseEnter: (e: React.MouseEvent) => handleMouseEnter(e, item.id, item.label), onMouseLeave: handleMouseLeave, + // Collapsed (mini) links show only a glyph: name them by their label. + "aria-label": collapsed ? item.label : undefined, }; if (item.external) { @@ -537,13 +540,15 @@ export default function Sidebar({ isMacElectron ? "pt-3" : "pt-5", collapsed ? "px-3 justify-center" : "px-4" )} - aria-hidden="true" > + {/* Only the decorative window dots are hidden: the collapse button beside them + must stay reachable (a focusable control inside aria-hidden is invisible to + screen readers yet still takes keyboard focus). */} {!isMacElectron && ( <> -
-
-
+