fix(desktop): close focused threads on Escape#2154
Conversation
Co-authored-by: npub1rf6fvdj6ut0c4kcmjv4p5mmgh89nj58n69uu3fz3cvk3jn500hqs7emz79 <1a7496365ae2df8adb1b932a1a6f68b9cb3950f3d179c8a451c32d194e8f7dc1@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1rf6fvdj6ut0c4kcmjv4p5mmgh89nj58n69uu3fz3cvk3jn500hqs7emz79 <1a7496365ae2df8adb1b932a1a6f68b9cb3950f3d179c8a451c32d194e8f7dc1@sprout-oss.stage.blox.sqprod.co>
b295ea9 to
65161ad
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65161adce5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| event.preventDefault(); | ||
| event.stopImmediatePropagation(); | ||
| onClose(); |
There was a problem hiding this comment.
Because this handler is registered on window in the capture phase, calling stopImmediatePropagation() here prevents any foreground dialog/lightbox opened from a focused thread from seeing Escape. For example, markdown image previews and composer attachment dialogs in the thread already use Escape to close themselves; with this change, pressing Escape closes/unmounts the entire focus drawer instead of dismissing the topmost preview and returning to the focused thread.
Useful? React with 👍 / 👎.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
No behavioral findings on 65161adce5af8ce467fc353120ed9775ce3b0c76. The capture-phase handler intentionally gives focused-thread dismissal priority over composer/autocomplete Escape handling, and the E2E covers that regression. CI is green. Approval is withheld because this head is 12 commits behind current main; per the repository review gate, approval requires a rebased head with fresh green CI.
…rics * origin/main: (96 commits) fix(desktop): prefer live agent mentions (#2149) fix(desktop): close focused threads on Escape (#2154) fix: skip membership lookup for open relays (#2107) fix(desktop): make invite QR downloadable (#2168) Archive managed agents when deleted (#2135) perf(relay): cache Git pack hydration (#2169) chore(deps): update rust crate rustls to v0.23.42 (#2151) chore(deps): update dependency @tanstack/react-virtual to v3.14.6 (#2153) Add Agent Config Core: harness capability model behind agent config surfaces (#2158) chore(deps): update all non-major dependencies (#2152) chore(deps): update rust crate getrandom to v0.4.3 (#2150) fix(relay): bound and observe Git read operations (#2167) fix(desktop): derive default clone URL for relay-hosted repos (#2166) fix(desktop): mask composer rounded corners (#2165) feat(desktop): add PR merge conflict recovery (#2164) fix(desktop): mask scrolled content behind channel and thread composers (#2163) feat(desktop): add inline PR diff comments (#2162) feat(desktop): add commit-scoped PR review decisions (#2161) fix(desktop): batch project work item queries (#2160) feat(desktop): make missing project checkouts actionable (#2159) ...
2 participants
Why
Escape did not reliably exit focused thread mode when focus was inside the thread composer.
What
Risk Assessment
Low — the capture listener exists only while the focused thread drawer is mounted.
References
pnpm --dir desktop build:e2eGenerated with Fizz 🐝