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

Merged

Grandiras merged 1 commit from fix/search-enter-escape into main 2026-10-03 14:39:41 +00:00

Conversation 8 Commits 1 Files changed 6 +36 -39

Grandiras commented 2026-10-03 13:55:50 +00:00

Owner

Copy link

search.spec.js ("search finds a card by what is written on it…") failed intermittently: the card opened from search ignored Escape, and the test then broke at the 960px click because the centred dialog's scrim covered the field.

Rebased onto #248, which landed the same CardModal focus-first reorder and made the spec wait for the dialog's focus before pressing Escape. That hides the member-facing bug instead of fixing it: #248's doc called this the case modal-escape.js "cannot cover". This PR now carries the app fix and its test. The spec presses Escape without waiting, and checks right after Enter that the field let go.

Cause

A focus/Escape log captured during a failing run on an isolated stack:

3265 key Enter  -> INPUT.nav-search-input
3287 modal true
3295 key Escape -> INPUT.nav-search-input      ← field still focused
3308 focusin DIV.modal-backdrop                ← dialog's focus lands 13 ms too late

Enter opened the card but left the search field focused. modal-escape.js only redirects an Escape that targets <body>, so the field's own Escape handler took the key and the dialog stayed open. A member pressing Escape right after opening a card from search ran into the same thing. #241 made the gap wider: CardModal imported the inspector module (two round trips) before it focused anything.

Fix

• search-interop.js: Enter on a listed row blurs the field. A clicked row already lets go of focus by disappearing. SearchBox now opens a row on Enter only while the panel is open, so C# and the JS check ([role=option] in the panel) agree.
• CardModal: focuses first and imports the inspector module afterwards. Now already on main via #248.
• Tests:
  • The spec reads activeElement right after Enter. Against the old search-interop.js this check failed 5/5, so it catches a regression every time, not only when the race hits.
  • The spec asserts that Escape closed the dialog where it happens, not later at line 152.
  • openComposer waits for the title's focus before typing.
  • search.spec.js uses the shared createCard.
  • The spec's stamp is random. Date.now() stamps from runs a few seconds apart were sometimes only two edits apart, and search tolerates two edits, so earlier repeats' cards matched (3/40 under --repeat-each).
• Docs: docs/ui.md (Escape before focus lands) and docs/search.md updated.

No changelog line: before the inspector this was a sub-perceptible race, and the widened window was never released.

Verification

All runs were on an isolated, port-shifted copy of the test stack (ponenteescape):

• before: 1/20 failed, at the stuck Escape
• after: 40/40, plus 20/20 with the db container throttled to 0.25 CPU
• the specs using the changed helper (search, board, boards, filter, attachments, home, mobile, permissions): 63/63
• tools/verify.sh: build and 584 unit tests green

Not touched: share.spec.js has its own copy of createCard, which can hit the same title-focus race.

🤖 Generated with Claude Code

