diff --git a/CHANGELOG.md b/CHANGELOG.md index a815eaf..c310bba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,46 @@ All notable changes to `@ngbracket/a11y-devtools` are documented here. This project adheres to [Semantic Versioning](https://semver.org/). +## 0.17.0 + +### Added + +- New keyboard rule `ngbr/drag-without-keyboard`. It reports a list whose items + move by drag and drop when the tool finds no keyboard way to move them. + Angular CDK drag and drop has no keyboard support of its own, so a list of + rows with `cdkDragHandle` icons is mouse-only unless the app adds one. The + rule finds drag items by the `cdk-drag` class inside a `cdk-drop-list`, and + by `draggable="true"` when a parent holds two or more, in any build. It + reports each list once. It skips a list with a tab stop in or next to it + whose name says it moves an item ("Move up", "Reorder", an arrow, a Material + `arrow_upward` icon). Serious when the keyboard can't reach a drag handle of a + CDK list; moderate for a native `draggable` list. A handle or item with a + `tabindex` counts as reachable when a drag list on the page has a tab stop, + as in a board where one card is the tab stop and the arrow keys move to the + rest. Moderate in a dev build when a handle is reachable but no Angular key + listener sits on the handle, its item, the list, or the elements up to and + including the component that holds the list. Maps to WCAG 2.1.1 and + 2.5.7 in the ACR worksheet. + +### Fixed + +- The tab-order layer now hides the page's tab stops while an Angular Material + dialog is open. Material 22 opens `mat-dialog` with `aria-modal="false"` by + default and hides the page from screen readers with `aria-hidden`. The + scanner only looked for the CDK focus trap around `aria-modal="true"` + elements, so it missed the trap and numbered the page's stops under the + dialog as well as the dialog's own. It now finds the trap around any + `mat-dialog-container`, `.cdk-dialog-container` or dialog role. Report mode + also no longer counts Tab cycling inside a default `mat-dialog` as a + `ngbr/focus-trap` finding. +- The scanner now recognises the focus traps in PrimeNG, ngx-bootstrap and + ng-bootstrap dialogs. PrimeNG and ngx-bootstrap put their sentinels inside + the dialog, as its first and last tab stops. ng-bootstrap's modal and + offcanvas trap Tab with a script and no sentinels, so they're recognised by + element name. Before, the tab-order layer showed the page's stops under these + dialogs, and `ngbr/modal-focus-not-contained` reported a leak that wasn't + there. + ## 0.16.0 ### Added diff --git a/e2e/angular-app/src/app.ts b/e2e/angular-app/src/app.ts index 6101439..99d55d6 100644 --- a/e2e/angular-app/src/app.ts +++ b/e2e/angular-app/src/app.ts @@ -3,7 +3,8 @@ * E2E spec can check that findings are attributed to the right component * through the real `window.ng` debug API. */ -import { Component, Directive } from '@angular/core'; +import { Dialog } from '@angular/cdk/dialog'; +import { afterNextRender, Component, Directive, inject } from '@angular/core'; const PIXEL = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='; @@ -57,6 +58,17 @@ export class FancyDirective {} ⓘ + + + `, }) export class HomePageComponent { @@ -71,14 +83,39 @@ export class HomePageComponent { }) export class SettingsPageComponent {} +/** The content of the real CDK dialog that DialogPageComponent opens. */ +@Component({ + selector: 'app-dialog-content', + template: `

Edit level

`, +}) +export class DialogContentComponent {} + +/** + * Opens a real Angular CDK dialog on load. CDK and Material dialogs default to + * aria-modal="false" and put their focus-trap anchors beside the container, so + * this checks the tab-order layer shows only the dialog's stops. + */ +@Component({ + selector: 'app-dialog-page', + template: `

Dialog

`, +}) +export class DialogPageComponent { + private readonly dialog = inject(Dialog); + constructor() { + afterNextRender(() => this.dialog.open(DialogContentComponent)); + } +} + @Component({ selector: 'app-root', - imports: [HeaderComponent, HomePageComponent, SettingsPageComponent], + imports: [HeaderComponent, HomePageComponent, SettingsPageComponent, DialogPageComponent], template: `
@if (page === 'settings') { + } @else if (page === 'dialog') { + } @else { } @@ -87,5 +124,9 @@ export class SettingsPageComponent {} }) export class AppComponent { // No router needed: the route picks the page. - page = location.pathname.startsWith('/settings') ? 'settings' : 'home'; + page = location.pathname.startsWith('/settings') + ? 'settings' + : location.pathname.startsWith('/dialog') + ? 'dialog' + : 'home'; } diff --git a/package-lock.json b/package-lock.json index 5f4ae77..a6ddc04 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@ngbracket/a11y-devtools", - "version": "0.16.0", + "version": "0.17.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@ngbracket/a11y-devtools", - "version": "0.16.0", + "version": "0.17.0", "license": "MIT", "dependencies": { "axe-core": "^4.13.0" @@ -18,10 +18,12 @@ "@analogjs/vite-plugin-angular": "^2.8.0", "@analogjs/vitest-angular": "^2.8.0", "@angular/build": "^22.2.1", + "@angular/cdk": "^22.2.1", "@angular/common": "^22.2.1", "@angular/compiler": "^22.2.1", "@angular/compiler-cli": "^22.2.1", "@angular/core": "^22.2.1", + "@angular/forms": "^22.2.1", "@angular/platform-browser": "^22.2.1", "@angular/platform-browser-dynamic": "^22.2.1", "@emnapi/core": "^1.11.3", @@ -814,6 +816,24 @@ "url": "https://github.com/sponsors/jonschlinkert" } }, + "node_modules/@angular/cdk": { + "version": "22.2.1", + "resolved": "https://registry.npmjs.org/@angular/cdk/-/cdk-22.2.1.tgz", + "integrity": "sha512-0vjlqYBbLif1EwHhKz2r4thTDvtRI2CKi2js9TzPRD0n4VlFUnAXaaW47s5TCCObIoYun6Bo1zZ0uPgwb+qEMw==", + "dev": true, + "license": "MIT", + "dependencies": { + "parse5": "^8.0.0", + "tslib": "^2.3.0" + }, + "peerDependencies": { + "@angular/common": "^22.0.0 || ^23.0.0", + "@angular/core": "^22.0.0 || ^23.0.0", + "@angular/forms": "^22.0.0 || ^23.0.0", + "@angular/platform-browser": "^22.0.0 || ^23.0.0", + "rxjs": "^6.5.3 || ^7.4.0" + } + }, "node_modules/@angular/common": { "version": "22.2.1", "resolved": "https://registry.npmjs.org/@angular/common/-/common-22.2.1.tgz", @@ -903,6 +923,27 @@ } } }, + "node_modules/@angular/forms": { + "version": "22.2.1", + "resolved": "https://registry.npmjs.org/@angular/forms/-/forms-22.2.1.tgz", + "integrity": "sha512-xHM+PhrvYDJ+edPxh6PWhkE99tXWbsMcxdXCWF6QcyByX+bsW2rVlNZ7dumOsUle9MYa8Yj0lZFACUWbwFw+4w==", + "dev": true, + "license": "MIT", + "dependencies": { + "@standard-schema/spec": "^1.0.0", + "tslib": "^2.3.0", + "zod": "^4.0.10" + }, + "engines": { + "node": "^22.22.3 || ^24.15.0 || >=26.0.0" + }, + "peerDependencies": { + "@angular/common": "22.2.1", + "@angular/core": "22.2.1", + "@angular/platform-browser": "22.2.1", + "rxjs": "^6.5.3 || ^7.4.0" + } + }, "node_modules/@angular/platform-browser": { "version": "22.2.1", "resolved": "https://registry.npmjs.org/@angular/platform-browser/-/platform-browser-22.2.1.tgz", @@ -6788,6 +6829,16 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } + }, + "node_modules/zod": { + "version": "4.6.5", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.6.5.tgz", + "integrity": "sha512-v5l/aFXZQeai4awLbOpSoHecE9UiMrnfx75tEXLjNonXVARxQ5mOeipTjROUchszUNCqnE+hqAMujRsRHsut2Q==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } } } } diff --git a/package.json b/package.json index 151e82c..cc5f243 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@ngbracket/a11y-devtools", - "version": "0.16.0", + "version": "0.17.0", "description": "Finds accessibility problems in Angular apps and names the component behind each one. A dev overlay, or headless reports for CI.", "license": "MIT", "author": "Duncan Faulkner", @@ -74,10 +74,12 @@ "@analogjs/vite-plugin-angular": "^2.8.0", "@analogjs/vitest-angular": "^2.8.0", "@angular/build": "^22.2.1", + "@angular/cdk": "^22.2.1", "@angular/common": "^22.2.1", "@angular/compiler": "^22.2.1", "@angular/compiler-cli": "^22.2.1", "@angular/core": "^22.2.1", + "@angular/forms": "^22.2.1", "@angular/platform-browser": "^22.2.1", "@angular/platform-browser-dynamic": "^22.2.1", "@emnapi/core": "^1.11.3", diff --git a/src/keyboard/drag-drop.ts b/src/keyboard/drag-drop.ts new file mode 100644 index 0000000..af71e81 --- /dev/null +++ b/src/keyboard/drag-drop.ts @@ -0,0 +1,112 @@ +/** + * What marks a list whose items move by drag and drop, and what counts as a + * keyboard way to move them. The keyboard scan reports such a list when it finds + * no keyboard way (`ngbr/drag-without-keyboard`): Angular CDK drag-drop has no + * keyboard support of its own, so a CDK list is mouse-only unless the app adds it. + * + * Drag items are recognised by DOM markers that are there in production builds + * too: the `cdk-drag` class that `cdkDrag` puts on its host (only inside a + * `cdk-drop-list`; a free-standing `cdkDrag`, such as a draggable dialog title, + * moves something around the screen and is left out), and an explicit + * `draggable="true"` (native HTML drag and drop, ngx-drag-drop's `dndDraggable`). + */ + +/** A list of drag items under one container. */ +export interface DragGroup { + /** The drop list, or the items' parent for native drag and drop. */ + container: Element; + /** The items that can be dragged now (not disabled, not hidden). */ + items: Element[]; + /** True for CDK drag and drop; false for native `draggable="true"`. */ + cdk: boolean; +} + +const DRAG_ITEM_SELECTOR = '.cdk-drop-list .cdk-drag, [draggable="true"]'; + +/** Something that is, or holds, a drag list. */ +const DRAG_LIST_SELECTOR = '.cdk-drop-list, [draggable="true"]'; + +/** + * Names of controls that move an item: "Move up", "Move to top", "Reorder", + * arrow glyphs, and the Material icon ligatures for up and down arrows. Words + * such as "up" or "top" count only as the whole name, so "Sign up" or "Back to + * top" elsewhere near the list doesn't count. + */ +const MOVE_NAME = + /\b(move|reorder)\b|[↑↓⬆⬇▲▼]|\b(arrow_upward|arrow_downward|keyboard_arrow_up|keyboard_arrow_down)\b|^\s*(up|down|top|bottom|earlier|later)\s*$/i; + +const CONTROL_SELECTOR = 'button, a[href], [role="button"], [role="menuitem"]'; + +/** + * The drag lists in `root`, one per container. CDK items group by their + * `cdk-drop-list`; native ones by their parent, and only when there are at least + * two (one draggable on its own is more often an image or file chip to drag + * out of the page than a list to reorder). A disabled list or item (CDK's + * `cdk-drop-list-disabled` / `cdk-drag-disabled`) and CDK's drag preview and + * placeholder are left out. + */ +export function dragGroups(root: ParentNode, isHidden: (el: Element) => boolean): DragGroup[] { + const groups = new Map(); + for (const item of root.querySelectorAll(DRAG_ITEM_SELECTOR)) { + if (item.matches('.cdk-drag-disabled, .cdk-drag-preview, .cdk-drag-placeholder')) continue; + // A native draggable inside a CDK item (an image or link in a card) moves with the item. + if (!item.classList.contains('cdk-drag') && item.parentElement?.closest('.cdk-drag')) continue; + // From the parent: an element can be both an item and a drop list (nested lists). + const container = item.classList.contains('cdk-drag') + ? item.parentElement?.closest('.cdk-drop-list') + : item.parentElement; + if (!container || container.classList.contains('cdk-drop-list-disabled')) continue; + if (isHidden(item)) continue; + const items = groups.get(container) ?? []; + items.push(item); + groups.set(container, items); + } + return [...groups] + .map(([container, items]) => ({ container, items, cdk: items[0].classList.contains('cdk-drag') })) + .filter((group) => group.cdk || group.items.length >= 2); +} + +/** + * Where a keyboard user would start a move for `item`: its CDK drag handles that + * belong to it (not to a nested drag item), or the item itself. + */ +export function dragHandles(item: Element): Element[] { + const handles = [...item.querySelectorAll('.cdk-drag-handle')].filter( + (handle) => handle.closest('.cdk-drag, [draggable="true"]') === item, + ); + return handles.length > 0 ? handles : [item]; +} + +/** A rough accessible name for a control: aria-label, title, then its text. */ +function roughName(element: Element): string { + const labelledBy = (element.getAttribute('aria-labelledby') ?? '') + .split(/\s+/) + .map((id) => (id ? element.ownerDocument?.getElementById(id)?.textContent ?? '' : '')) + .join(' '); + return [element.getAttribute('aria-label'), labelledBy, element.getAttribute('title'), element.textContent] + .filter(Boolean) + .join(' '); +} + +/** + * True when there's a control near the list whose name says it moves an item + * ("Move up", "↓", an `arrow_upward` icon): in the list, or in an element next + * to it such as a toolbar above it. A sibling that holds another drag list is + * left out, so one list's Move buttons don't vouch for the list beside it. A + * drag handle doesn't count, even one named "Reorder": whether it works from the + * keyboard depends on its key handling, which the scan checks separately. + * `isTabbable` decides whether the keyboard can reach it. + */ +export function hasMoveControl(container: Element, isTabbable: (el: Element) => boolean): boolean { + const siblings = [...(container.parentElement?.children ?? [])].filter( + (el) => el !== container && !el.matches(DRAG_LIST_SELECTOR) && !el.querySelector(DRAG_LIST_SELECTOR), + ); + for (const scope of [container, ...siblings]) { + const controls = [...(scope.matches(CONTROL_SELECTOR) ? [scope] : []), ...scope.querySelectorAll(CONTROL_SELECTOR)]; + for (const control of controls) { + if (control.closest('.cdk-drag-handle')) continue; + if (MOVE_NAME.test(roughName(control)) && isTabbable(control)) return true; + } + } + return false; +} diff --git a/src/keyboard/focus-walk.ts b/src/keyboard/focus-walk.ts index fa6e293..17c1405 100644 --- a/src/keyboard/focus-walk.ts +++ b/src/keyboard/focus-walk.ts @@ -108,7 +108,10 @@ function deepActiveElement(doc: Document): Element | null { /** True when focus cycling among `elements` is an open modal containing focus (fine). */ function containedByOpenModal(elements: readonly Element[]): boolean { const modalOf = (el: Element): Element | null => { - const aria = el.closest('[aria-modal="true"]'); + // CDK / Material dialog containers trap focus by design, even with + // aria-modal="false" (Material's default). This trusts the container, so a + // hand-built .cdk-dialog-container with no trap that leaks focus isn't flagged. + const aria = el.closest('[aria-modal="true"], mat-dialog-container, .cdk-dialog-container'); if (aria) return aria; try { return el.closest('dialog:modal'); diff --git a/src/keyboard/keyboard-scan.ts b/src/keyboard/keyboard-scan.ts index 8149f9f..f49c600 100644 --- a/src/keyboard/keyboard-scan.ts +++ b/src/keyboard/keyboard-scan.ts @@ -8,8 +8,8 @@ * component like everything else. * * Honesty guardrail: these are *heuristics*, not axe rules. `click-without-key`, - * `hover-only-content` and `tab-order-mismatch` are flagged as candidates to - * verify by hand, never as confirmed failures. + * `hover-only-content`, `drag-without-keyboard` and `tab-order-mismatch` are + * flagged as candidates to verify by hand, never as confirmed failures. */ import { appComponentFromPath, @@ -32,6 +32,7 @@ import { } from './tab-sequence.js'; import { findUncontainedModals } from './focus-trap.js'; import { hoverSignal, type HoverSignal } from './hover-content.js'; +import { dragGroups, dragHandles, hasMoveControl } from './drag-drop.js'; import { blockingModalDialog } from '../top-layer.js'; import { OVERLAY_EXCLUDE_SELECTOR } from '../overlay.js'; @@ -75,6 +76,12 @@ function hasKeyEvent(events: readonly string[]): boolean { return events.some((e) => KEY_EVENTS.includes(e.split('.')[0])); } +/** True when `element` has a numeric `tabindex`, so script can focus it. */ +function hasTabindex(element: Element): boolean { + const tabindex = element.getAttribute('tabindex'); + return tabindex !== null && !Number.isNaN(Number.parseInt(tabindex, 10)); +} + /** * Composite widgets: Tab reaches the widget once, and arrow keys move between * its items (WAI-ARIA APG "Keyboard navigation inside components"). @@ -566,6 +573,82 @@ export function scanKeyboard( findings.push(hoverFinding(element, signal)); } + // Drag and drop with no keyboard way to move the items: no Move up / Move down + // style control near the list, and no drag handle Tab reaches that handles keys. + // One finding per list, on the list. + const groups = dragGroups(root, (el) => isHidden(el, isVisible)); + const startsOf = (container: Element, items: Element[]) => [ + ...new Set([container, ...items.flatMap(dragHandles)]), + ]; + // A roving-tabindex board has one tab stop among all its lists; without any tab + // stop on the page's drag lists, a tabindex="-1" item is out of reach. + const anyTabStop = groups.some(({ container, items }) => + startsOf(container, items).some((el) => isTabbable(el, isVisible)), + ); + for (const { container, items, cdk } of groups) { + if (modal && !modal.contains(container)) continue; + if (container.closest(OVERLAY_EXCLUDE_SELECTOR)) continue; + if (hasMoveControl(container, (el) => isTabbable(el, isVisible))) continue; + const starts = startsOf(container, items); + // Reachable: a tab stop, or an element script can focus (any tabindex) when a + // drag list on the page has a tab stop, as in a roving-tabindex board where + // one card is the tab stop and arrow keys move to the rest. The key-listener + // check below then decides. + const reachable = starts.filter( + (el) => + isTabbable(el, isVisible) || + (anyTabStop && hasTabindex(el) && !isDisabled(el) && !isHidden(el, isVisible)), + ); + const url = `${RULE_DOCS}/drag-without-keyboard`; + const count = items.length === 1 ? '1 item here moves' : `${items.length} items here move`; + const fix = + `Add buttons that move an item (Move up, Move down), or make each drag handle a ` + + `button that moves its item with the arrow keys and announces the new position. ` + + `Buttons also help people who can't drag with a mouse.`; + if (reachable.length === 0) { + findings.push( + make( + container, + 'ngbr/drag-without-keyboard', + // Native draggable="true" is weaker evidence of a list to reorder than CDK's. + cdk ? 'serious' : 'moderate', + `Keyboard users can't move these items: ${count} by drag and drop, but Tab ` + + `doesn't reach a drag handle and there's no other control to move them. ` + + `${fix} Heuristic — verify manually.`, + url, + ), + ); + continue; + } + // The keyboard can reach a handle or item. In a production build the key + // listeners can't be seen, so assume it handles keys. + if (!listenersVisible()) continue; + // From each reachable handle up through the list, to and including the + // nearest component host above it: a sortable tab list, say, handles keys on + // a wrapper. + const ng = ngDebug(); + const keyed = (start: Element): boolean => { + for (let el: Element | null = start; el && el !== el.ownerDocument?.body; el = el.parentElement) { + if (hasKeyEvent(eventsOf(el))) return true; + if (!container.contains(el) && isComponentHost(el, ng)) break; + } + return false; + }; + const handlesKeys = reachable.some(keyed); + if (handlesKeys) continue; + findings.push( + make( + container, + 'ngbr/drag-without-keyboard', + 'moderate', + `Keyboard users may not be able to move these items: ${count} by drag and drop, ` + + `and the keyboard can reach their drag handles, but nothing on them handles keys. ` + + `${fix} Heuristic — verify manually.`, + url, + ), + ); + } + // Missing focus trap (M3): an open aria-modal whose focus isn't contained. // Reuses the tab-order machinery (inert/hidden already excluded), so a modal // that correctly inerts the background is not flagged. diff --git a/src/keyboard/tab-sequence.ts b/src/keyboard/tab-sequence.ts index c47d5ed..a06a677 100644 --- a/src/keyboard/tab-sequence.ts +++ b/src/keyboard/tab-sequence.ts @@ -57,8 +57,8 @@ export interface TabSequenceOptions { /** * A focus-trap sentinel: an empty, script-driven tab stop that sends focus back * into the modal when reached — Angular CDK / Material's `cdk-focus-trap-anchor` - * (`tabindex="0"`, `aria-hidden="true"`), or a `data-focus-guard` element (the - * focus-lock pattern). No text and no child elements, so it isn't real content. + * (`tabindex="0"`, `aria-hidden="true"`), PrimeNG's `p-hidden-focusable` span, + * or a `data-focus-guard` element (the focus-lock pattern). No text and no child elements, so it isn't real content. */ export function isFocusSentinel(el: Element): boolean { if (el.childElementCount > 0 || (el.textContent ?? '').trim() !== '') return false; @@ -66,14 +66,43 @@ export function isFocusSentinel(el: Element): boolean { } /** - * The open `aria-modal` whose focus a JS trap keeps in, or null: its stops are - * one unbroken run in `order` with a sentinel directly before and after. The - * browser can reach those sentinels, but the trap sends focus from them back - * into the modal. Checked with real Tab presses on a CDK dialog: from the last + * Dialogs from libraries that trap Tab with a script and add no sentinels: + * ng-bootstrap's modal and offcanvas (`ngbFocusTrap`). They're recognised by + * element name, which is there in production builds too. + */ +const SCRIPT_TRAP_SELECTOR = 'ngb-modal-window, ngb-offcanvas-panel'; + +/** + * Elements that may hold a JS focus trap: any `aria-modal`, any dialog role, the + * Angular CDK / Material dialog containers, and {@link SCRIPT_TRAP_SELECTOR}. + * Material 22 opens `mat-dialog` with `aria-modal="false"` (it hides the page + * with `aria-hidden` instead), so `aria-modal="true"` alone misses every default + * Material dialog. + */ +const TRAP_CANDIDATE_SELECTOR = [ + '[aria-modal="true"]', + '[role="dialog"]', + '[role="alertdialog"]', + 'mat-dialog-container', + '.cdk-dialog-container', + SCRIPT_TRAP_SELECTOR, +].join(', '); + +/** + * The open dialog whose focus a JS trap keeps in, or null. Its stops must be one + * unbroken run in `order`, and one of these holds: + * - a sentinel sits directly before and after the run, outside the dialog + * (Angular CDK / Material: the anchors are the dialog container's siblings); + * - the run starts and ends with a sentinel inside the dialog (PrimeNG's + * `pFocusTrap` prepends and appends hidden spans; ngx-bootstrap's + * `modal-container` holds its CDK-style anchors around `.modal-dialog`); + * - the dialog is one of {@link SCRIPT_TRAP_SELECTOR}. + * The browser can reach those sentinels, but the trap sends focus from them back + * into the dialog. Checked with real Tab presses on a CDK dialog: from the last * control, Tab wraps to the first. With several, the last is the topmost. */ function sentinelTrappedModal(root: ParentNode, order: Element[]): Element | null { - const modals = [...root.querySelectorAll('[aria-modal="true"]')].reverse(); + const modals = [...root.querySelectorAll(TRAP_CANDIDATE_SELECTOR)].reverse(); for (const modal of modals) { const inside = order.map((el) => modal.contains(el)); const first = inside.indexOf(true); @@ -82,6 +111,8 @@ function sentinelTrappedModal(root: ParentNode, order: Element[]): Element | nul const before = order[first - 1]; const after = order[last + 1]; if (before && after && isFocusSentinel(before) && isFocusSentinel(after)) return modal; + if (last > first && isFocusSentinel(order[first]) && isFocusSentinel(order[last])) return modal; + if (modal.matches(SCRIPT_TRAP_SELECTOR)) return modal; } return null; } diff --git a/src/provider.ts b/src/provider.ts index d23bc4c..781b63b 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -65,7 +65,8 @@ export interface A11yDevtoolsOptions { /** * Turn on the **keyboard layer** — the ~2/3 of accessibility axe can't test. * Adds heuristic `ngbr/*` findings (keyboard-unreachable controls, - * click-without-keyboard handlers, hover-only content, tab-order mismatches) + * click-without-keyboard handlers, hover-only content, drag and drop with no + * keyboard way, tab-order mismatches) * to the report, and, * when `overlay` is on, draws the numbered tab-order path over the page. * Default false. diff --git a/src/report/acr.ts b/src/report/acr.ts index e08eaf3..22ba419 100644 --- a/src/report/acr.ts +++ b/src/report/acr.ts @@ -73,6 +73,7 @@ const NGBR_RULES: Record { expect(row(on.rows, '2.1.2')).toMatchObject({ result: 'no-failures-detected', checkedBy: ['ngbr/focus-trap'] }); expect(row(on.rows, '2.4.3').checkedBy).toEqual(['ngbr/tab-order-mismatch', 'ngbr/modal-focus-not-contained']); expect(row(on.rows, '2.1.1').checkedBy).toContain('ngbr/hover-only-content'); + expect(row(on.rows, '2.1.1').checkedBy).toContain('ngbr/drag-without-keyboard'); + expect(row(on.rows, '2.5.7')).toMatchObject({ result: 'no-failures-detected', checkedBy: ['ngbr/drag-without-keyboard'] }); }); it('marks heuristic-only findings as possible failures, and axe findings win', () => { diff --git a/src/testing/drag-drop.spec.ts b/src/testing/drag-drop.spec.ts new file mode 100644 index 0000000..e29b847 --- /dev/null +++ b/src/testing/drag-drop.spec.ts @@ -0,0 +1,293 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { RULE_DOCS, scanKeyboard } from '../keyboard/keyboard-scan'; +import { dragGroups, dragHandles } from '../keyboard/drag-drop'; + +/** Install a fake `window.ng` whose `getListeners` reports `listeners`. */ +function withNg( + fn: () => void, + listeners: Map = new Map(), + hosts: ReadonlySet = new Set(), +): void { + const original = (globalThis as { ng?: unknown }).ng; + (globalThis as { ng?: unknown }).ng = { + getComponent: (el: Element) => (hosts.has(el) ? {} : null), + getOwningComponent: () => null, + getDirectives: () => [], + getListeners: (el: Element) => (listeners.get(el) ?? []).map((name) => ({ name, type: 'dom' as const })), + }; + try { + fn(); + } finally { + (globalThis as { ng?: unknown }).ng = original; + } +} + +/** Run `fn` with no `window.ng`, as in a production build. */ +function withoutNg(fn: () => void): void { + const original = (globalThis as { ng?: unknown }).ng; + (globalThis as { ng?: unknown }).ng = undefined; + try { + fn(); + } finally { + (globalThis as { ng?: unknown }).ng = original; + } +} + +function fixture(html: string): HTMLElement { + const host = document.createElement('div'); + host.innerHTML = html; + document.body.appendChild(host); + return host; +} + +const drag = (host: ParentNode) => scanKeyboard(host).filter((f) => f.id === 'ngbr/drag-without-keyboard'); + +/** A CDK table whose rows reorder by a drag-handle icon, like a list of levels. */ +const row = (name: string, extra = '') => ` + + drag_indicator + + ${extra} + `; +const table = (rows: string, listClass = '') => ` + ${rows}
`; + +describe('ngbr/drag-without-keyboard', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('flags a CDK drop list whose handles Tab never reaches, once, on the list', () => { + const host = fixture(table(row('Novice') + row('Beginner') + row('Adept'))); + const found = drag(host); + expect(found).toHaveLength(1); + expect(found[0].impact).toBe('serious'); + expect(found[0].target).toContain('levels'); + expect(found[0].help).toContain('3 items here move'); + expect(found[0].help).toContain('Heuristic — verify manually'); + expect(found[0].helpUrl).toBe(`${RULE_DOCS}/drag-without-keyboard`); + }); + + it('skips a list with Move up / Move down buttons in its rows', () => { + const buttons = ''; + const host = fixture(table(row('Novice', buttons) + row('Beginner', buttons))); + expect(drag(host)).toEqual([]); + }); + + it('skips icon-only move buttons named by a Material arrow ligature or aria-label', () => { + const icons = ''; + expect(drag(fixture(table(row('Novice', icons))))).toEqual([]); + document.body.innerHTML = ''; + const labelled = ''; + expect(drag(fixture(table(row('Novice', labelled))))).toEqual([]); + }); + + it('skips a move control next to the list, such as a toolbar above it', () => { + const host = fixture(` +
+
+
  • One
+
`); + expect(drag(host)).toEqual([]); + }); + + it("doesn't let one list's Move buttons vouch for a list beside it", () => { + const host = fixture(` +
+
  1. ⠿A
+
  1. ⠿A
+
`); + expect(drag(host).map((f) => f.target)).toEqual([expect.stringContaining('mouse-only')]); + }); + + it('still flags a list whose move buttons Tab can\'t reach', () => { + const disabled = ''; + expect(drag(fixture(table(row('Novice', disabled))))).toHaveLength(1); + }); + + it('skips a focusable handle that handles keys (dev build)', () => { + const host = fixture(` +
    +
  • One
  • +
`); + const handle = host.querySelector('.cdk-drag-handle')!; + withNg(() => expect(drag(host)).toEqual([]), new Map([[handle, ['keydown.arrowup']]])); + }); + + it('skips a focusable handle when a row or list listener handles the keys', () => { + const host = fixture(` +
    +
  • One
  • +
`); + const item = host.querySelector('.cdk-drag')!; + withNg(() => expect(drag(host)).toEqual([]), new Map([[item, ['keydown']]])); + }); + + it('reports a focusable handle with no key handling as moderate (dev build)', () => { + const host = fixture(` +
    +
  • One
  • +
`); + withNg(() => { + const found = drag(host); + expect(found).toHaveLength(1); + expect(found[0].impact).toBe('moderate'); + expect(found[0].help).toContain('nothing on them handles keys'); + }); + }); + + it('gives a focusable handle the benefit of the doubt in a production build', () => { + const host = fixture(` +
    +
  • One
  • +
`); + withoutNg(() => expect(drag(host)).toEqual([])); + }); + + it('still flags an unreachable handle in a production build (markers only)', () => { + const host = fixture(table(row('Novice'))); + withoutNg(() => expect(drag(host)).toHaveLength(1)); + }); + + it('skips a roving-tabindex board: cards with tabindex="-1" that handle keys', () => { + // Like @ngbracket/board: one card on the whole board is the tab stop and arrow + // keys move between the rest, so a column can have no tab stop of its own. + const host = fixture(` +
A
+
B
`); + const cards = [...host.querySelectorAll('.cdk-drag')]; + withNg(() => expect(drag(host)).toEqual([]), new Map(cards.map((c) => [c, ['keydown']]))); + withoutNg(() => expect(drag(host)).toEqual([])); + }); + + it('reports tabindex="-1" items with no key handling as moderate (dev build)', () => { + const host = fixture(`
  • One
  • Two
`); + withNg(() => expect(drag(host).map((f) => f.impact)).toEqual(['moderate'])); + }); + + it('still reports serious when every item is tabindex="-1" and nothing is a tab stop', () => { + const host = fixture(`
  • One
  • Two
`); + withNg(() => expect(drag(host).map((f) => f.impact)).toEqual(['serious'])); + withoutNg(() => expect(drag(host).map((f) => f.impact)).toEqual(['serious'])); + }); + + it('skips native draggable images inside a CDK item', () => { + const host = fixture(` +
  • + ab
`); + expect(drag(host)).toEqual([]); + }); + + it('skips disabled lists and items, and a cdkDrag outside a drop list', () => { + expect(drag(fixture(table(row('Novice'), 'cdk-drop-list-disabled')))).toEqual([]); + document.body.innerHTML = ''; + const disabledRow = row('Novice').replace('class="cdk-drag"', 'class="cdk-drag cdk-drag-disabled"'); + expect(drag(fixture(table(disabledRow)))).toEqual([]); + document.body.innerHTML = ''; + expect(drag(fixture('

Drag me

'))).toEqual([]); + }); + + it('skips hidden items', () => { + const hiddenRow = row('Novice').replace('', ''); + expect(drag(fixture(table(hiddenRow)))).toEqual([]); + }); + + it('flags native draggable="true" lists as moderate, grouped by their parent', () => { + const host = fixture(` +
  • One
  • Two
+
  • Three
  • Four
`); + const found = drag(host); + expect(found.map((f) => f.target)).toEqual([expect.stringContaining('cards'), expect.stringContaining('other')]); + expect(found.map((f) => f.impact)).toEqual(['moderate', 'moderate']); + }); + + it('skips a lone draggable="true" element, such as an image to drag out of the page', () => { + expect(drag(fixture('

Logo

'))).toEqual([]); + }); + + it('says "1 item" for a list of one', () => { + expect(drag(fixture(table(row('Novice'))))[0].help).toContain('1 item here moves'); + }); + + it('is not silenced by "Sign up" or "Back to top" near the list', () => { + const host = fixture(` +
+ Back to top + ${table(row('Novice') + row('Beginner'))} +
`); + expect(drag(host)).toHaveLength(1); + }); + + it('accepts a whole-name "Up" / "Down" button', () => { + expect(drag(fixture(table(row('Novice', ''))))).toEqual([]); + }); + + it('finds key handling on a component host above the list (a sortable tab list)', () => { + const host = fixture(` +
+
+ +
+
`); + const appTabs = host.querySelector('app-tabs')!; + const wrapper = host.querySelector('.wrapper')!; + withNg(() => expect(drag(host)).toEqual([]), new Map([[wrapper, ['keydown']]]), new Set([appTabs])); + }); + + it('stops looking for key handling at the component host', () => { + const host = fixture(` +
+
One
+
`); + const page = host.querySelector('.page')!; + const appList = host.querySelector('app-list')!; + withNg(() => expect(drag(host)).toHaveLength(1), new Map([[page, ['keydown']]]), new Set([appList])); + }); +}); + +describe('dragGroups and dragHandles', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('groups CDK items by drop list and ignores the drag preview and placeholder', () => { + const host = fixture(` +
+
1
p
+
+
2
+
preview
`); + const groups = dragGroups(host, () => false); + expect(groups.map((g) => [g.container.id, g.items.length])).toEqual([ + ['a', 1], + ['b', 1], + ]); + }); + + it('groups an item that is also a drop list under the list around it', () => { + const host = fixture(` +
+
child
+
`); + const groups = dragGroups(host, () => false); + expect(groups.map((g) => [g.container.id, g.items.length])).toEqual([ + ['outer', 1], + ['both', 1], + ]); + }); + + it("returns an item's own handles, not a nested item's, else the item", () => { + const host = fixture(` +
+
+ +
+
+
no handle
+
`); + const ids = (id: string) => dragHandles(host.querySelector(`#${id}`)!).map((el) => el.id); + expect(ids('outer')).toEqual(['h1']); + expect(ids('inner')).toEqual(['h2']); + expect(ids('plain')).toEqual(['plain']); + }); +}); diff --git a/src/testing/e2e-angular.spec.ts b/src/testing/e2e-angular.spec.ts index 2af599c..3ecd857 100644 --- a/src/testing/e2e-angular.spec.ts +++ b/src/testing/e2e-angular.spec.ts @@ -162,6 +162,13 @@ describe.skipIf(!ready)('attribution in a real Angular app (E2E)', () => { expect(find(page('/').findings, 'ngbr/hover-only-content', 'title-icon').impact).toBe('minor'); }); + it('flags a drag list whose handles Tab can\'t reach, not one whose handle handles keys', () => { + const list = find(page('/').findings, 'ngbr/drag-without-keyboard', 'drag-mouse-only'); + expect(list.impact).toBe('serious'); + expect(list.component).toBe('HomePageComponent'); + expect(page('/').findings.filter((f) => f.html.includes('drag-keys'))).toEqual([]); + }); + it('attributes findings on a second route to that route’s page component', () => { expect(find(page('/settings').findings, 'color-contrast', 'faint').component).toBe('SettingsPageComponent'); }); @@ -188,6 +195,21 @@ describe.skipIf(!ready)('attribution in a real Angular app (E2E)', () => { expect(stderr).toContain('no component attribution (window.ng absent)'); }, 60_000); + it('the tab-order layer shows only the stops of an open CDK dialog (aria-modal="false")', async () => { + const { chromium } = await import('playwright'); + const browser = await chromium.launch(); + try { + const tab = await browser.newPage(); + await tab.goto(`${dev}/dialog?overlay`); + await tab.locator('.dialog-save').waitFor(); + expect(await tab.locator('[role="dialog"]').getAttribute('aria-modal')).toBe('false'); + const badges = tab.locator('[data-ngb-tab-order]'); + await expect.poll(() => badges.count(), { timeout: 15_000 }).toBe(2); + } finally { + await browser.close(); + } + }, 60_000); + it('the in-app overlay labels findings with their components', async () => { const { chromium } = await import('playwright'); const browser = await chromium.launch(); diff --git a/src/testing/focus-trap.spec.ts b/src/testing/focus-trap.spec.ts index 25aa2a1..f8bb68b 100644 --- a/src/testing/focus-trap.spec.ts +++ b/src/testing/focus-trap.spec.ts @@ -45,6 +45,50 @@ describe('findUncontainedModals', () => { expect(findUncontainedModals(host, alwaysVisible)).toEqual([]); }); + it('does not flag a default mat-dialog (aria-modal="false") as a leak', () => { + const host = fixture(` + +
+ + + + + +
+ `); + expect(findUncontainedModals(host, alwaysVisible)).toEqual([]); + }); + + it('does not flag PrimeNG, ngx-bootstrap or ng-bootstrap modals, which trap focus', () => { + const primeng = fixture(` + +
+ + + +
+ `); + expect(findUncontainedModals(primeng, alwaysVisible)).toEqual([]); + document.body.innerHTML = ''; + + const ngx = fixture(` + + + + + + + `); + expect(findUncontainedModals(ngx, alwaysVisible)).toEqual([]); + document.body.innerHTML = ''; + + const ngb = fixture(` + + + `); + expect(findUncontainedModals(ngb, alwaysVisible)).toEqual([]); + }); + it('accepts data-focus-guard sentinels (the focus-lock pattern)', () => { const host = fixture(` diff --git a/src/testing/focus-walk.spec.ts b/src/testing/focus-walk.spec.ts index dd0b916..63d890d 100644 --- a/src/testing/focus-walk.spec.ts +++ b/src/testing/focus-walk.spec.ts @@ -105,4 +105,31 @@ describe('createFocusWalkProbe', () => { expect(walk.trapFinding([ok, cancel], false)).toBeNull(); }); + it('does not report focus cycling inside a default mat-dialog (aria-modal="false")', () => { + document.body.innerHTML = ` + + + + `; + const walk = createFocusWalkProbe(document); + focus('ok'); + const ok = walk.probe().id!; + focus('cancel'); + const cancel = walk.probe().id!; + + expect(walk.trapFinding([ok, cancel], false)).toBeNull(); + }); + + it('still reports focus cycling inside a plain non-modal dialog', () => { + document.body.innerHTML = ` +
+ `; + const walk = createFocusWalkProbe(document); + focus('ok'); + const ok = walk.probe().id!; + focus('cancel'); + const cancel = walk.probe().id!; + + expect(walk.trapFinding([ok, cancel], false)).not.toBeNull(); + }); }); diff --git a/src/testing/tab-sequence.spec.ts b/src/testing/tab-sequence.spec.ts index 5997c43..80b1e09 100644 --- a/src/testing/tab-sequence.spec.ts +++ b/src/testing/tab-sequence.spec.ts @@ -156,6 +156,110 @@ describe('tab-sequence', () => { expect(ids).toEqual(['name', 'save']); }); + it('only counts stops inside a default mat-dialog (aria-modal="false", CDK anchors beside it)', () => { + // Material 22's real structure: anchors are siblings of the container, and + // ariaModal defaults to false (the page is hidden with aria-hidden instead). + const host = fixture(` + +
+
+
+ + + + + +
+
+
+ `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids).toEqual(['name', 'save']); + }); + + it('picks the mat-dialog-container, not a role="dialog" wrapper around its anchors', () => { + const host = fixture(` + + + `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids).toEqual(['save']); + }); + + it('only counts stops inside a PrimeNG dialog (pFocusTrap spans inside the dialog)', () => { + const host = fixture(` + + + + `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids).toEqual(['name', 'save']); + }); + + it('only counts stops inside an ngx-bootstrap modal (anchors around .modal-dialog)', () => { + const host = fixture(` + + + + + + + `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids).toEqual(['close']); + }); + + it('only counts stops inside an ng-bootstrap modal or offcanvas (script trap, no anchors)', () => { + for (const tag of ['ngb-modal-window', 'ngb-offcanvas-panel']) { + const host = fixture(` + + <${tag} class="modal d-block" role="dialog" tabindex="-1" aria-modal="true"> + + + `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids, tag).toEqual(['ok']); + document.body.innerHTML = ''; + } + }); + + it('keeps the page stops when a dialog has a sentinel inside at one end only', () => { + const host = fixture(` + +
+ + +
+ `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids).toEqual(['behind', 'ok']); + }); + + it('keeps the page stops for a dialog with no focus-trap anchors (non-modal)', () => { + const host = fixture(` + +
+ + `); + const ids = tabSequence(host, { isVisible: alwaysVisible }).map((s) => s.element.id); + expect(ids).toEqual(['behind', 'ok', 'after']); + }); + it('isTabbable honours the visibility predicate', () => { const host = fixture(``); const button = host.querySelector('#b')!;