You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
src/components/hooks/useFocusTrap.js has no test coverage of any kind. It is the shared accessibility primitive behind every modal dialog on the site — used by both src/components/MemberDirectory/index.js and src/components/CommunityPeople/index.js — and it is responsible for five separate behaviours, none of which is currently guarded:
locking body scroll on mount and restoring the previous value on unmount,
moving focus to the close button on mount,
cycling Tab / Shift-Tab within the dialog (wrapping at both ends),
closing on Escape,
restoring focus to the triggering element on unmount, falling back to whatever was focused before.
Every one of these is a silent failure mode. A broken scroll restore leaves the page permanently unscrollable after a dialog closes; a broken focus wrap lets keyboard users tab out of a modal into inert content behind it; a broken focus restore strands screen-reader users at the top of the document. None of it is visible to the Docusaurus build, to prettier, or to any validator script, and the repo has no browser suite that would catch it.
Evidence
Unit:node --test --experimental-test-coverage (node v26.8.2), local clone of cncf/endusers at 00b44df after npm ci, run 2026-09-17. 55 tests pass, and src/ is absent from the coverage report entirely — every module under src/ is 0%, never loaded.
End-to-end:unavailable. This repository has no end-to-end or browser suite (no Playwright/Cypress/e2e dependency, config or workflow), and no suite publishes a coverage artifact — tracked separately in [quality] CI publishes no coverage evidence, so coverage findings cannot be verified #186. No claim is made here that this path lacks end-to-end coverage; it cannot be established either way, and the two sources therefore cannot be combined.
#228 covers filterArchitectures and the JSX import path, and explicitly defers this hook:
src/components/hooks/useFocusTrap.js and the rendered components ... still have no coverage. Exercising those needs a DOM environment and a React renderer, which is a materially larger decision than a JSX transform. That should be its own issue once this import path exists.
This is that issue. It turns out the "DOM environment and React renderer" are not actually required, and this work does not depend on #229 landing: useFocusTrap.js contains no JSX and imports no CSS module, so Node can import it as-is. The two remaining obstacles — React refusing to run hooks outside a render, and the absence of a document — can both be met without adding a dependency.
Recommendation
Add three test-only files. No production code changes, no new dependency, no change to package.json or to .github/workflows/**:
tests/tools/fake-dom.mjs — a stand-in for only the DOM surface this hook actually touches: body.style.overflow, activeElement, add/removeEventListener('keydown'), element.focus() and querySelectorAll('button, a[href]'). Keeping it this small avoids a full DOM implementation and makes the hook's real contract with the DOM explicit.
tests/use-focus-trap.test.mjs — 17 unit tests covering: stable ref identity across renders; scroll lock and restore of the previous overflow value; focus moved to the close button on mount; unattached closeRef tolerated; Escape invoking onClose; other keys ignored; Shift-Tab wrapping from first to last; Tab wrapping from last to first; Tab in the middle left to the browser; a single focusable element trapping onto itself both ways; Tab ignored with no focusable children and with an unattached dialogRef; non-focusable descendants (<a> without href, <span>) excluded from the cycle; listener removal on unmount; focus restored to the trigger; fallback to the previously focused element; and unmount not throwing when there is nothing to restore to.
Expected result
72 tests pass (up from 55), and src/components/hooks/useFocusTrap.js reports 100.00% line / 100.00% branch / 100.00% funcs — the first src/ module to appear in the report at all. Repo totals move 73.51 → 82.93 line and 47.12 → 63.53 branch.
The suite should be mutation-checked rather than assumed effective. Seven independent mutations of the hook — overflow not restored, preventDefault dropped, keydown listener never removed, first/last focusable swapped, Escape no longer closing, close button not focused on mount, focus not restored on unmount — should each produce failures.
Priority
Impact: medium-high — a shared accessibility primitive used by two published directory pages, where every failure mode is silent and keyboard/screen-reader users are the ones affected
Finding
src/components/hooks/useFocusTrap.jshas no test coverage of any kind. It is the shared accessibility primitive behind every modal dialog on the site — used by bothsrc/components/MemberDirectory/index.jsandsrc/components/CommunityPeople/index.js— and it is responsible for five separate behaviours, none of which is currently guarded:Every one of these is a silent failure mode. A broken scroll restore leaves the page permanently unscrollable after a dialog closes; a broken focus wrap lets keyboard users tab out of a modal into inert content behind it; a broken focus restore strands screen-reader users at the top of the document. None of it is visible to the Docusaurus build, to
prettier, or to any validator script, and the repo has no browser suite that would catch it.Evidence
node --test --experimental-test-coverage(node v26.8.2), local clone ofcncf/endusersat00b44dfafternpm ci, run 2026-09-17. 55 tests pass, andsrc/is absent from the coverage report entirely — every module undersrc/is 0%, never loaded.Relationship to #228 / #229
#228 covers
filterArchitecturesand the JSX import path, and explicitly defers this hook:This is that issue. It turns out the "DOM environment and React renderer" are not actually required, and this work does not depend on #229 landing:
useFocusTrap.jscontains no JSX and imports no CSS module, so Node can import it as-is. The two remaining obstacles — React refusing to run hooks outside a render, and the absence of adocument— can both be met without adding a dependency.Recommendation
Add three test-only files. No production code changes, no new dependency, no change to
package.jsonor to.github/workflows/**:tests/tools/react-hook-driver.mjs— arenderHook()that swaps React's per-render dispatcher slot souseRef/useEffectresolve against a tiny local implementation, then restores the slot immediately. It honours effect dependency arrays (re-running an effect, previous cleanup first, only when deps change) and exposesrerender()/unmount(). Because the slot is restored synchronously and no module is aliased process-wide, this cannot disturb other tests — including [quality] test: cover src/components/ArchitectureFilters filterArchitectures and add a JSX import path (tests/tools/jsx-hooks.mjs + tests/helpers-jsx.mjs + tests/architecture-filters.test.mjs) #229's, which import React for real.tests/tools/fake-dom.mjs— a stand-in for only the DOM surface this hook actually touches:body.style.overflow,activeElement,add/removeEventListener('keydown'),element.focus()andquerySelectorAll('button, a[href]'). Keeping it this small avoids a full DOM implementation and makes the hook's real contract with the DOM explicit.tests/use-focus-trap.test.mjs— 17 unit tests covering: stable ref identity across renders; scroll lock and restore of the previous overflow value; focus moved to the close button on mount; unattachedcloseReftolerated; Escape invokingonClose; other keys ignored; Shift-Tab wrapping from first to last; Tab wrapping from last to first; Tab in the middle left to the browser; a single focusable element trapping onto itself both ways; Tab ignored with no focusable children and with an unattacheddialogRef; non-focusable descendants (<a>withouthref,<span>) excluded from the cycle; listener removal on unmount; focus restored to the trigger; fallback to the previously focused element; and unmount not throwing when there is nothing to restore to.Expected result
72 tests pass (up from 55), and
src/components/hooks/useFocusTrap.jsreports 100.00% line / 100.00% branch / 100.00% funcs — the firstsrc/module to appear in the report at all. Repo totals move 73.51 → 82.93 line and 47.12 → 63.53 branch.The suite should be mutation-checked rather than assumed effective. Seven independent mutations of the hook — overflow not restored,
preventDefaultdropped, keydown listener never removed, first/last focusable swapped, Escape no longer closing, close button not focused on mount, focus not restored on unmount — should each produce failures.Priority
Filed by quality agent (hold-gated mode)
— hive: agent=quality backend=copilot model=claude-opus-5