Skip to content

fix(ui): restore live escape handling in PTY flows#173

Merged
benvinegar merged 11 commits into
mainfrom
fix/live-ui-keyboard-interactions
Apr 7, 2026
Merged

fix(ui): restore live escape handling in PTY flows#173
benvinegar merged 11 commits into
mainfrom
fix/live-ui-keyboard-interactions

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • add live PTY coverage for missing keyboard interactions, including filter focus and escape behavior, help, menu navigation, layout hotkeys, and pager navigation
  • restore live escape handling by accepting both escape and esc in the app shortcut layer and by letting the focused filter input clear or exit directly
  • verify the change with bun test src/ui/AppHost.interactions.test.tsx and bun run test:integration

@greptile-apps

greptile-apps Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restores live escape key handling in PTY flows by: (1) adding an isEscapeKey helper that accepts both "escape" and "esc" key names (fixing the root cause of the regression), (2) wiring escape handling into handleFilterShortcut (clear filter, then exit filter mode) and handleAppShortcut (quit), and (3) adding an onKeyDown intercept on the filter <input> in StatusBar so the widget itself can handle escape before the event propagates. The PR also adds a broad suite of PTY integration tests covering filter, help, menu, layout, and pager keyboard interactions.

  • isEscapeKey helper added in useAppKeyboardShortcuts.ts normalises the escape/esc duality in one place — but the same check is duplicated inline in StatusBar.tsx (key.name !== "escape" && key.name !== "esc"), while keyboard.ts already exports the canonical pattern for this kind of predicate.
  • Escape handling when the filter input is focused exists in two places: the input's onKeyDown in StatusBar.tsx (calling key.stopPropagation()) and handleFilterShortcut in the keyboard hook. Both paths call idempotent actions (clearFilter / focusFiles), so there's no visible bug, but the dual path sits in tension with the CLAUDE.md guideline to prefer one implementation path per feature.
  • The new PTY integration tests (ui-integration.test.ts) are well-structured, cover all the newly added interactions, and match the project's integration testing conventions.

Confidence Score: 5/5

Safe to merge — all findings are non-blocking style suggestions with no correctness or reliability impact.

Both open comments are P2 (style/consistency) and do not affect runtime behaviour. The escape actions are idempotent so the dual-path handling causes no visible bugs. The PR is well-tested with new PTY integration coverage.

No files require special attention; the style notes in StatusBar.tsx and useAppKeyboardShortcuts.ts are minor clean-up opportunities.

Important Files Changed

Filename Overview
src/ui/hooks/useAppKeyboardShortcuts.ts Adds isEscapeKey helper, wires escape into handleFilterShortcut (clear then exit) and handleAppShortcut (quit); logic is correct but isEscapeKey could live in keyboard.ts for consistency
src/ui/components/chrome/StatusBar.tsx Adds onKeyDown intercept on the filter input to handle escape; duplicates the escape key name check that isEscapeKey already encapsulates
test/pty/ui-integration.test.ts Broad new PTY test coverage for filter/escape, help, menu navigation, layout hotkeys, and pager navigation; well-structured and follows existing harness conventions

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    K[Keyboard event] --> MT{F10 pressed?}
    MT -- yes --> MH[handleMenuToggleShortcut
open/close file menu]
    MT -- no --> PM{pagerMode?}
    PM -- yes --> PS[handlePagerShortcut
q / Esc → quit
space/b/d/u/arrows → scroll]
    PM -- no --> HS{showHelp?}
    HS -- yes + Esc --> CH[closeHelp]
    HS -- no / other key --> MS{activeMenuId?}
    MS -- yes --> MN[handleMenuShortcut
Esc → closeMenu
arrows → navigate
enter → activate]
    MS -- no --> FS{focusArea = filter?}
    FS -- yes + Esc with content --> CF[clearFilter]
    FS -- yes + Esc empty --> FF[focusFiles]
    FS -- yes + other key --> CI[consume — let input widget type]
    FS -- no --> AS[handleAppShortcut
Esc → requestQuit
/ → focusFilter
q → quit
0/1/2 → layout
 etc.]
    K2[Keyboard event to focused input] --> ID{StatusBar input focused?}
    ID -- yes + Esc with content --> OFI[onFilterInput '' → clearFilter]
    ID -- yes + Esc empty --> OFS[onFilterSubmit → focusFiles]
    ID -- no --> PASS[event not intercepted]
Loading

Reviews (1): Last reviewed commit: "fix(ui): restore live escape handling in..." | Re-trigger Greptile

Comment thread src/ui/components/chrome/StatusBar.tsx Outdated
Comment thread src/ui/hooks/useAppKeyboardShortcuts.ts Outdated
Comment thread src/ui/components/chrome/StatusBar.tsx
@benvinegar benvinegar force-pushed the fix/live-ui-keyboard-interactions branch from b0d89d9 to 5bc9afa Compare April 7, 2026 01:11
@benvinegar benvinegar merged commit b37663f into main Apr 7, 2026
3 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