`search.spec.js` ("search finds a card by what is written on it…") failed intermittently: the card opened from search ignored Escape, and the test then broke at the 960px click because the centred dialog's scrim covered the field. > **Rebased onto #248**, which landed the same `CardModal` focus-first reorder and made the spec *wait* for the dialog's focus before pressing Escape. That hides the member-facing bug instead of fixing it: #248's doc called this the case `modal-escape.js` "cannot cover". This PR now carries the app fix and its test. The spec presses Escape without waiting, and checks right after Enter that the field let go. ## Cause A focus/Escape log captured during a failing run on an isolated stack: ``` 3265 key Enter -> INPUT.nav-search-input 3287 modal true 3295 key Escape -> INPUT.nav-search-input ← field still focused 3308 focusin DIV.modal-backdrop ← dialog's focus lands 13 ms too late ``` Enter opened the card but left the search field focused. `modal-escape.js` only redirects an Escape that targets `<body>`, so the field's own Escape handler took the key and the dialog stayed open. A member pressing Escape right after opening a card from search ran into the same thing. #241 made the gap wider: `CardModal` imported the inspector module (two round trips) before it focused anything. ## Fix - **`search-interop.js`**: Enter on a listed row blurs the field. A clicked row already lets go of focus by disappearing. `SearchBox` now opens a row on Enter only while the panel is open, so C# and the JS check (`[role=option]` in the panel) agree. - **`CardModal`**: focuses first and imports the inspector module afterwards. Now already on `main` via #248. - **Tests**: - The spec reads `activeElement` right after Enter. Against the old `search-interop.js` this check failed 5/5, so it catches a regression every time, not only when the race hits. - The spec asserts that Escape closed the dialog where it happens, not later at line 152. - `openComposer` waits for the title's focus before typing. - `search.spec.js` uses the shared `createCard`. - The spec's stamp is random. `Date.now()` stamps from runs a few seconds apart were sometimes only two edits apart, and search tolerates two edits, so earlier repeats' cards matched (3/40 under `--repeat-each`). - **Docs**: `docs/ui.md` (*Escape before focus lands*) and `docs/search.md` updated. No changelog line: before the inspector this was a sub-perceptible race, and the widened window was never released. ## Verification All runs were on an isolated, port-shifted copy of the test stack (`ponenteescape`): - before: 1/20 failed, at the stuck Escape - after: 40/40, plus 20/20 with the db container throttled to 0.25 CPU - the specs using the changed helper (search, board, boards, filter, attachments, home, mobile, permissions): 63/63 - `tools/verify.sh`: build and 584 unit tests green Not touched: `share.spec.js` has its own copy of `createCard`, which can hit the same title-focus race. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit 2026-10-03 13:55:50 +00:00

Let go of the search field when Enter opens a card, so Escape closes it ... 0bd2137f83

Enter on a search row opened the card but left the field focused. Until the
dialog's own focus landed, Escape went to the field, whose handler took it,
and the dialog stayed open: search.spec.js failed about one run in ten.
modal-escape.js only redirects Escape from <body>, so the field now blurs on
Enter, as a clicked row does by vanishing, and C# opens a row only while the
panel shows it.

CardModal also focuses before importing the inspector's module, which had
widened that window by two round trips; openComposer waits for the title's
focus so a test cannot type into the description and lose it to the title.
The spec's stamp is random: two runs' Date.now() can be two edits apart,
which search forgives.

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

opencode-bot commented 2026-10-03 13:55:52 +00:00

Collaborator

Copy link

🤖 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. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-249/run-2026-10-03T13-55-51-436Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented 2026-10-03 13:55:52 +00:00

Collaborator

Copy link

🤖 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/689); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-249/run-689.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes 2026-10-03 13:56:20 +00:00

Dismissed

claude-reviewer left a comment

Copy link

✅ Verdict: approve — no blocking issues found.

The fix matches the cause in the PR's focus log.

• Blur on Enter: it only runs while the panel lists a [role=option]. SearchBox has no @onblur handler, so blurring does not close the panel or interfere with the C# Enter handler.
• C#/JS agreement: Enter when _open && _hits.Count > 0 lines up with the JS check on the rendered panel.
• CardModal: focusing before the inspector import is safe. The Dockable early return keeps the same behaviour as before.
• Tests and docs: the e2e changes are sound. The immediate activeElement check is deterministic, the random stamp removes the near-clock collision, and openComposer now waits for the title's focus. The docs are updated alongside the code.

One small thing, not blocking: share.spec.js still has its own createCard copy with the title-focus race, as the PR notes. It would be worth a follow-up to move it onto the shared helper.

---

