Skip to content

feat(web): Escape returns focus to the composer when nothing else wants it - #169

Merged
yordis merged 5 commits into
mainfrom
yordis/feat-esc-focus-composer
Oct 27, 2026
Merged

yordis merged 5 commits into
mainfrom
yordis/feat-esc-focus-composer

Conversation

@yordis

@yordis yordis commented Oct 27, 2026

Copy link
Copy Markdown
Member
  • After clicking into the timeline or dismissing a popup, focus is left on nothing and getting back to the composer takes a mouse trip.
  • Escape is the natural "get me back to typing" key, and it already does nothing in that state, so claiming it there costs no existing behavior.

…ts it

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Global Escape handling now competes with thread find, citations, sidebar drags, and MCP fullscreen dismissal; guards and tests reduce regressions but keyboard routing remains sensitive.

Overview
Escape can return focus to the chat composer when the user is not in an input, overlay, or other handler that should keep it—addressing the case where focus lands on “nothing” after clicking the timeline or closing UI.

useChatShortcuts extends the existing Escape path (thread find still closes first) with shouldEscapeFocusComposer, which mirrors type-to-focus guards and additionally skips non-collapsed text selection and fullscreen MCP app surfaces so native popover light-dismiss still works. shouldEscapeFocusComposer lives in composerFocusRedirect.ts.

Several features now claim Escape only when they actually handle it: sidebar pointer drags call preventDefault on cancel; citation dismissal prevents default when dismiss runs; tests lock in that behavior.

Browser tests cover composer focus, focused fields, selection, MCP fullscreen, and already-claimed Escape.

Reviewed by Cursor Bugbot for commit 3521a7d. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S labels Oct 27, 2026
…es the composer

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@github-actions github-actions Bot added size:M and removed size:S labels Oct 27, 2026

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 9387fd1 and 3521a7d.

Files selected for processing: 7
  • apps/web/src/components/Sidebar.pointer.browser.test.tsx
  • apps/web/src/components/Sidebar.pointer.ts
  • apps/web/src/components/chat/composerFocusRedirect.ts
  • apps/web/src/components/chat/useAssistantCitationTarget.browser.test.tsx
  • apps/web/src/components/chat/useAssistantCitationTarget.ts
  • apps/web/src/components/chat/useChatShortcuts.browser.test.tsx
  • apps/web/src/components/chat/useChatShortcuts.ts

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 87c6089 and 9387fd1.

Files selected for processing: 3
  • apps/web/src/components/chat/composerFocusRedirect.ts
  • apps/web/src/components/chat/useChatShortcuts.browser.test.tsx
  • apps/web/src/components/chat/useChatShortcuts.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

Escape can close active thread find or move focus to the composer when the event passes the focus checks. Browser tests cover cases that allow or prevent the focus change.

Changes: Escape focus handling

Layer / File(s) Summary
Escape focus eligibility
apps/web/src/components/chat/composerFocusRedirect.ts
Added shouldEscapeFocusComposer to check key, composition, modifiers, input redirection, and selection state.
Escape routing and browser tests
apps/web/src/components/chat/useChatShortcuts.ts, apps/web/src/components/chat/useChatShortcuts.browser.test.tsx
The Escape handler closes active thread find. Otherwise, it focuses the composer at its end when shortcuts are enabled and the event qualifies. Browser tests cover focus outcomes and prevented events.

Priority: Pending latest changes

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Window
  participant useChatShortcuts
  participant shouldEscapeFocusComposer
  participant Composer
  Window->>useChatShortcuts: Dispatch Escape keydown
  useChatShortcuts->>shouldEscapeFocusComposer: Check event eligibility
  shouldEscapeFocusComposer-->>useChatShortcuts: Return eligibility
  useChatShortcuts->>Composer: Focus at end when eligible
Loading

Suggested labels: size:M

Suggested reviewers: juliusmarminge

Merge Risk: Low · previous review 9387f

Dismissing a citation can also focus the composer. The impact is limited and does not block merging.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description gives two short bullets. The first describes the problem: focus is left on nothing after clicking into the timeline or dismissing a popup, so returning to the composer requires a mouse… Rewrite the description to follow the repository template. Add these sections:

1. Problem: State the problem in one or two sentences. Include the expected behavior and reproduction steps. For example: after clicking the timeline or dismiss…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title "feat(web): Escape returns focus to the composer when nothing else wants it" matches the main change. The raw summary shows a new shouldEscapeFocusComposer handler in useChatShortcuts that f…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "feat(web): Escape returns focus to the composer when nothing else wants it" matches the main change. The raw summary shows a new shouldEscapeFocusComposer handler in useChatShortcuts that focuses the composer on Escape when thread find is inactive and the event qualifies. The title follows the conventional commit format and is specific to the Escape key behavior. It omits the guard conditions, but it does not need to list every detail. The title is 74 characters, which is acceptable.

Full details: Description check

Explanation

The description gives two short bullets. The first describes the problem: focus is left on nothing after clicking into the timeline or dismissing a popup, so returning to the composer requires a mouse. The second gives a rationale: Escape already does nothing in that state. The template requires a Problem section, a Change section, a Scope and approval section, and a Verification section. The description has no headings and no Change explanation. It has no link to a triaged issue or maintainer approval. It has no verification steps, no tests described, and no statement of what was not checked. It also does not state the expected behavior or reproduction steps in a structured way. The description is therefore largely incomplete against the template.

Resolution

Rewrite the description to follow the repository template. Add these sections:

  1. Problem: State the problem in one or two sentences. Include the expected behavior and reproduction steps. For example: after clicking the timeline or dismissing a popup, no element has focus, and the user must use the mouse to return to the composer.
  2. Change: Explain how the Escape handler in useChatShortcuts now calls shouldEscapeFocusComposer. Explain that Escape focuses the composer only when thread find is inactive, shortcuts are enabled, and the event is unmodified, not composing, and has no non-collapsed selection.
  3. Scope and approval: Link the triaged issue or the discussion that gives maintainer approval for this behavior. If this change qualifies as a small, focused fix of an obvious bug, explain why no prior issue is needed. Note that a new keyboard shortcut behavior may need approval under CONTRIBUTING.md.
  4. Verification: List the browser tests that were run, such as the four Escape-key tests in useChatShortcuts.browser.test.tsx, and the observed results. Describe any manual checks. State anything that was not checked. Because this is a UI interaction change, add a short recording or before and after evidence if the change is visible.
  5. If an agent produced the change, name the model and harness at the end of the description.
Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…mselves

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…overs

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b3caf03. Configure here.

…pported

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis merged commit 6b860bf into main Oct 27, 2026
11 checks passed
@yordis
yordis deleted the yordis/feat-esc-focus-composer branch October 27, 2026 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant