luckberg/familysync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(17): CR-01 make focus-trap visibility filter tolerant of jsdom; IN-01 drop inert containment guard

Browse Source
This commit is contained in:
Lucas Berger
2026-06-18 14:04:33 -04:00
parent 89dee4f586
commit 287ecae2f7
1 changed files with 17 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/pwa/src/hooks/useFocusTrap.ts
+17 -11
View File
@@ -29,12 +29,22 @@ export function useFocusTrap(
).filter((el) => { ).filter((el) => {
// Exclude disabled / explicitly-untabbable nodes … // Exclude disabled / explicitly-untabbable nodes …
if (el.hasAttribute('disabled') || el.getAttribute('tabindex') === '-1') return false; if (el.hasAttribute('disabled') || el.getAttribute('tabindex') === '-1') return false;
// … and the `hidden` attribute, which is unambiguous regardless of layout.
if (el.hasAttribute('hidden')) return false;
// … and nodes that are not actually rendered/visible (WR-01). A focusable // … and nodes that are not actually rendered/visible (WR-01). A focusable
// inside a hidden/collapsed block would otherwise become the computed // inside a hidden/collapsed block would otherwise become the computed
// first/last and `last.focus()` would no-op, leaking Tab to background // first/last and `last.focus()` would no-op, leaking Tab to background
// content that `aria-modal="true"` promises is unreachable. // content that `aria-modal="true"` promises is unreachable.
if (el.hasAttribute('hidden') || el.offsetParent === null) return false; //
// Guard against a non-layout environment (jsdom): there, every node reports
// all-zero geometry and a null offsetParent, so applying the visibility
// heuristic unconditionally would reject *every* focusable and silently
// disable the trap. Only filter on visibility when there is positive
// evidence a layout engine is present; otherwise treat the node as visible.
const r = el.getBoundingClientRect(); const r = el.getBoundingClientRect();
const hasLayout = r.width > 0 || r.height > 0 || el.offsetParent !== null;
if (!hasLayout) return true; // no layout engine → don't filter on visibility
if (el.offsetParent === null) return false;
return r.width > 0 && r.height > 0; return r.width > 0 && r.height > 0;
}); });
@@ -43,16 +53,12 @@ export function useFocusTrap(
const first = focusable[0]; const first = focusable[0];
const last = focusable[focusable.length - 1]; const last = focusable[focusable.length - 1];
// Containment guard (IN-01): if focus has somehow landed outside the dialog // Boundary-only trap (IN-01): this handler is wired to the dialog's own
// (programmatic focus, browser quirk), neither boundary condition below // onKeyDown, so it only runs while focus is already inside the dialog
// matches and Tab would walk background content. Pull focus back to the // subtree — a `document`-level containment guard would be required to pull
// first focusable instead of relying on a boundary-only trap. // back focus that originates outside, and is unnecessary for the current
if (!dialogRef.current.contains(document.activeElement)) { // always-focus-the-heading-on-open flows. We intentionally do not claim a
e.preventDefault(); // containment guarantee the wiring cannot provide.
first.focus();
return;
}
if (e.shiftKey) { if (e.shiftKey) {
// Shift+Tab: if on first element, wrap to last // Shift+Tab: if on first element, wrap to last
if (document.activeElement === first) { if (document.activeElement === first) {