Skip to content

fix: quit only on q shortcut#505

Merged
benvinegar merged 2 commits into
mainfrom
fix/quit-only-q
Jul 3, 2026
Merged

fix: quit only on q shortcut#505
benvinegar merged 2 commits into
mainfrom
fix/quit-only-q

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • Stop treating Escape as a global quit shortcut in regular and pager modes
  • Keep q as the quit shortcut while preserving Escape for dialogs and focused controls
  • Add regression coverage for Escape not quitting

Tests

  • bun run typecheck
  • bun run lint
  • bun test src/ui/AppHost.interactions.test.tsx -t "q routes|Escape does not quit"

This PR description was generated by Pi using OpenAI GPT-5

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR narrows the quit shortcut in both regular and pager modes so that only q triggers requestQuit(), while Escape is now reserved exclusively for dialogs (agent skill, help, theme selector, menu) and focused inputs (filter, note). Regression tests are added to confirm Escape no longer quits in either mode.

  • useAppKeyboardShortcuts.ts: removes || isEscapeKey(key) from the regular-mode quit branch and removes the standalone isEscapeKey → requestQuit block from the pager-mode handler; Escape still propagates through handleDialogShortcut, handleThemeSelectorShortcut, handleMenuShortcut, and handleFocusedInputShortcut as before.
  • AppHost.interactions.test.tsx: renames the existing quit test and adds a new "Escape does not quit" test covering both regular and pager modes.

Confidence Score: 5/5

Safe to merge — the change is a targeted removal of two Escape-to-quit code paths with no unintended side effects on dialog or input Escape handling.

The keyboard event dispatch order is unchanged; handleDialogShortcut, handleThemeSelectorShortcut, handleMenuShortcut, and handleFocusedInputShortcut all continue to intercept Escape before it reaches the pager/app handlers, so their behavior is unaffected. The only change is that an unhandled Escape in regular or pager mode now silently passes through rather than triggering quit, which is the intended fix. New negative-case tests confirm the regression is covered, and the existing quit test was updated to reflect the narrowed scope.

No files require special attention.

Important Files Changed

Filename Overview
src/ui/hooks/useAppKeyboardShortcuts.ts Removes Escape as a global quit key in both regular and pager mode; isEscapeKey import remains valid as it is still used by dialog/menu/input handlers.
src/ui/AppHost.interactions.test.tsx Renames the existing quit test and adds explicit negative-case coverage for Escape in both regular and pager modes.
.changeset/quit-with-q-only.md Standard patch-level changeset entry accurately describing the behavioral change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    K([KeyEvent]) --> MT{handleMenuToggleShortcut\nf10?}
    MT -->|consumed| DONE([return])
    MT -->|pass| DS{handleDialogShortcut\nEscape?}
    DS -->|closeAgentSkill or closeHelp| DONE
    DS -->|pass| TS{handleThemeSelectorShortcut\nopen?}
    TS -->|Escape → closeThemeSelector\nor nav/select| DONE
    TS -->|pass| MS{handleMenuShortcut\nactive menu?}
    MS -->|Escape → closeMenu or nav| DONE
    MS -->|pass| FI{handleFocusedInputShortcut\nfilter or note focus?}
    FI -->|Escape → cancelDraftNote\nor text input| DONE
    FI -->|pass| PM{pagerMode?}
    PM -->|yes| HP[handlePagerShortcut\nq → quit / scroll / nav]
    PM -->|no| HA[handleAppShortcut\nq → quit / all other shortcuts]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    K([KeyEvent]) --> MT{handleMenuToggleShortcut\nf10?}
    MT -->|consumed| DONE([return])
    MT -->|pass| DS{handleDialogShortcut\nEscape?}
    DS -->|closeAgentSkill or closeHelp| DONE
    DS -->|pass| TS{handleThemeSelectorShortcut\nopen?}
    TS -->|Escape → closeThemeSelector\nor nav/select| DONE
    TS -->|pass| MS{handleMenuShortcut\nactive menu?}
    MS -->|Escape → closeMenu or nav| DONE
    MS -->|pass| FI{handleFocusedInputShortcut\nfilter or note focus?}
    FI -->|Escape → cancelDraftNote\nor text input| DONE
    FI -->|pass| PM{pagerMode?}
    PM -->|yes| HP[handlePagerShortcut\nq → quit / scroll / nav]
    PM -->|no| HA[handleAppShortcut\nq → quit / all other shortcuts]
Loading

Reviews (1): Last reviewed commit: "fix: quit only on q shortcut" | Re-trigger Greptile

@benvinegar
benvinegar merged commit ed8268a into main Jul 3, 2026
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant