mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Require accessible names for shared dialogs
Make the shared Dialog prop contract require exactly one accessible-name strategy so new modal call sites cannot silently omit screen-reader context. Record the canonical primitive contract, preserve existing runtime behavior, and cover both valid strategies plus invalid unnamed and ambiguous props. Change-source: pulse-maintainer
This commit is contained in:
@@ -41,6 +41,10 @@ overlay's accessible heading. A reusable panel may suppress its standalone
|
||||
title when the owning overlay supplies the canonical title, while preserving
|
||||
that title in inline and desktop contexts; the overlay remains responsible for
|
||||
one visible heading, its accessible label, dismissal, and focus return.
|
||||
The shared `Dialog` component requires exactly one accessible-name strategy at
|
||||
its component boundary: consumers provide either `ariaLabelledBy` for a visible
|
||||
heading or `ariaLabel` when no visible label is available. Unnamed dialogs and
|
||||
consumers that provide both strategies must fail the frontend type boundary.
|
||||
The alert schedule's initial-delivery selector composes `SettingsPanel` and
|
||||
`FormSelect`, uses the shared alert-configuration presentation vocabulary, and
|
||||
exposes the same email, webhook, Apprise, and all-destination labels used by
|
||||
|
||||
@@ -1,20 +1,16 @@
|
||||
{
|
||||
"version": 1,
|
||||
"base_sha": "44e274e5b386e5e990e9bf09b85b6258043487ba",
|
||||
"verified_at": "2026-09-02T02:59:02Z",
|
||||
"base_sha": "9fba43ffed507f092f876acaf23bef013f0f6fab",
|
||||
"verified_at": "2026-09-02T03:39:02Z",
|
||||
"result": "passed",
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/AppLayout.tsx",
|
||||
"frontend-modern/src/components/Settings/GeneralSettingsPanel.tsx"
|
||||
],
|
||||
"changed_paths": ["frontend-modern/src/components/shared/Dialog.tsx"],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/AppLayout.tsx": "be386bcaa58656a9397fed71a9d70147e2540ff6cb4aff53261d9c03e6ca3834",
|
||||
"frontend-modern/src/components/Settings/GeneralSettingsPanel.tsx": "f1e1c0630cc841ae2820b15130a887aeef0ec7838cb93f652213ebb8fb278199"
|
||||
"frontend-modern/src/components/shared/Dialog.tsx": "185f09e185a9e5ccddf73906a81552ea0cc8b07390fb5ab587fbe1b952697735"
|
||||
},
|
||||
"routes": [
|
||||
"/settings/infrastructure",
|
||||
"/actions",
|
||||
"/alerts/overview",
|
||||
"/settings/infrastructure",
|
||||
"/settings/system-general",
|
||||
"/patrol",
|
||||
"/"
|
||||
@@ -27,20 +23,23 @@
|
||||
{
|
||||
"width": 390,
|
||||
"height": 844
|
||||
},
|
||||
{
|
||||
"width": 393,
|
||||
"height": 851
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"authenticated empty Actions surface at phone width with the RC Preview badge visible, no horizontal overflow, and the skip link focused",
|
||||
"authenticated General Settings surface with Full-width mode off and Outbound usage telemetry on, each exposed as a named pressed-state button",
|
||||
"authenticated Alerts, Infrastructure, General Settings, and Patrol surfaces with reduced motion enabled and no automatically detectable WCAG A/AA violations",
|
||||
"Add infrastructure dialog open with its accessible description, close control focused, and underlying shell retained",
|
||||
"logged-out welcome surface with reduced motion enabled and no automatically detectable WCAG A/AA violations"
|
||||
"Add infrastructure dialog open at desktop and narrow widths with reduced motion, a visible labelled heading, accessible description, focused close control, contained panel geometry, and no horizontal document overflow",
|
||||
"Add infrastructure dialog dismissed at desktop and narrow widths with the underlying Infrastructure surface restored",
|
||||
"authenticated Actions, Alerts, Infrastructure, General Settings, and Patrol surfaces with reduced motion and no automatically detectable WCAG A/AA violations",
|
||||
"logged-out welcome surface with reduced motion and no automatically detectable WCAG A/AA violations"
|
||||
],
|
||||
"interactions": [
|
||||
"tabbed from the document start to Skip to main content at phone width, activated it with Enter, and confirmed focus moved to the main landmark",
|
||||
"inspected the rendered 390x844 Actions screenshot for badge contrast, placement, clipping, scrolling, and bottom-navigation coherence",
|
||||
"navigated the authenticated desktop routes and scanned their rendered states for WCAG A/AA violations and unexpected reduced-motion effects",
|
||||
"opened Add infrastructure, verified initial close-control focus and dialog description, dismissed it with Escape, and confirmed focus returned to Add infrastructure",
|
||||
"opened the logged-out entry surface and verified its heading, disabled welcome/form motion, and accessibility scan"
|
||||
"opened Add infrastructure from its named trigger at desktop and narrow widths and verified the dialog accessible name and description",
|
||||
"inspected final desktop and 390x844 screenshots for placement, clipping, stacking, scrolling, focus treatment, and responsive layout",
|
||||
"verified the dialog bounds stay inside both viewports and the document has no horizontal overflow",
|
||||
"dismissed the dialog with Escape at desktop and narrow widths and verified focus returned to Add infrastructure",
|
||||
"scanned representative authenticated and logged-out surfaces for WCAG A/AA violations and unexpected reduced-motion effects"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -9,19 +9,29 @@ import {
|
||||
} from './dialogModel';
|
||||
import { useDialogState } from './useDialogState';
|
||||
|
||||
interface DialogProps {
|
||||
interface DialogBaseProps {
|
||||
isOpen: boolean;
|
||||
onClose: () => void;
|
||||
children: JSX.Element;
|
||||
panelClass?: string;
|
||||
layout?: DialogLayout;
|
||||
closeOnBackdrop?: boolean;
|
||||
ariaLabel?: string;
|
||||
ariaLabelledBy?: string;
|
||||
ariaDescribedBy?: string;
|
||||
returnFocus?: () => HTMLElement | null | undefined;
|
||||
}
|
||||
|
||||
type DialogProps = DialogBaseProps &
|
||||
(
|
||||
| {
|
||||
ariaLabel: string;
|
||||
ariaLabelledBy?: never;
|
||||
}
|
||||
| {
|
||||
ariaLabel?: never;
|
||||
ariaLabelledBy: string;
|
||||
}
|
||||
);
|
||||
|
||||
export const Dialog: Component<DialogProps> = (props) => {
|
||||
const state = useDialogState(props);
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { afterEach, describe, expect, expectTypeOf, it, vi } from 'vitest';
|
||||
import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library';
|
||||
import { createSignal, Show } from 'solid-js';
|
||||
import type { ComponentProps, JSX } from 'solid-js';
|
||||
import { Dialog } from '@/components/shared/Dialog';
|
||||
import { dialogStackHasBlockingDialog } from '@/components/shared/useDialogState';
|
||||
import dialogSource from '@/components/shared/Dialog.tsx?raw';
|
||||
@@ -53,10 +54,30 @@ describe('Dialog', () => {
|
||||
expect(dialogModelSource).toContain('FOCUSABLE_SELECTOR');
|
||||
});
|
||||
|
||||
it('requires exactly one accessible-name strategy at the component boundary', () => {
|
||||
type UnnamedDialogProps = {
|
||||
isOpen: boolean;
|
||||
onClose: () => void;
|
||||
children: JSX.Element;
|
||||
};
|
||||
type DialogComponentProps = ComponentProps<typeof Dialog>;
|
||||
|
||||
expectTypeOf<UnnamedDialogProps>().not.toMatchTypeOf<DialogComponentProps>();
|
||||
expectTypeOf<
|
||||
UnnamedDialogProps & { ariaLabel: string }
|
||||
>().toMatchTypeOf<DialogComponentProps>();
|
||||
expectTypeOf<
|
||||
UnnamedDialogProps & { ariaLabelledBy: string }
|
||||
>().toMatchTypeOf<DialogComponentProps>();
|
||||
expectTypeOf<
|
||||
UnnamedDialogProps & { ariaLabel: string; ariaLabelledBy: string }
|
||||
>().not.toMatchTypeOf<DialogComponentProps>();
|
||||
});
|
||||
|
||||
it('renders as a modal dialog and closes on backdrop click', () => {
|
||||
const onClose = vi.fn();
|
||||
render(() => (
|
||||
<Dialog isOpen={true} onClose={onClose}>
|
||||
<Dialog isOpen={true} onClose={onClose} ariaLabel="Test dialog">
|
||||
<div class="p-4">
|
||||
<button type="button">Action</button>
|
||||
</div>
|
||||
@@ -76,7 +97,7 @@ describe('Dialog', () => {
|
||||
it('closes on Escape and locks body scroll while open', () => {
|
||||
const onClose = vi.fn();
|
||||
const { unmount } = render(() => (
|
||||
<Dialog isOpen={true} onClose={onClose}>
|
||||
<Dialog isOpen={true} onClose={onClose} ariaLabel="Test dialog">
|
||||
<div class="p-4">Body</div>
|
||||
</Dialog>
|
||||
));
|
||||
@@ -96,7 +117,7 @@ describe('Dialog', () => {
|
||||
expect(dialogStackHasBlockingDialog()).toBe(false);
|
||||
|
||||
const { unmount } = render(() => (
|
||||
<Dialog isOpen={true} onClose={() => undefined}>
|
||||
<Dialog isOpen={true} onClose={() => undefined} ariaLabel="Test dialog">
|
||||
<div class="p-4">Body</div>
|
||||
</Dialog>
|
||||
));
|
||||
@@ -112,7 +133,7 @@ describe('Dialog', () => {
|
||||
document.body.appendChild(background);
|
||||
|
||||
const { unmount } = render(() => (
|
||||
<Dialog isOpen={true} onClose={() => undefined}>
|
||||
<Dialog isOpen={true} onClose={() => undefined} ariaLabel="Test dialog">
|
||||
<button type="button">Dialog action</button>
|
||||
</Dialog>
|
||||
));
|
||||
@@ -133,7 +154,7 @@ describe('Dialog', () => {
|
||||
document.body.appendChild(background);
|
||||
|
||||
const { unmount } = render(() => (
|
||||
<Dialog isOpen={true} onClose={() => undefined}>
|
||||
<Dialog isOpen={true} onClose={() => undefined} ariaLabel="Test dialog">
|
||||
<button type="button">Dialog action</button>
|
||||
</Dialog>
|
||||
));
|
||||
@@ -178,7 +199,7 @@ describe('Dialog', () => {
|
||||
|
||||
it('makes body-level surfaces added while a dialog is open inert', async () => {
|
||||
render(() => (
|
||||
<Dialog isOpen={true} onClose={() => undefined}>
|
||||
<Dialog isOpen={true} onClose={() => undefined} ariaLabel="Test dialog">
|
||||
<button type="button">Dialog action</button>
|
||||
</Dialog>
|
||||
));
|
||||
@@ -194,7 +215,7 @@ describe('Dialog', () => {
|
||||
it('keeps keyboard focus trapped in the dialog', async () => {
|
||||
const onClose = vi.fn();
|
||||
render(() => (
|
||||
<Dialog isOpen={true} onClose={onClose}>
|
||||
<Dialog isOpen={true} onClose={onClose} ariaLabel="Test dialog">
|
||||
<div class="p-4">
|
||||
<button type="button">First</button>
|
||||
<button type="button">Last</button>
|
||||
@@ -294,7 +315,7 @@ describe('Dialog', () => {
|
||||
|
||||
it('honors an explicitly requested initial focus target', async () => {
|
||||
render(() => (
|
||||
<Dialog isOpen={true} onClose={() => undefined}>
|
||||
<Dialog isOpen={true} onClose={() => undefined} ariaLabel="Test dialog">
|
||||
<div class="p-4">
|
||||
<button type="button">Close</button>
|
||||
<textarea aria-label="Outcome" autofocus />
|
||||
@@ -313,7 +334,7 @@ describe('Dialog', () => {
|
||||
<button type="button" onClick={() => setIsOpen(true)}>
|
||||
Open investigation
|
||||
</button>
|
||||
<Dialog isOpen={isOpen()} onClose={() => setIsOpen(false)}>
|
||||
<Dialog isOpen={isOpen()} onClose={() => setIsOpen(false)} ariaLabel="Investigation">
|
||||
<button type="button" onClick={() => setIsOpen(false)}>
|
||||
Close investigation
|
||||
</button>
|
||||
@@ -347,6 +368,7 @@ describe('Dialog', () => {
|
||||
</Show>
|
||||
<Dialog
|
||||
isOpen={isOpen()}
|
||||
ariaLabel="Remove item"
|
||||
onClose={() => {
|
||||
setShowTrigger(false);
|
||||
setIsOpen(false);
|
||||
|
||||
Reference in New Issue
Block a user