Timmy_Foundation/the-door
15
0
Fork 0
Code Issues 19 Pull Requests 18 Actions Packages Projects Releases Wiki Activity

fix: overlay initial focus targets enabled element (closes #69) #116

Closed
Rockachopa wants to merge 2 commits from door/issue-69 into main
pull from: door/issue-69
merge into: Timmy_Foundation:main
Conversation 6 Commits 2 Files Changed 2 +54 -1
Rockachopa commented 2026-04-15 04:35:37 +00:00
Owner
Copy Link

Closes #69
Supersedes PR #107 (adds focus recovery for escaped focus)

Problem

When the crisis overlay opens, showOverlay() disables the dismiss button for a 10-second countdown, then calls overlayDismissBtn.focus(). Disabled buttons cannot receive focus, so keyboard and assistive-technology users lose context at the most critical moment.

Fix

  1. Initial focus: Focus the Call 988 link (.overlay-call) instead — always enabled, most important action
  2. Escape key: Dismiss overlay via Escape when countdown completes
  3. Focus recovery: If focus escapes the overlay (e.g., via screen reader), redirect it back
  4. Regression tests: 3 new test cases covering initial focus, Escape, and focus recovery

Changes

  • index.html: Focus .overlay-call on open, Escape handler, focus recovery in trap
  • tests/test_crisis_overlay_focus_trap.py: 3 new regression tests

Why this supersedes #107

PR #107 correctly fixes the initial focus bug but does not handle focus recovery if focus escapes the overlay. This PR adds that safety net, which is critical for assistive-technology users.

Testing

29 passed, 3 subtests passed
Closes #69 Supersedes PR #107 (adds focus recovery for escaped focus) ## Problem When the crisis overlay opens, `showOverlay()` disables the dismiss button for a 10-second countdown, then calls `overlayDismissBtn.focus()`. Disabled buttons cannot receive focus, so keyboard and assistive-technology users lose context at the most critical moment. ## Fix 1. **Initial focus**: Focus the Call 988 link (`.overlay-call`) instead — always enabled, most important action 2. **Escape key**: Dismiss overlay via Escape when countdown completes 3. **Focus recovery**: If focus escapes the overlay (e.g., via screen reader), redirect it back 4. **Regression tests**: 3 new test cases covering initial focus, Escape, and focus recovery ## Changes - `index.html`: Focus `.overlay-call` on open, Escape handler, focus recovery in trap - `tests/test_crisis_overlay_focus_trap.py`: 3 new regression tests ## Why this supersedes #107 PR #107 correctly fixes the initial focus bug but does not handle focus recovery if focus escapes the overlay. This PR adds that safety net, which is critical for assistive-technology users. ## Testing ``` 29 passed, 3 subtests passed ```
Rockachopa added 2 commits 2026-04-15 04:35:38 +00:00
fix: overlay initial focus targets enabled element (closes #69) 7271e853ff 
The crisis overlay was calling overlayDismissBtn.focus() while the
button was disabled during the 10-second countdown. Disabled buttons
cannot receive focus, leaving keyboard and assistive-technology users
without a valid focus target at the most critical interruption point.

Changes:
- Focus the Call 988 link (always enabled) on overlay open
- Add Escape key dismiss handler (refs #95)
- Add focus recovery if focus escapes the overlay
- Add regression tests for initial focus, Escape, and focus recovery
test: add regression tests for overlay focus fix (#69)
All checks were successful
Sanity Checks / sanity-test (pull_request) Successful in 12s
Details
Smoke Test / smoke (pull_request) Successful in 20s
Details
83a5a963ff
Timmy approved these changes 2026-04-15 05:13:18 +00:00
Dismissed
Timmy left a comment
Owner
Copy Link

Solid accessibility fix. The changes are well-scoped and correct:

  1. Initial focus target: Moving focus from the disabled dismiss button to the always-enabled Call 988 link is the right call — focusing a disabled element is a WCAG violation and breaks screen reader workflows.

  2. Escape key handling: Properly gated behind overlayDismissBtn.disabled check so users cannot dismiss the crisis overlay prematurely during the safety countdown. Good use of e.preventDefault() to avoid interference.

  3. Focus recovery: The crisisOverlay.contains(document.activeElement) guard catches the edge case where focus escapes the overlay (e.g., via browser extension or script), which strengthens the focus trap.

  4. Tests: The three new test cases validate the structural changes at the HTML/JS level. They check for the correct patterns without being brittle.

One minor note: the Escape key handler fires overlayDismissBtn.click() — this assumes the dismiss button's click handler exists and handles cleanup. That dependency is implicit but acceptable given the single-file structure.

Approved — this improves accessibility for a safety-critical overlay.

Solid accessibility fix. The changes are well-scoped and correct: 1. **Initial focus target**: Moving focus from the disabled dismiss button to the always-enabled Call 988 link is the right call — focusing a disabled element is a WCAG violation and breaks screen reader workflows. 2. **Escape key handling**: Properly gated behind `overlayDismissBtn.disabled` check so users cannot dismiss the crisis overlay prematurely during the safety countdown. Good use of `e.preventDefault()` to avoid interference. 3. **Focus recovery**: The `crisisOverlay.contains(document.activeElement)` guard catches the edge case where focus escapes the overlay (e.g., via browser extension or script), which strengthens the focus trap. 4. **Tests**: The three new test cases validate the structural changes at the HTML/JS level. They check for the correct patterns without being brittle. One minor note: the Escape key handler fires `overlayDismissBtn.click()` — this assumes the dismiss button's click handler exists and handles cleanup. That dependency is implicit but acceptable given the single-file structure. Approved — this improves accessibility for a safety-critical overlay.
Timmy approved these changes 2026-04-15 06:21:59 +00:00
Dismissed
Timmy left a comment
Owner
Copy Link

Auto-approved: clean merge, no conflicts.

Auto-approved: clean merge, no conflicts.
Timmy commented 2026-04-15 11:58:30 +00:00
Owner
Copy Link

⚠️ Required CI checks are not passing. Please ensure all tests pass before requesting merge.

⚠️ Required CI checks are not passing. Please ensure all tests pass before requesting merge.
Timmy approved these changes 2026-04-15 14:31:26 +00:00
Timmy left a comment
Owner
Copy Link

Good fix for issue #69. The changes are correct and well-targeted:

  1. Initial focus now goes to the Call 988 link (always enabled) instead of the disabled dismiss button — this is the right UX decision for accessibility.
  2. Escape key handling is properly gated behind !overlayDismissBtn.disabled, preventing premature dismissal during the countdown.
  3. Focus recovery via crisisOverlay.contains(document.activeElement) catches focus escaping the overlay.

Tests are structural (checking HTML source) which is appropriate for this project.

One minor note: the focus recovery check uses crisisOverlay.contains() which is a cleaner approach than what PR #104 does (iterating through focusable elements). If both PRs are intended to merge, there will be a conflict in the focus trap logic — coordinate with PR #104.

Approved.

Good fix for issue #69. The changes are correct and well-targeted: 1. Initial focus now goes to the Call 988 link (always enabled) instead of the disabled dismiss button — this is the right UX decision for accessibility. 2. Escape key handling is properly gated behind `!overlayDismissBtn.disabled`, preventing premature dismissal during the countdown. 3. Focus recovery via `crisisOverlay.contains(document.activeElement)` catches focus escaping the overlay. Tests are structural (checking HTML source) which is appropriate for this project. One minor note: the focus recovery check uses `crisisOverlay.contains()` which is a cleaner approach than what PR #104 does (iterating through focusable elements). If both PRs are intended to merge, there will be a conflict in the focus trap logic — coordinate with PR #104. Approved.
Timmy reviewed 2026-04-15 16:16:23 +00:00
Timmy left a comment
Owner
Copy Link

Review: fix: overlay initial focus targets enabled element

Good fix addressing issue #69 — focuses Call 988 link on overlay open and adds Escape key dismissal. Also adds focus recovery when focus escapes the overlay.

  1. Overlaps with PRs #126, #84, and #104 — all address issue #69 and/or #95. Only one should merge; close duplicates.

  2. The Escape key handler correctly checks !overlayDismissBtn.disabled before dismissing — this prevents premature dismissal during the countdown. Good.

  3. Focus recovery using crisisOverlay.contains(document.activeElement) is cleaner than PR #104's approach of iterating focusable elements. Prefer this version.

Tests look good.

**Review: fix: overlay initial focus targets enabled element** Good fix addressing issue #69 — focuses Call 988 link on overlay open and adds Escape key dismissal. Also adds focus recovery when focus escapes the overlay. 1. **Overlaps with PRs #126, #84, and #104** — all address issue #69 and/or #95. Only one should merge; close duplicates. 2. **The Escape key handler correctly checks `!overlayDismissBtn.disabled`** before dismissing — this prevents premature dismissal during the countdown. Good. 3. **Focus recovery using `crisisOverlay.contains(document.activeElement)`** is cleaner than PR #104's approach of iterating focusable elements. Prefer this version. Tests look good.
Rockachopa commented 2026-04-16 01:47:39 +00:00
Author
Owner
Copy Link

Closing: Superseded by #126.

Closing: Superseded by #126.
Rockachopa closed this pull request 2026-04-16 01:47:41 +00:00
All checks were successful
Sanity Checks / sanity-test (pull_request) Successful in 12s
Details
Smoke Test / smoke (pull_request) Successful in 20s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Timmy
Labels
Clear labels
backend
Backend / API
 batch-pipeline
batch-pipeline label
 content
Content, copy, testimony
 deploy
Deployment / go-live
 epic
Epic / parent issue
 frontend
Frontend / UI
 hardening
Resilience, fallback, security
 infra
Infrastructure, DevOps, deployment
 priority:critical
Must have for launch
 priority:high
Important for launch
 priority:medium
Nice to have at launch
 protocol
Crisis detection protocol
 throughput-10x
throughput-10x label
 token-masterplan
token-masterplan label
 velocity-engine
Auto-generated by velocity engine
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Rockachopa Timmy allegro antigravity bezalel claude codex-agent ezra gemini google grok hermes kimi manus perplexity sonnet
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Timmy_Foundation/the-door#116
Delete Branch "door/issue-69"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?