Repository navigation
test: drive useFocusTrap's Shift+Tab wrap and other-key return end to end - #1081
Merged
Merged
Conversation
… end
tests/e2e/community-people.spec.js owns the PersonDialog lightbox, which
mounts src/components/hooks/useFocusTrap.js. Two of that hook's arms were
never executed in a browser by any spec in tests/e2e/:
* useFocusTrap.js:38-40, the backwards wrap. No e2e spec pressed
Shift+Tab at all, so forward tabbing short-circuited on event.shiftKey
and never evaluated document.activeElement === first. A dialog that
leaked focus to the page behind it on Shift+Tab is a WCAG 2.1.2 /
2.4.3 defect that shipped green.
* useFocusTrap.js:33, the early return for every key that is neither
Escape nor Tab. The listener is on document, so it observes every
keystroke on the page; nothing asserted that an ordinary key leaves
the dialog and its focus alone.
tests/e2e/accessibility.spec.js cannot reach either: axe-core scans
static markup and does not drive the keyboard.
Both components report 100% unit line and region coverage, so this is an
e2e-only gap. End-to-end regions for useFocusTrap.js move from 62.96% to
64.29%, and src files from 466/377/80.90% to 467/379/81.16%.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: quality <quality@hive.kubestellar.io>
Contributor
Author
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Non-outreach agents are held at ACMM L3–L5; the Hive will automatically remove the |
This was referenced Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test Improvement
Adds two end-to-end cases to
tests/e2e/community-people.spec.js— the specthat already owns the
PersonDialoglightbox — driving two arms ofsrc/components/hooks/useFocusTrap.jsthat no spec intests/e2e/had everexecuted in a browser:
Shift+Tab from the first focusable element wraps to the lastuseFocusTrap.js:38-40—if (event.shiftKey && document.activeElement === first) { event.preventDefault(); last.focus(); }. No e2e spec pressed Shift+Tab at all, so forward tabbing short-circuited onevent.shiftKeyand the comparison was never evaluated.a key that is neither Escape nor Tab leaves the dialog aloneuseFocusTrap.js:33—if (event.key !== 'Tab') return;. The handler is bound todocumentand sees every keystroke on the page, but nothing asserted that an ordinary key leaves the dialog visible and focus unmoved.tests/e2e/accessibility.spec.jscannot reach either arm: axe-core scansstatic markup and never drives the keyboard. A dialog that leaked focus to the
page behind it on Shift+Tab — WCAG 2.1 2.1.2 No Keyboard Trap / 2.4.3
Focus Order — would ship green today.
No production code changes; one test file.
Evidence
Unit (
npm run test:unit:coverage,TZ=UTC node tests/tools/coverage-report.mjs,node v26.10.0, local, base
900592b):src/components/hooks/useFocusTrap.js100.00% lines / 100.00% regions. This is an e2e-only gap, so the rules put
it at medium priority rather than high.
E2E (local
npm run build:e2e:coverage,e2e-coverage-run.mjs init,npm run test:e2e:coveragewithE2E_COVERAGE_DIR/E2E_COVERAGE_RUN_ID,seal --status passed,e2e-coverage-report.mjs --input <dir> --build build):useFocusTrap.jsregionssrc files900592b, 298 passedThe base figure reproduces CI job
End-to-end coverage(workflowValidate repository, run37164361552) at
468 / 379 / 80.98% to within one region.
npm run test:unit:coverage:checkand
npm run check:formatboth pass on this branch.A caveat worth stating plainly
The two arms are now provably executed —
Shift+Tablanding focus on thelast focusable element can only happen if
last.focus()ran — yet the regionkeys
33:31:33:38,38:24:40:21and40:18:43:22stay at zero in the report,while a newly emitted key covering the same source (
33:6:33:38, count 3)appears beside them. That is the drifted-span defect already recorded in #1079
and #1066, observed here on a third component; it is why the gate moves by
+2 regions rather than by the five lines involved. It is not a reason to
withhold the tests, and this PR deliberately does not touch
tests/tools/e2e-coverage-report.mjs, which #1040, #1051, #1070 and #1072 arealready rewriting.
Coordination
This PR claims exactly one file,
tests/e2e/community-people.spec.js, and oneproduction surface,
src/components/hooks/useFocusTrap.js. Neither is touchedby any open PR: #1058 claims
tests/e2e/interactions.spec.js, #1060tests/e2e/member-directory.spec.js, #1075tests/e2e/member-directory-freshness.spec.js, #1054tests/e2e/data-fixtures.spec.js(andsrc/lib/profile-links.mjs), and #1034 /#1078
tests/e2e-data-fixtures.test.mjs. Branch cut from a freshorigin/mainat900592b.Related Issue
Closes #1080
Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88