🧠 claude-sonnet-5-5 · ⏱ 0m 9s · 3 turns · tokens: 14.9k in / 701 out (+38.8k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The fix matches the cause in the PR's focus log. - **Blur on Enter:** it only runs while the panel lists a `[role=option]`. `SearchBox` has no `@onblur` handler, so blurring does not close the panel or interfere with the C# Enter handler. - **C#/JS agreement:** `Enter when _open && _hits.Count > 0` lines up with the JS check on the rendered panel. - **`CardModal`:** focusing before the inspector import is safe. The `Dockable` early return keeps the same behaviour as before. - **Tests and docs:** the e2e changes are sound. The immediate `activeElement` check is deterministic, the random stamp removes the near-clock collision, and `openComposer` now waits for the title's focus. The docs are updated alongside the code. One small thing, not blocking: `share.spec.js` still has its own `createCard` copy with the title-focus race, as the PR notes. It would be worth a follow-up to move it onto the shared helper. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 9s · 3 turns · tokens: 14.9k in / 701 out (+38.8k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-249/run-689.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:0bd2137f839ebe35291c93e0d2a7e02e115dbcd8 -->

opencode-reviewer approved these changes 2026-10-03 14:17:37 +00:00

Dismissed

opencode-reviewer left a comment

Copy link

✅ Verdict: approve — no blocking issues found.

Good change. The diagnosis is coherent and each fix is the minimal one:

• search-interop.js: the JS listsHits check (panel in DOM with [role="option"]) and C#'s Enter when _open && _hits.Count > 0 now agree, and since the JS keydown listener is on the document (bubble phase, after Blazor's own listener on the input), the blur happens synchronously during the same keydown dispatch — so Playwright reading activeElement right after press('Enter') really is deterministic, not another race. Escape then targets <body> and modal-escape.js handles it, as the captured log wanted.
• CardModal: focusing before the inspector import is right; FocusAsync was already awaited on this path, so no new failure mode. For !Dockable the early return also skips nothing else.
• Tests/build: openComposer now waits for the title's focus before any typing, which protects every caller that fills fields after opening (checked the existing call sites — none assumed a non-focused dialog). The random 8-hex stamp is the right call: two random stamps within two edits has ~10⁻⁴ probability versus Date.now()'s systematic near-misses. The removed local helper's object-form callers are all in the shared createCard now, and share.spec.js's private copy is unaffected.

Two non-blocking notes:

• Mid-IME Enter: with a composition in progress (CJK input), the DOM Enter keydown has isComposing: true, and it now also blurs the field and (as before, via C#) navigates. That would be an odd way to commit a composition. The C# side already navigated in that case, so this PR doesn't make it worse, but listsHits could skip isComposing events the way modal-escape.js's capture listener does — a candidate for a follow-up rather than this PR.
• Skipping the Changelog.Unreleased bullet looks defensible given the widened window was never released (orientation.md gates the bullet on "a member would notice"); just noting the house rule so the choice is visible.

---

⏱ 21m 46s · 16 steps · $0.0468 · tokens: 233.4k in / 1.1k out (+138.1k cache-read) · Go: 5h 0% · wk 10% · mo 88%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Good change. The diagnosis is coherent and each fix is the minimal one: - **`search-interop.js`**: the JS `listsHits` check (panel in DOM with `[role="option"]`) and C#'s `Enter when _open && _hits.Count > 0` now agree, and since the JS keydown listener is on the document (bubble phase, after Blazor's own listener on the input), the blur happens synchronously during the same keydown dispatch — so Playwright reading `activeElement` right after `press('Enter')` really is deterministic, not another race. Escape then targets `<body>` and `modal-escape.js` handles it, as the captured log wanted. - **`CardModal`**: focusing before the inspector import is right; `FocusAsync` was already awaited on this path, so no new failure mode. For `!Dockable` the early return also skips nothing else. - **Tests/build**: `openComposer` now waits for the title's focus before any typing, which protects every caller that fills fields after opening (checked the existing call sites — none assumed a non-focused dialog). The random 8-hex stamp is the right call: two random stamps within two edits has ~10⁻⁴ probability versus Date.now()'s systematic near-misses. The removed local helper's object-form callers are all in the shared `createCard` now, and `share.spec.js`'s private copy is unaffected. Two non-blocking notes: - Mid-IME Enter: with a composition in progress (CJK input), the DOM Enter keydown has `isComposing: true`, and it now also blurs the field and (as before, via C#) navigates. That would be an odd way to commit a composition. The C# side already navigated in that case, so this PR doesn't make it worse, but `listsHits` could skip `isComposing` events the way `modal-escape.js`'s capture listener does — a candidate for a follow-up rather than this PR. - Skipping the `Changelog.Unreleased` bullet looks defensible given the widened window was never released (orientation.md gates the bullet on "a member would notice"); just noting the house rule so the choice is visible. --- ⏱ 21m 46s · 16 steps · $0.0468 · tokens: 233.4k in / 1.1k out (+138.1k cache-read) · Go: 5h 0% · wk 10% · mo 88% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-249/run-2026-10-03T13-55-51-436Z.md) <!-- opencode-reviewed-head:0bd2137f839ebe35291c93e0d2a7e02e115dbcd8 --> <!-- opencode-verdict:approve -->

Grandiras force-pushed fix/search-enter-escape from 0bd2137f83 to 9ce5ba0c54 2026-10-03 14:20:58 +00:00 Compare

opencode-bot commented 2026-10-03 14:22:00 +00:00

Collaborator

Copy link

🤖 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. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-249/run-2026-10-03T14-22-00-172Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented 2026-10-03 14:22:02 +00:00

Collaborator

Copy link

🤖 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/690); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-249/run-690.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes 2026-10-03 14:22:20 +00:00

claude-reviewer left a comment

Copy link

✅ Verdict: approve — no blocking issues found.

I didn't re-read the files after the rebase onto #248. This is based on the diff and my earlier pass, and my earlier approval still holds.

• Rebase: The CardModal reorder is now on main via #248. The diff keeps only the app fix, which is the Enter blur in search-interop.js plus the _open guard in SearchBox.
• Spec: The spec now checks activeElement right after Enter instead of waiting for the dialog's focus. That tests the member-facing bug rather than hiding it.
• Docs: ui.md and search.md are updated to match.

Non-blocking: share.spec.js still has its own createCard copy with the title-focus race. It would be worth moving it onto the shared helper in a follow-up.

---

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

✅ **Verdict: approve** — no blocking issues found. I didn't re-read the files after the rebase onto #248. This is based on the diff and my earlier pass, and my earlier approval still holds. - **Rebase:** The `CardModal` reorder is now on `main` via #248. The diff keeps only the app fix, which is the Enter blur in `search-interop.js` plus the `_open` guard in `SearchBox`. - **Spec:** The spec now checks `activeElement` right after Enter instead of waiting for the dialog's focus. That tests the member-facing bug rather than hiding it. - **Docs:** `ui.md` and `search.md` are updated to match. Non-blocking: `share.spec.js` still has its own `createCard` copy with the title-focus race. It would be worth moving it onto the shared helper in a follow-up. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 5s · 2 turns · tokens: 14.9k in / 424 out (+12.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-249/run-690.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:9ce5ba0c540a4be8e3121b68b6bbbe1f89cb1865 -->

opencode-reviewer approved these changes 2026-10-03 14:22:48 +00:00

opencode-reviewer left a comment

Copy link

✅ Verdict: approve — no blocking issues found.

Re-review: the rebase onto #248 is clean and the state in the tree matches the PR description end to end.

• App fix: search-interop.js blurs the field synchronously in the document-level keydown listener on the bubble phase — Blazor's input handler runs during the target phase, so C# still opens the row, and activeElement is off the field by the time Playwright's press('Enter') resolves. The immediate activeElement check in the spec is therefore deterministic, not another race. modal-escape.js only redirects Escape from <body>/<html>, so a blurred field's Escape reaches it and closes the dialog; the spec's removal of the former backdrop-focus wait before pressing Escape is exactly what tests the fixed behaviour, and the assertion now sits where the race happens rather than downstream at the 960px block.
• C#/JS agreement: Enter when _open && _hits.Count > 0 in SearchBox.razor matches listsHits (panel in DOM with [role="option"]); tips rows are not options, so the tips-only Enter case no longer navigates. Other keys fall through listsHits correctly, and the slash shortcut is still gated by isTyping.
• CardModal (already on main via #248): OnAfterRenderAsync awaits the title/backdrop FocusAsync before importing inspector.js, closing the widened window.
• Helpers: openComposer's title-focus wait protects shared createCard, whose { description } form covers the spec's converted call sites. The random 8-hex stamp removes Date.now()'s near-clock collisions.

Non-blocking, unchanged from last pass and both acknowledged in the PR: share.spec.js keeps its own createCard copy (worth a follow-up), and a mid-IME Enter now also blurs — listsHits could skip isComposing events like modal-escape.js's capture listener does.

---

⏱ 0m 48s · 6 steps · $0.0073 · tokens: 29.4k in / 762 out (+54.4k cache-read) · Go: 5h 0% · wk 10% · mo 88%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Re-review: the rebase onto #248 is clean and the state in the tree matches the PR description end to end. - **App fix**: `search-interop.js` blurs the field synchronously in the document-level keydown listener on the bubble phase — Blazor's input handler runs during the target phase, so C# still opens the row, and `activeElement` is off the field by the time Playwright's `press('Enter')` resolves. The immediate `activeElement` check in the spec is therefore deterministic, not another race. `modal-escape.js` only redirects Escape from `<body>`/`<html>`, so a blurred field's Escape reaches it and closes the dialog; the spec's removal of the former backdrop-focus wait before pressing Escape is exactly what tests the fixed behaviour, and the assertion now sits where the race happens rather than downstream at the 960px block. - **C#/JS agreement**: `Enter when _open && _hits.Count > 0` in `SearchBox.razor` matches `listsHits` (panel in DOM with `[role="option"]`); tips rows are not options, so the tips-only Enter case no longer navigates. Other keys fall through `listsHits` correctly, and the slash shortcut is still gated by `isTyping`. - **`CardModal`** (already on `main` via #248): `OnAfterRenderAsync` awaits the title/backdrop `FocusAsync` before importing `inspector.js`, closing the widened window. - **Helpers**: `openComposer`'s title-focus wait protects shared `createCard`, whose `{ description }` form covers the spec's converted call sites. The random 8-hex stamp removes `Date.now()`'s near-clock collisions. Non-blocking, unchanged from last pass and both acknowledged in the PR: `share.spec.js` keeps its own `createCard` copy (worth a follow-up), and a mid-IME Enter now also blurs — `listsHits` could skip `isComposing` events like `modal-escape.js`'s capture listener does. --- ⏱ 0m 48s · 6 steps · $0.0073 · tokens: 29.4k in / 762 out (+54.4k cache-read) · Go: 5h 0% · wk 10% · mo 88% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-249/run-2026-10-03T14-22-00-172Z.md) <!-- opencode-reviewed-head:9ce5ba0c540a4be8e3121b68b6bbbe1f89cb1865 --> <!-- opencode-verdict:approve -->

Grandiras merged commit 50997eec5a into main 2026-10-03 14:39:41 +00:00

Grandiras referenced this pull request from a commit 2026-10-03 14:39:42 +00:00

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

Grandiras referenced this pull request 2026-10-03 14:44:08 +00:00

Use the shared createCard in share.spec.js, so a description never lands in the title #250

Grandiras referenced this pull request from a commit 2026-10-03 14:45:23 +00:00

Use the shared createCard in share.spec.js, so a description never lands in the title (#250)

Grandiras referenced this pull request 2026-10-04 15:23:57 +00:00

Make search the v2 palette: centred on desktop, full screen on a phone #254

Sign in to join this conversation.

Reviewers

No reviewers

claude-reviewer

opencode-reviewer

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

5 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Grandiras/Ponente!249

No description provided.