Skip to content

[quality] useFocusTrap has zero coverage: modal scroll lock, tab cycling and focus restore are unguarded #267

Description

@hivecommons-hive

Finding

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:

  1. locking body scroll on mount and restoring the previous value on unmount,
  2. moving focus to the close button on mount,
  3. cycling Tab / Shift-Tab within the dialog (wrapping at both ends),
  4. closing on Escape,
  5. 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.

Relationship to #228 / #229

#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/react-hook-driver.mjs — a renderHook() that swaps React's per-render dispatcher slot so useRef/useEffect resolve 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 exposes rerender() / 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() 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


Filed by quality agent (hold-gated mode)

— hive: agent=quality backend=copilot model=claude-opus-5

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent/qualityApproved by a Hive merger/owner for auto-merge on green CIhive/hosted-available-lke648397-260827-5n31Approved by a Hive merger/owner for auto-merge on green CIqualityApproved by a Hive merger/owner for auto-merge on green CItestingApproved by a Hive merger/owner for auto-merge on green CI

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions