fix(desktop): keep Escape inside compose instead of exiting the stream #294

Merged
talksik merged 1 commits from claude/escape-key-compose-review-a496b2 into main 2026-06-21 15:32:59 +00:00
talksik commented 2026-06-21 01:29:52 +00:00 (Migrated from github.com)

While recording or reviewing a compose, Escape both cancelled the compose
and exited the stream. The stream's window-level navigation handler skips
keys when an input is focused (isTypingTarget), which is why the text path
was unaffected, but recording/reviewing have no focused input so Escape
leaked through to the exit handler.

Fix it at the compose layer rather than teaching the navigation pipe about
compose: the compose key handler now consumes (preventDefault +
stopPropagation) any key it handles and listens in the capture phase, so it
reliably wins over the stream's bubble-phase navigation/action handlers
regardless of listener registration order. Pure pause states (hold-space)
are untouched, so Escape still exits the stream there.

This leaves the typing/task/configuring paths equivalent (their onCancel is
the same cancel() the handler invokes) while ⌘+Enter / ⌘+M still fall
through to the text editor.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01DapjFX1MhYPZeJv4s56K5L

Summary by CodeRabbit

  • Bug Fixes
    • Fixed keyboard shortcut handling in the compose overlay to prevent compose-related key presses from inadvertently triggering other application actions.
While recording or reviewing a compose, Escape both cancelled the compose and exited the stream. The stream's window-level navigation handler skips keys when an input is focused (isTypingTarget), which is why the text path was unaffected, but recording/reviewing have no focused input so Escape leaked through to the exit handler. Fix it at the compose layer rather than teaching the navigation pipe about compose: the compose key handler now consumes (preventDefault + stopPropagation) any key it handles and listens in the capture phase, so it reliably wins over the stream's bubble-phase navigation/action handlers regardless of listener registration order. Pure pause states (hold-space) are untouched, so Escape still exits the stream there. This leaves the typing/task/configuring paths equivalent (their onCancel is the same cancel() the handler invokes) while ⌘+Enter / ⌘+M still fall through to the text editor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DapjFX1MhYPZeJv4s56K5L <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed keyboard shortcut handling in the compose overlay to prevent compose-related key presses from inadvertently triggering other application actions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
coderabbitai[bot] commented 2026-06-21 01:30:01 +00:00 (Migrated from github.com)

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: cb414d9c-f26b-4f7e-86ff-17aa8dcfbebd

📥 Commits

Reviewing files that changed from the base of the PR and between bfe53b46cf and 4f7fee538b.

📒 Files selected for processing (1)
  • js/desktop/src/features/compose/compose-overlay.tsx

📝 Walkthrough

Walkthrough

ComposeOverlay's keydown handler gains a local consume() helper (preventDefault + stopPropagation) applied to all compose-state shortcut paths. The listener is now registered in the capture phase so compose receives key events before any bubble-phase window handlers.

Changes

ComposeOverlay keyboard event propagation

Layer / File(s) Summary
consume() helper and per-state key handling
js/desktop/src/features/compose/compose-overlay.tsx
Adds a consume() function and applies it to Escape/cancel in early compose steps, idle shortcuts (`, S, T, D), and recording/reviewing controls (Q/Escape cancel, `/S stop, Enter send).
Capture-phase listener registration
js/desktop/src/features/compose/compose-overlay.tsx
Switches window.addEventListener('keydown', ..., true) and the matching removeEventListener to capture phase, ensuring compose's handler fires before bubble-phase handlers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A key pressed once, now heard by me first,
No bubble shall quench this compose-overlay's thirst.
consume() I call — default, propagation, gone!
Capture phase claimed, the shortcut lives on.
Hop hop, no leak, the keyboard is mine! 🎹


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

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

<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/flowy-live/llink/pull/294?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <details> <summary>ℹ️ Recent review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Free **Run ID**: `cb414d9c-f26b-4f7e-86ff-17aa8dcfbebd` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between bfe53b46cf5f113e232e71bfa92ec657632714ce and 4f7fee538b07e716ce99138a0ef226bb58a656b0. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `js/desktop/src/features/compose/compose-overlay.tsx` </details> </details> --- <!-- walkthrough_start --> <details> <summary>📝 Walkthrough</summary> ## Walkthrough `ComposeOverlay`'s keydown handler gains a local `consume()` helper (`preventDefault` + `stopPropagation`) applied to all compose-state shortcut paths. The listener is now registered in the capture phase so compose receives key events before any bubble-phase window handlers. ## Changes **ComposeOverlay keyboard event propagation** | Layer / File(s) | Summary | |---|---| | **`consume()` helper and per-state key handling** <br> `js/desktop/src/features/compose/compose-overlay.tsx` | Adds a `consume()` function and applies it to Escape/cancel in early compose steps, idle shortcuts (`` ` ``, `S`, `T`, `D`), and recording/reviewing controls (`Q`/Escape cancel, `` ` ``/`S` stop, `Enter` send). | | **Capture-phase listener registration** <br> `js/desktop/src/features/compose/compose-overlay.tsx` | Switches `window.addEventListener('keydown', ..., true)` and the matching `removeEventListener` to capture phase, ensuring compose's handler fires before bubble-phase handlers. | ## Estimated code review effort 🎯 2 (Simple) | ⏱️ ~10 minutes ## Poem > 🐇 A key pressed once, now heard by me first, > No bubble shall quench this compose-overlay's thirst. > `consume()` I call — default, propagation, gone! > Capture phase claimed, the shortcut lives on. > Hop hop, no leak, the keyboard is mine! 🎹 </details> <!-- walkthrough_end --> <!-- tips_start --> --- > [!NOTE] > <details> > <summary>🎁 Summarized by CodeRabbit Free</summary> > > Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting <https://app.coderabbit.ai/login>. > > </details> <sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub> <!-- tips_end -->
Sign in to join this conversation.