Claude: changes for issue #165 #172

Merged

Grandiras merged 1 commit from claude/issue-165 into main

2026-08-26 23:19:34 +00:00

claude-bot commented

2026-08-26 23:18:14 +00:00

Collaborator

Closes #165.

claude-bot added 1 commit

2026-08-26 23:18:14 +00:00

Escape closes a dialog opened a moment ago

CI / build (pull_request) Successful in 33s

Details

2130c3fd94

The three modals listen for Escape with an @onkeydown on their
.modal-backdrop and are focused from OnAfterRenderAsync. That focus call
is not part of the render that put the dialog on screen — in Blazor
Server the batch goes out, the browser acknowledges it, the server then
runs OnAfterRenderAsync, and a second interop message comes back saying
"focus this" — so for one and a half round trips activeElement is
<body>, which no component owns, and the keydown is routed nowhere.

Measured at ~5ms on loopback with the server idle; it is the circuit's
latency, so it is however far away the server is. It made board.spec.js's
"edit a card, verify changes persist" fail about one run in three on its
last assertion (#165), and to a member it is a dialog that ignores the
first Escape after it opens.

js/modal-escape.js redirects the keypress: a delegated keydown listener
on the document — outside the circuit, so it arms without interop and
survives a reconnect — which on Escape with *nothing* focused focuses the
topmost backdrop and re-dispatches the event there, where Blazor's own
delegated listener routes it as the backdrop's own. The guard is exactly
"nothing focused", which is the broken state and nothing else: every real
Escape handler in the app needs a focused element to receive the event,
so this can never take a keypress off one.

The test now blurs before pressing Escape, asking for the losing state
every run instead of racing for it. Verified against a host-side dev
server: 15/15 with the fix, 3/3 failing at that assertion with the JS
emptied.

Closes #165

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude-bot referenced this pull request

2026-08-26 23:18:15 +00:00

Flaky e2e: "edit a card, verify changes persist" fails ~1 run in 3 on Escape #165

claude-bot commented

2026-08-26 23:18:18 +00:00

Author

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/399); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-172/run-399.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-26 23:19:14 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Solid, well-scoped fix for the Escape-race described in #165. Verified the mechanics directly against the code: all three modals (CardModal, FeedbackModal, ChangelogModal) put tabindex="-1" on .modal-backdrop (so top.focus() in the new script actually works) and their OnKeyDown handlers only check e.Key == "Escape", which is exactly what the synthetic KeyboardEvent({key: 'Escape', code: 'Escape', bubbles: true}) supplies. The re-dispatch guard (e.target !== document.body && e.target !== document.documentElement) correctly prevents the synthetic, re-targeted event from being caught and reprocessed by the same listener, so there's no infinite loop. The "last in document order is topmost" assumption for stacked modals is reasonable given how these are appended, and is called out explicitly as an assumption in the comments.

The Playwright fix (blur() before pressing Escape) correctly reproduces the exact race window described, rather than papering over flakiness with a retry or timeout.

Documentation (docs/frontend.md, docs/ui.md) and inline comments are unusually thorough and explain both the why and the explicit non-solutions (don't move the handler to whatever has focus, don't reach for the close button) — consistent with this repo's conventions. Changelog entries are present in both locales. No correctness, security, or omission issues found.

⏱ 0m 44s · 7 turns · tokens: 26.8k in / 3.1k out (+269.8k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid, well-scoped fix for the Escape-race described in #165. Verified the mechanics directly against the code: all three modals (`CardModal`, `FeedbackModal`, `ChangelogModal`) put `tabindex="-1"` on `.modal-backdrop` (so `top.focus()` in the new script actually works) and their `OnKeyDown` handlers only check `e.Key == "Escape"`, which is exactly what the synthetic `KeyboardEvent({key: 'Escape', code: 'Escape', bubbles: true})` supplies. The re-dispatch guard (`e.target !== document.body && e.target !== document.documentElement`) correctly prevents the synthetic, re-targeted event from being caught and reprocessed by the same listener, so there's no infinite loop. The "last in document order is topmost" assumption for stacked modals is reasonable given how these are appended, and is called out explicitly as an assumption in the comments. The Playwright fix (`blur()` before pressing Escape) correctly reproduces the exact race window described, rather than papering over flakiness with a retry or timeout. Documentation (`docs/frontend.md`, `docs/ui.md`) and inline comments are unusually thorough and explain both the *why* and the explicit non-solutions (don't move the handler to whatever has focus, don't reach for the close button) — consistent with this repo's conventions. Changelog entries are present in both locales. No correctness, security, or omission issues found. --- ⏱ 0m 44s · 7 turns · tokens: 26.8k in / 3.1k out (+269.8k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-172/run-399.md)