Grandiras/Ponente

Fork 0

Close the search hit's card before the 960px check, and focus a card dialog first #248

Merged

Grandiras merged 1 commit from fix/search-spec-inspector into main

2026-10-03 13:52:17 +00:00

Grandiras commented

2026-10-03 13:48:31 +00:00

Owner

search.spec.js › search finds a card by what is written on it, and by who and when failed in full runs: a 60 s timeout clicking .nav-search-input, intercepted by .modal-backdrop-dockable. It passed when run alone.

Cause

It wasn't createCard. Enter on a search hit opens the card, and the spec pressed Escape as soon as ?card= left the URL. SearchBox keeps focus in its field until CardModal focuses its backdrop in OnAfterRenderAsync. An Escape before that goes to the search field, and modal-escape.js only redirects from <body>, so the inspector stayed open. At 1280px it sits on the right and doesn't cover the sidebar search, so the next click still worked. At 960px it becomes the centred dialog, and the click on the search box at search.spec.js:152 waited until the test timed out.

#241 made this likely: CardModal imports inspector.js and calls watch before focusing, so focus now lands three round trips after the dialog shows instead of one.

Fix

Spec: wait for the backdrop to take focus before Escape, and check that the dialog closed. That makes it deterministic and pins the keyboard flow (Enter on a hit moves focus into the card).
App: CardModal focuses first, then wires the inspector, which restores the one-round-trip gap from before #241. It also shortens the window in which a new card's title drops typing. #241 is unreleased, so there's no changelog line.
docs/ui.md (Escape before focus lands) records the order and the search-field case.

Verified

All runs used an isolated, port-shifted copy (ponentesearch, ports 1809x). The shared stack wasn't touched.

Full suite on main (84c1e46): 161 passed, 1 failed (this test, at line 152). The screenshot showed the hit's card still open, centred.
A throwaway spec with a 60 ms WebSocket lag showed focus still on .nav-search-input when Escape landed, and the dialog stayed open.
With the fix: search.spec.js --repeat-each=3 9/9, full suite 162/162, tools/verify.sh 584/584.

🤖 Generated with Claude Code

`search.spec.js` › *search finds a card by what is written on it, and by who and when* failed in full runs: a 60 s timeout clicking `.nav-search-input`, intercepted by `.modal-backdrop-dockable`. It passed when run alone. ## Cause It wasn't `createCard`. Enter on a search hit opens the card, and the spec pressed Escape as soon as `?card=` left the URL. `SearchBox` keeps focus in its field until `CardModal` focuses its backdrop in `OnAfterRenderAsync`. An Escape before that goes to the search field, and `modal-escape.js` only redirects from `<body>`, so the inspector stayed open. At 1280px it sits on the right and doesn't cover the sidebar search, so the next click still worked. At 960px it becomes the centred dialog, and the click on the search box at `search.spec.js:152` waited until the test timed out. #241 made this likely: `CardModal` imports `inspector.js` and calls `watch` before focusing, so focus now lands three round trips after the dialog shows instead of one. ## Fix - **Spec**: wait for the backdrop to take focus before Escape, and check that the dialog closed. That makes it deterministic and pins the keyboard flow (Enter on a hit moves focus into the card). - **App**: `CardModal` focuses first, then wires the inspector, which restores the one-round-trip gap from before #241. It also shortens the window in which a new card's title drops typing. #241 is unreleased, so there's no changelog line. - `docs/ui.md` (*Escape before focus lands*) records the order and the search-field case. ## Verified All runs used an isolated, port-shifted copy (`ponentesearch`, ports 1809x). The shared stack wasn't touched. - Full suite on `main` (84c1e46): 161 passed, 1 failed (this test, at line 152). The screenshot showed the hit's card still open, centred. - A throwaway spec with a 60 ms WebSocket lag showed focus still on `.nav-search-input` when Escape landed, and the dialog stayed open. - With the fix: `search.spec.js --repeat-each=3` 9/9, full suite **162/162**, `tools/verify.sh` 584/584. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-10-03 13:48:31 +00:00

Close the search hit's card before the 960px check, and focus a card dialog first

CI / build (pull_request) Successful in 41s

Details

e43764caad

search.spec's Enter on a hit opens the card, then pressed Escape as soon as
the query left the URL. The search field keeps focus until the dialog takes
it, so an early Escape went to the field and the inspector stayed open; at
960px it is a centred dialog, whose backdrop then swallowed the click on the
search box until the test timed out. The spec now waits for the backdrop's
focus and checks the dialog closed.

#241 had CardModal import and wire inspector.js before focusing, so focus
landed three round trips after the dialog showed instead of one: longer for
an Escape from a field outside it to miss, and for a new card's title to drop
typing. Focus goes first again.

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

