diff --git a/dashboard/src/components/AppDialog.test.tsx b/dashboard/src/components/AppDialog.test.tsx new file mode 100644 index 0000000000..9bdd9788bc --- /dev/null +++ b/dashboard/src/components/AppDialog.test.tsx @@ -0,0 +1,87 @@ +import { describe, expect, it, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import AppDialog from "./AppDialog"; + +/** + * A11Y-5 regression guard: a dialog that declares `aria-modal="true"` must + * behave modally for the keyboard too. + * + * The hand-rolled overlays this replaces announced modality to assistive tech + * while leaving focus on the trigger behind them, letting Tab walk into the + * inert page and making Escape a no-op. + */ +describe("AppDialog", () => { + const renderDialog = ( + props: Partial> = {}, + ) => + render( + + + + , + ); + + it("moves focus into the dialog on open", () => { + renderDialog(); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + // First focusable control, not the trigger behind the overlay. + expect(screen.getByRole("button", { name: "first" })).toHaveFocus(); + }); + + it("dismisses on Escape when dismissable", async () => { + const onClose = vi.fn(); + renderDialog({ onClose }); + + await userEvent.keyboard("{Escape}"); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("ignores Escape when not dismissable (e.g. mid-restart)", async () => { + const onClose = vi.fn(); + renderDialog({ onClose, dismissable: false }); + + await userEvent.keyboard("{Escape}"); + expect(onClose).not.toHaveBeenCalled(); + }); + + it("traps Tab: focus stays inside the dialog", async () => { + renderDialog(); + const first = screen.getByRole("button", { name: "first" }); + const last = screen.getByRole("button", { name: "last" }); + + // Tab forward off the last control wraps back to the first. + last.focus(); + await userEvent.tab(); + expect(first).toHaveFocus(); + + // Shift+Tab from the first wraps to the last. + await userEvent.tab({ shift: true }); + expect(last).toHaveFocus(); + }); + + it("restores focus to the trigger on unmount", async () => { + const trigger = document.createElement("button"); + trigger.textContent = "trigger"; + document.body.appendChild(trigger); + trigger.focus(); + + const { unmount } = renderDialog(); + expect(screen.getByRole("button", { name: "first" })).toHaveFocus(); + + unmount(); + expect(trigger).toHaveFocus(); + trigger.remove(); + }); + + it("dismisses on backdrop click but not on inner click", async () => { + const onClose = vi.fn(); + renderDialog({ onClose }); + + await userEvent.click(screen.getByRole("button", { name: "first" })); + expect(onClose).not.toHaveBeenCalled(); + + await userEvent.click(screen.getByRole("dialog")); + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); diff --git a/dashboard/src/components/AppDialog.tsx b/dashboard/src/components/AppDialog.tsx new file mode 100644 index 0000000000..50918fd575 --- /dev/null +++ b/dashboard/src/components/AppDialog.tsx @@ -0,0 +1,138 @@ +import { useCallback, useEffect, useRef, type ReactNode } from "react"; +import { createPortal } from "react-dom"; + +/** + * Modal dialog with the keyboard behaviour `role="dialog" aria-modal="true"` + * promises. + * + * The three hand-rolled overlays (PwaInstallPrompt's IosGuide and + * DesktopInstallGuide, ServiceRestartOverlay) already declare themselves modal, + * but none of them implement it: focus stays on the trigger behind the overlay, + * Tab walks out into the page underneath, Escape does nothing, and closing does + * not return focus. Assistive tech is told the rest of the page is inert while + * the keyboard can still reach it. + * + * Scope is deliberately narrow — focus move on open, Tab trap, Escape, restore on + * close. Layout and styling stay with the caller, so this replaces the shared + * behaviour without dictating appearance. + */ + +/** Elements that can receive focus, in DOM order. */ +const FOCUSABLE = [ + "a[href]", + "button:not([disabled])", + "input:not([disabled])", + "select:not([disabled])", + "textarea:not([disabled])", + '[tabindex]:not([tabindex="-1"])', +].join(","); + +export interface AppDialogProps { + /** Accessible name; required unless the dialog is labelled by its content. */ + "aria-label"?: string; + "aria-labelledby"?: string; + /** `alertdialog` for urgent interruptions (restart, destructive confirm). */ + role?: "dialog" | "alertdialog"; + /** When false, Escape and backdrop clicks do not dismiss (e.g. mid-restart). */ + dismissable?: boolean; + /** Escape key or backdrop click. Omit for a non-dismissable dialog. */ + onClose?: () => void; + /** Extra attributes for the overlay (className, style, aria-busy, …). */ + overlayProps?: React.HTMLAttributes; + /** Class for the overlay element, appended to the default. */ + overlayClassName?: string; + children: ReactNode; +} + +export default function AppDialog({ + role = "dialog", + dismissable = true, + onClose, + overlayProps, + overlayClassName, + children, + ...aria +}: AppDialogProps) { + const overlayRef = useRef(null); + /** The element focused before we opened, so we can put focus back. */ + const restoreRef = useRef(null); + + const focusables = useCallback((): HTMLElement[] => { + const root = overlayRef.current; + if (!root) return []; + // No visibility filter: offsetParent is always null in jsdom (and would + // wrongly drop legitimately focusable controls there), so rely on the + // disabled/type selectors above plus `hidden` to exclude inert nodes. + return Array.from(root.querySelectorAll(FOCUSABLE)).filter( + (el) => + !el.hasAttribute("hidden") && el.getAttribute("aria-hidden") !== "true", + ); + }, []); + + // Move focus in on open, and restore it on close. + useEffect(() => { + restoreRef.current = + document.activeElement instanceof HTMLElement + ? document.activeElement + : null; + + // Prefer the first focusable control; fall back to the overlay itself so + // focus is never left behind on the trigger. + const target = focusables()[0] ?? overlayRef.current; + target?.focus(); + + return () => { + restoreRef.current?.focus?.(); + }; + }, [focusables]); + + const handleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === "Escape" && dismissable) { + e.stopPropagation(); + onClose?.(); + return; + } + if (e.key !== "Tab") return; + + // Trap Tab within the dialog. Without this the browser moves focus to the + // page behind the overlay, which aria-modal has already declared inert. + const items = focusables(); + if (items.length === 0) { + e.preventDefault(); + return; + } + const first = items[0]; + const last = items[items.length - 1]; + const active = document.activeElement; + + if ( + e.shiftKey && + (active === first || !overlayRef.current?.contains(active)) + ) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && active === last) { + e.preventDefault(); + first.focus(); + } + }; + + return createPortal( +
{ + overlayProps?.onClick?.(e); + if (dismissable && e.target === e.currentTarget) onClose?.(); + }} + > + {children} +
, + document.body, + ); +} diff --git a/dashboard/src/components/PwaInstallPrompt/index.tsx b/dashboard/src/components/PwaInstallPrompt/index.tsx index 65e18b3c42..f7a90ad8cf 100644 --- a/dashboard/src/components/PwaInstallPrompt/index.tsx +++ b/dashboard/src/components/PwaInstallPrompt/index.tsx @@ -1,5 +1,5 @@ import { useEffect, useSyncExternalStore, useState } from "react"; -import { createPortal } from "react-dom"; +import AppDialog from "../AppDialog"; import { Tooltip } from "antd"; import { useTranslation } from "react-i18next"; import { Download, MonitorDown, Share, X } from "lucide-react"; @@ -35,15 +35,11 @@ function isStandalone(): boolean { export function IosGuide({ onClose }: { onClose: () => void }) { const { t } = useTranslation(); - return createPortal( -
{ - if (e.target === e.currentTarget) onClose(); - }} + onClose={onClose} >
-
, - document.body, + ); } @@ -89,15 +84,11 @@ export function IosGuide({ onClose }: { onClose: () => void }) { export function DesktopInstallGuide({ onClose }: { onClose: () => void }) { const { t } = useTranslation(); const isEdge = /edg/i.test(navigator.userAgent); - return createPortal( -
{ - if (e.target === e.currentTarget) onClose(); - }} + onClose={onClose} >
-
, - document.body, + ); } diff --git a/dashboard/src/components/ServiceRestartOverlay.tsx b/dashboard/src/components/ServiceRestartOverlay.tsx index 528baadd5a..5ef5557a28 100644 --- a/dashboard/src/components/ServiceRestartOverlay.tsx +++ b/dashboard/src/components/ServiceRestartOverlay.tsx @@ -1,6 +1,6 @@ import { useEffect } from "react"; import type { CSSProperties } from "react"; -import { createPortal } from "react-dom"; +import AppDialog from "./AppDialog"; import { CheckCircle, AlertTriangle, Power } from "lucide-react"; import { Button, Spin } from "antd"; import { useTranslation } from "react-i18next"; @@ -82,13 +82,21 @@ export default function ServiceRestartOverlay({ if (phase === "idle") return null; - return createPortal( -
{phase === "confirm" && ( @@ -153,7 +161,6 @@ export default function ServiceRestartOverlay({ )}
-
, - document.body, + ); } diff --git a/dashboard/src/layouts/Sidebar.tsx b/dashboard/src/layouts/Sidebar.tsx index c4c8efdf95..6c825d5868 100644 --- a/dashboard/src/layouts/Sidebar.tsx +++ b/dashboard/src/layouts/Sidebar.tsx @@ -167,6 +167,7 @@ function NavItemButton({ type="button" className={styles.navItemMain} onClick={() => onNavigate(item.path)} + aria-current={active ? "page" : undefined} style={{ color: active ? "var(--fn-sidebar-item-active-text)" diff --git a/dashboard/src/layouts/SidebarCollapsedIconNav.a11y.test.tsx b/dashboard/src/layouts/SidebarCollapsedIconNav.a11y.test.tsx new file mode 100644 index 0000000000..2e0b0af2c4 --- /dev/null +++ b/dashboard/src/layouts/SidebarCollapsedIconNav.a11y.test.tsx @@ -0,0 +1,70 @@ +import { describe, expect, it, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import SidebarCollapsedIconNav from "./SidebarCollapsedIconNav"; +import type { NavItem } from "./sidebarNav"; + +/** + * A11Y-4 regression guard: the active sidebar entry must expose + * `aria-current="page"`. + * + * The item's active state was previously conveyed by colour alone, so a screen + * reader user got no indication of which section they were in. The name matters + * too: the collapsed rail hides the label in a Tooltip, which is NOT part of the + * accessibility tree, so without `aria-label` each button announces as "button". + */ + +// Tooltip renders a portal; the button itself is what we assert on. +const items: NavItem[] = [ + { key: "chat", path: "/chat", icon: c, labelKey: "nav.chat" }, + { + key: "experts", + path: "/experts", + icon: e, + labelKey: "nav.experts", + }, +]; + +/** Minimal t() that returns the key, so names are predictable. */ +const t = ((key: string) => key) as never; + +function renderNav(selectedKey: string) { + return render( + , + ); +} + +describe("SidebarCollapsedIconNav accessibility", () => { + it("marks the selected item with aria-current=page", () => { + renderNav("experts"); + + const active = screen.getByRole("button", { current: "page" }); + expect(active).toHaveAttribute("aria-current", "page"); + expect(active).toHaveAccessibleName("nav.experts"); + }); + + it("leaves non-selected items without aria-current", () => { + renderNav("experts"); + + const chat = screen.getByRole("button", { name: "nav.chat" }); + expect(chat).not.toHaveAttribute("aria-current"); + }); + + it("gives every collapsed item an accessible name", () => { + renderNav("chat"); + + // Would throw if the icon-only buttons had no accessible name. + expect( + screen.getByRole("button", { name: "nav.chat" }), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "nav.experts" }), + ).toBeInTheDocument(); + }); +}); diff --git a/dashboard/src/layouts/SidebarCollapsedIconNav.tsx b/dashboard/src/layouts/SidebarCollapsedIconNav.tsx index 82f6507eb1..201c5cfa7a 100644 --- a/dashboard/src/layouts/SidebarCollapsedIconNav.tsx +++ b/dashboard/src/layouts/SidebarCollapsedIconNav.tsx @@ -41,6 +41,10 @@ export default function SidebarCollapsedIconNav({