fix(shortcuts): allow ESC to close dialogs when terminal is focused

[IShaalan]

Summary

• When a dialog (Help, Settings, New Task) is open and a terminal has focus, pressing ESC now correctly closes the dialog instead of being swallowed by xterm.js
• matchesGlobalShortcut now also passes dialogSafe shortcuts through xterm when a .dialog-overlay is present in the DOM
• ESC still reaches the terminal normally when no dialog is open (vim, readline, etc. unaffected)

Test plan

• Open keyboard shortcuts dialog (Cmd+/ or F1), click into a terminal, press ESC — dialog should close
• Open settings dialog, click into a terminal, press ESC — dialog should close
• With no dialog open, press ESC in terminal — should reach the terminal as before (e.g. exit vim insert mode)
• Cmd+/ and F1 still toggle the help dialog from terminal focus (unchanged, already had global: true)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Updates shortcut handling so dialogs can be closed via Escape even when an xterm.js terminal has focus (i.e., the terminal no longer swallows ESC when a dialog is open).

Changes:

• Extend matchesGlobalShortcut to treat dialogSafe shortcuts as “bypass terminal input” when a .dialog-overlay is present.
• Preserve existing behavior when no dialog is open (ESC continues to reach the terminal normally).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

matchesGlobalShortcut now performs a DOM query (document.querySelector('.dialog-overlay')) every time xterm dispatches a keydown, and the same selector check is duplicated again in initShortcuts(). Since this code runs on hot paths (terminal typing), consider extracting a shared isDialogOpen() helper and/or maintaining a cached dialog-open flag (updated when dialogs mount/unmount) to avoid repeated DOM queries per keystroke.

[IShaalan]

Addressed the DRY concern in d946ba6 by extracting an isDialogOpen() helper used by both call sites.

Intentionally skipping the cached-flag suggestion — the "hot path" framing overstates the cost:

• Keydown handlers fire at human typing rates (~5–15 Hz), not at hot-loop frequencies.
• querySelector('.dialog-overlay') is a class-indexed lookup in modern browsers — sub-microsecond, no layout/paint.
• Caching would require hooking every dialog's mount/unmount to keep the flag in sync with the DOM, introducing a new source of truth and stale-state risk for work that isn't actually expensive.

If dialog detection ever needs to become reactive (e.g. a SolidJS signal), the new isDialogOpen() helper is the single place to change it.

[johannesjo]

One observation not covered by the existing review:

Double-close on ESC is idempotent but worth noting

Dialog.tsx already registers its own document.addEventListener('keydown', handler) (lines 26-33) that calls props.onClose() on ESC. With this PR, when a dialog is open and a terminal has focus, xterm releases the ESC keydown (because matchesGlobalShortcut now returns true), and the event then propagates to two listeners:

1. Dialog.tsx's own handler → calls props.onClose()
2. initShortcuts's handler → fires the registered Escape shortcut → calls toggleXxxDialog(false)

Both paths ultimately call the same toggle setter with false, so closing an already-closed dialog is a no-op — no observable bug. But it's worth knowing both fire. If onClose ever had side effects beyond toggling state (e.g. focus restoration, animation triggers), this could matter.

This is pre-existing design tension between the Dialog's internal ESC handler and the app-level shortcut registration — not introduced by this PR. The fix itself is correct and the approach is sound.

Everything else looks good:

• .dialog-overlay is only ever applied in Dialog.tsx — no false-positive risk from other elements
• ESC correctly reaches the terminal (vim, readline) when no dialog is open — Escape shortcut has no global: true, so matchesGlobalShortcut returns false when no dialog is present
• SolidJS's <Show> removes .dialog-overlay synchronously on close, so no stale detection during a "closing animation" window
• TypeScript is clean, no any
• isDialogOpen() helper correctly centralizes the DOM query (addressing the DRY concern Copilot raised)

[johannesjo]

Thank you very much! <3