opencode-bot commented

2026-10-03 13:48:34 +00:00

Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-248/run-2026-10-03T13-48-33-653Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented

2026-10-03 13:48:36 +00:00

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/688); 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-248/run-688.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-10-03 13:49:05 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The root cause is right: an Escape sent while focus is still in the search field is neither redirected by modal-escape.js (it only acts from <body>) nor handled by the dialog. Focusing before the inspector.js import and watch call removes two interop round trips from that window. The Dockable fallback still catches JSException, so a failure in the module path can't affect focus. The spec now waits for .modal-backdrop to take focus and then asserts the dialog closed, so a missed Escape fails at the right line instead of timing out later at 960px. docs/ui.md records the order and the reason, and omitting a changelog line is right since #241 is unreleased. I did not run the specs. The PR reports the full e2e suite at 162/162, search.spec.js --repeat-each=3 at 9/9 and tools/verify.sh at 584/584.

Not blocking: the window is shortened, not closed. A very fast human Escape right after Enter on a hit could still reach the search field. Closing it would need SearchBox to blur its field when it opens a card.

🧠 claude-sonnet-5-5 · ⏱ 0m 11s · 2 turns · tokens: 11.0k in / 1.3k out (+12.2k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The root cause is right: an Escape sent while focus is still in the search field is neither redirected by `modal-escape.js` (it only acts from `<body>`) nor handled by the dialog. Focusing before the `inspector.js` import and `watch` call removes two interop round trips from that window. The `Dockable` fallback still catches `JSException`, so a failure in the module path can't affect focus. The spec now waits for `.modal-backdrop` to take focus and then asserts the dialog closed, so a missed Escape fails at the right line instead of timing out later at 960px. `docs/ui.md` records the order and the reason, and omitting a changelog line is right since #241 is unreleased. I did not run the specs. The PR reports the full e2e suite at 162/162, `search.spec.js --repeat-each=3` at 9/9 and `tools/verify.sh` at 584/584. Not blocking: the window is shortened, not closed. A very fast human Escape right after Enter on a hit could still reach the search field. Closing it would need `SearchBox` to blur its field when it opens a card. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 11s · 2 turns · tokens: 11.0k in / 1.3k out (+12.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-248/run-688.md)

opencode-reviewer approved these changes

2026-10-03 13:50:35 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Good to merge. The reorder in CardModal.OnAfterRenderAsync (focus first, then the inspector's import + watch) restores the one-round-trip focus gap and keeps the catch (JSException) as the last awaited op, so the focus is never skipped by a dock failure. The spec now waits for .modal-backdrop to take focus before pressing Escape and asserts the dialog actually closed, which pins the Enter-on-hit → dialog → Escape flow deterministically instead of racing the SignalR/JS round trips. The docs/ui.md addition documents the ordering and the search-field case on the page that already covers Escape-before-focus; no changelog line is correct since #241 is unreleased and this only returns its behaviour. No correctness or security concerns.

⏱ 2m 2s · 4 steps · $0.0039 · tokens: 14.8k in / 606 out (+22.8k cache-read) · Go: 5h 0% · wk 10% · mo 87%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Good to merge. The reorder in `CardModal.OnAfterRenderAsync` (focus first, then the inspector's import + `watch`) restores the one-round-trip focus gap and keeps the `catch (JSException)` as the last awaited op, so the focus is never skipped by a dock failure. The spec now waits for `.modal-backdrop` to take focus before pressing Escape and asserts the dialog actually closed, which pins the Enter-on-hit → dialog → Escape flow deterministically instead of racing the SignalR/JS round trips. The `docs/ui.md` addition documents the ordering and the search-field case on the page that already covers Escape-before-focus; no changelog line is correct since #241 is unreleased and this only returns its behaviour. No correctness or security concerns. --- ⏱ 2m 2s · 4 steps · $0.0039 · tokens: 14.8k in / 606 out (+22.8k cache-read) · Go: 5h 0% · wk 10% · mo 87% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-248/run-2026-10-03T13-48-33-653Z.md)

Grandiras merged commit 2834faf372 into main

2026-10-03 13:52:17 +00:00

Grandiras deleted branch fix/search-spec-inspector

2026-10-03 13:52:17 +00:00

Grandiras referenced this pull request from a commit

2026-10-03 13:52:19 +00:00

Close the search hit's card before the 960px check, and focus a card dialog first (#248)

Grandiras referenced this pull request

2026-10-03 14:20:58 +00:00

Let go of the search field when Enter opens a card, so Escape closes it #249

Grandiras referenced this pull request from a commit

2026-10-03 14:20:58 +00:00