Claude: changes for issue #173 #175

Merged

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

2026-08-26 23:54:05 +00:00

claude-bot commented

2026-08-26 23:51:58 +00:00

Collaborator

Closes #173.

claude-bot added 1 commit

2026-08-26 23:51:58 +00:00

Escape closes the calendar's day sheet and subscribe panel

CI / build (pull_request) Successful in 37s

Details

c5ae3cff15

Both overlays declared themselves dialogs and had no keyboard handling at
all: no @onkeydown, no tabindex, nothing focused when they opened. They
closed only by clicking the overlay or the ✕, while every other dialog in
the app closes on Escape.

They now carry the shape the other three use — a tabindex="-1" layer with
an @onkeydown, focused from OnAfterRenderAsync on the render that opened
it. Focus goes to the overlay rather than to a control inside it, for
ChangelogModal's reason: neither dialog is here to be typed into.

The class stays .calendar-overlay and modal-escape.js names it alongside
.modal-backdrop, rather than the other way round: the class carries this
page's layering (z-index 900, under the card dialog's 1000, and no blur),
so renaming it would hand these two the modal's looks along with its
keyboard. Its "last in document order is topmost" rule still holds across
both classes — the only pair that can share the screen is an overlay and
the CardModal opened from it, which Calendar.razor renders after both.

calendar.spec.js presses Escape on each, in the fixture that already
opened them: on the day sheet immediately, which is the modal-escape.js
path (the cell that opens it is a div and takes no focus), and on the
subscribe panel from inside it, which is the bubbling path. The panel's
test also pins the overlay taking focus, because the Subscribe button is
a button and keeps it otherwise.

Closes #173

claude-bot referenced this pull request

2026-08-26 23:51:59 +00:00

Escape does not close either of the calendar's two dialogs #173

claude-bot commented

2026-08-26 23:52:01 +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/404); 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-175/run-404.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-26 23:53:38 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Extends the modal-escape fix (#165/#171 era work) to the calendar's day sheet and subscribe panel, following the exact CardModal/ChangelogModal/FeedbackModal shape: tabindex="-1" + @onkeydown on the overlay, focused a round trip late from OnAfterRenderAsync, with modal-escape.js widened to also match .calendar-overlay.

Verified against the current file:

FocusOverlayAsync's guard (_dayDetail is not null && _cards is not null) matches the @if that actually renders _dayOverlay (_dayDetail is { } detailDay && _cards is not null), so the ElementReference is never default when FocusAsync is called — the documented crash risk is correctly avoided.
The day sheet and subscribe panel can't be open simultaneously (different triggers, and the subscribe panel's overlay would block interaction), so sharing one _focusOverlay flag between both refs is safe.
OpenCard sets _dayDetail = null directly (bypassing CloseDay) without clearing _focusOverlay, but that's harmless since the flag is already consumed by the time a card can be picked from the sheet.
modal-escape.js's selector change and "last in document order is topmost" reasoning check out against app.css (.calendar-overlay z-index 900 vs .modal-backdrop/card modal 1000) and against Calendar.razor's render order (both overlays before CardModal).
KeyboardEventArgs is available via the global _Imports.razor using, consistent with OnKeyDown in the other three modals.

Docs (calendar.md, frontend.md, ui.md), the changelog entries in both locales, and the new e2e coverage (Escape closing the day sheet, and an activeElement poll proving the subscribe overlay actually takes focus before Escape closing it from a focused control) all line up with the code change. No issues found.

⏱ 1m 25s · 19 turns · tokens: 39.9k in / 6.6k out (+1015.4k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Extends the modal-escape fix (#165/#171 era work) to the calendar's day sheet and subscribe panel, following the exact CardModal/ChangelogModal/FeedbackModal shape: `tabindex="-1"` + `@onkeydown` on the overlay, focused a round trip late from `OnAfterRenderAsync`, with `modal-escape.js` widened to also match `.calendar-overlay`. Verified against the current file: - `FocusOverlayAsync`'s guard (`_dayDetail is not null && _cards is not null`) matches the `@if` that actually renders `_dayOverlay` (`_dayDetail is { } detailDay && _cards is not null`), so the ElementReference is never `default` when `FocusAsync` is called — the documented crash risk is correctly avoided. - The day sheet and subscribe panel can't be open simultaneously (different triggers, and the subscribe panel's overlay would block interaction), so sharing one `_focusOverlay` flag between both refs is safe. - `OpenCard` sets `_dayDetail = null` directly (bypassing `CloseDay`) without clearing `_focusOverlay`, but that's harmless since the flag is already consumed by the time a card can be picked from the sheet. - `modal-escape.js`'s selector change and "last in document order is topmost" reasoning check out against `app.css` (`.calendar-overlay` z-index 900 vs `.modal-backdrop`/card modal 1000) and against `Calendar.razor`'s render order (both overlays before `CardModal`). - `KeyboardEventArgs` is available via the global `_Imports.razor` using, consistent with `OnKeyDown` in the other three modals. Docs (`calendar.md`, `frontend.md`, `ui.md`), the changelog entries in both locales, and the new e2e coverage (Escape closing the day sheet, and an `activeElement` poll proving the subscribe overlay actually takes focus before Escape closing it from a focused control) all line up with the code change. No issues found. --- ⏱ 1m 25s · 19 turns · tokens: 39.9k in / 6.6k out (+1015.4k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-175/run-404.md)