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

fix: implementation for #75 #84

Closed
Rockachopa wants to merge 1 commits from fix/issue-75-1 into main
pull from: fix/issue-75-1
merge into: Timmy_Foundation:main
Conversation 3 Commits 1 Files Changed 2 +65 -7
Rockachopa commented 2026-04-15 01:17:18 +00:00
Owner
Copy Link

Summary:

  • implements the remaining unresolved crisis-overlay safety item called out by triage issue #75: initial focus now lands on the enabled Call 988 link instead of the disabled Continue to chat button
  • hardens the overlay background lock to target the live DOM nodes (#app, #chat-area, #footer) so inert/aria-hidden behavior applies to the real page structure
  • adds regression coverage for initial focus and live DOM locking behavior

Verification:

  • python3 -m pytest tests/test_crisis_overlay_focus_trap.py tests/test_crisis_overlay_initial_focus.py -q
  • browser check on local static server: explicit suicidal message opens overlay, focus lands on Call 988, and #app is inert while active

Notes:

  • issue #75 is a triage/meta issue; this PR addresses the still-open overlay focus item #69 from that work order
  • no new issues discovered in this pass

Refs #75
Closes #69

Summary: - implements the remaining unresolved crisis-overlay safety item called out by triage issue #75: initial focus now lands on the enabled `Call 988` link instead of the disabled `Continue to chat` button - hardens the overlay background lock to target the live DOM nodes (`#app`, `#chat-area`, `#footer`) so inert/aria-hidden behavior applies to the real page structure - adds regression coverage for initial focus and live DOM locking behavior Verification: - `python3 -m pytest tests/test_crisis_overlay_focus_trap.py tests/test_crisis_overlay_initial_focus.py -q` - browser check on local static server: explicit suicidal message opens overlay, focus lands on `Call 988`, and `#app` is inert while active Notes: - issue #75 is a triage/meta issue; this PR addresses the still-open overlay focus item #69 from that work order - no new issues discovered in this pass Refs #75 Closes #69
Rockachopa added 1 commit 2026-04-15 01:17:19 +00:00
fix: implementation for #75
All checks were successful
Sanity Checks / sanity-test (pull_request) Successful in 4s
Details
Smoke Test / smoke (pull_request) Successful in 7s
Details
a8f2b0925f
Timmy approved these changes 2026-04-15 02:23:01 +00:00
Timmy left a comment
Owner
Copy Link

Review: Crisis Overlay Focus & DOM Targeting Fix

Verdict: APPROVED — This is a meaningful safety-critical accessibility fix and is correct on all counts.

What this PR does

Fixes two related issues in the crisis overlay (showOverlay() / dismiss path):

  1. Initial focus now lands on the Call 988 link instead of the disabled Continue to chat button. This is exactly right for a crisis app — a user who triggers the overlay should have immediate keyboard/AT access to the most important action (calling 988), not a button that is disabled and counted down.

  2. DOM selector hardening: replaces three fragile selectors (.app class query, #chat id, footer tag) with the correct live DOM ids (#app, #chat-area, #footer). If those original selectors did not match, the inert/aria-hidden background lock would silently fail, meaning keyboard focus and screen reader access to the underlying page would not be properly suppressed during a crisis overlay. That is a real safety gap.

Specific observations

  • Focus guard is defensive: typeof overlayCallLink.focus === "function" check before calling .focus() is good defensive programming, though in practice all anchor elements will have .focus(). Slightly over-cautious but harmless.
  • Symmetry is correct: both showOverlay() and the dismiss/cleanup path use the same corrected selectors (getElementById calls), so the lock/unlock is symmetric.
  • overlayDismissBtn.focus() is fully removed: confirmed — the old incorrect focus call is gone.
  • Tests are static HTML checks: test_crisis_overlay_initial_focus.py verifies the code patterns via regex against index.html. This is appropriate for a single-file app and the assertions are well-targeted.
  • No regressions introduced: the only changes are selector corrections and the focus target swap — no logic or behavior changes beyond the stated fix.

Minor notes (non-blocking)

  • The test assertNotRegex for overlayDismissBtn.focus() is a good regression guard — keep it.
  • Consider a follow-up to verify the .overlay-call selector actually resolves in the HTML (i.e., that the 988 link has class overlay-call); if that class were ever renamed the focus call would silently become a no-op. But this is out of scope for this PR.

Solid fix for a real accessibility and safety issue. Ready to merge.

## Review: Crisis Overlay Focus & DOM Targeting Fix **Verdict: APPROVED** — This is a meaningful safety-critical accessibility fix and is correct on all counts. ### What this PR does Fixes two related issues in the crisis overlay (`showOverlay()` / dismiss path): 1. **Initial focus now lands on the `Call 988` link** instead of the disabled `Continue to chat` button. This is exactly right for a crisis app — a user who triggers the overlay should have immediate keyboard/AT access to the most important action (calling 988), not a button that is disabled and counted down. 2. **DOM selector hardening**: replaces three fragile selectors (`.app` class query, `#chat` id, `footer` tag) with the correct live DOM ids (`#app`, `#chat-area`, `#footer`). If those original selectors did not match, the `inert`/`aria-hidden` background lock would silently fail, meaning keyboard focus and screen reader access to the underlying page would not be properly suppressed during a crisis overlay. That is a real safety gap. ### Specific observations - **Focus guard is defensive**: `typeof overlayCallLink.focus === "function"` check before calling `.focus()` is good defensive programming, though in practice all anchor elements will have `.focus()`. Slightly over-cautious but harmless. - **Symmetry is correct**: both `showOverlay()` and the dismiss/cleanup path use the same corrected selectors (`getElementById` calls), so the lock/unlock is symmetric. - **`overlayDismissBtn.focus()` is fully removed**: confirmed — the old incorrect focus call is gone. - **Tests are static HTML checks**: `test_crisis_overlay_initial_focus.py` verifies the code patterns via regex against `index.html`. This is appropriate for a single-file app and the assertions are well-targeted. - **No regressions introduced**: the only changes are selector corrections and the focus target swap — no logic or behavior changes beyond the stated fix. ### Minor notes (non-blocking) - The test `assertNotRegex` for `overlayDismissBtn.focus()` is a good regression guard — keep it. - Consider a follow-up to verify the `.overlay-call` selector actually resolves in the HTML (i.e., that the 988 link has class `overlay-call`); if that class were ever renamed the focus call would silently become a no-op. But this is out of scope for this PR. Solid fix for a real accessibility and safety issue. Ready to merge.
Timmy reviewed 2026-04-15 16:16:34 +00:00
Timmy left a comment
Owner
Copy Link

Review: fix: implementation for #75

Fixes overlay initial focus (to Call 988 link) and corrects DOM selectors (.app → #app, #chat → #chat-area, footer → #footer). The selector fixes are valuable — they target elements that actually exist in the DOM.

  1. Overlaps with PRs #126, #116, and #104 for the focus fix portion.
  2. The DOM selector corrections (.app → #app, etc.) appear to be unique to this PR and are genuinely needed — the old selectors referenced non-existent elements, meaning the inert/aria-hidden behavior was silently broken.
  3. typeof overlayCallLink.focus === 'function' check is overly defensive but harmless.

The selector fixes should be extracted and merged independently of the focus fix.

**Review: fix: implementation for #75** Fixes overlay initial focus (to Call 988 link) and corrects DOM selectors (`.app` → `#app`, `#chat` → `#chat-area`, `footer` → `#footer`). The selector fixes are valuable — they target elements that actually exist in the DOM. 1. **Overlaps with PRs #126, #116, and #104** for the focus fix portion. 2. **The DOM selector corrections** (`.app` → `#app`, etc.) appear to be unique to this PR and are genuinely needed — the old selectors referenced non-existent elements, meaning the inert/aria-hidden behavior was silently broken. 3. **`typeof overlayCallLink.focus === 'function'`** check is overly defensive but harmless. The selector fixes should be extracted and merged independently of the focus fix.
Rockachopa commented 2026-04-16 01:48:05 +00:00
Author
Owner
Copy Link

Closing: stale PR, superseded by newer mergeable implementation.

Closing: stale PR, superseded by newer mergeable implementation.
Rockachopa closed this pull request 2026-04-16 01:48:07 +00:00
All checks were successful
Sanity Checks / sanity-test (pull_request) Successful in 4s
Details
Smoke Test / smoke (pull_request) Successful in 7s
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#84
Delete Branch "fix/issue-75-1"

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?