Fix Pane navigation shortcuts in active TUIs - #367
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a92ee6795a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (state.isTuiActive && isPaneNavigationShortcut(event, state)) { | ||
| return { action: 'release-to-app' }; |
There was a problem hiding this comment.
When a TUI is active, every allowlisted chord is removed from xterm before the hotkey store checks its enabled predicate. For example, with only one session, mod+Tab is released here, but cycle-session-next-0 is disabled and the event therefore neither performs a Pane action nor reaches the PTY; the same loss occurs for group navigation without multiple groups, zoom without a split, unavailable tab numbers, and splitting a group with fewer than two tabs. Consult the matching hotkey's enabled state before returning release-to-app, otherwise terminal-native chords are silently swallowed whenever the Pane command cannot run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dc5c56c. The terminal now consults the live hotkey registry and its enabled predicate before releasing a candidate chord; disabled actions return directly to TUI passthrough instead of falling through to the old unconditional checks.
| const ctrlOrMeta = event.ctrlKey || event.metaKey; | ||
| if (!ctrlOrMeta) return false; |
There was a problem hiding this comment.
On macOS this treats Ctrl as Pane's mod for every new bypass except backslash. Consequently, active TUIs lose terminal-native chords such as Ctrl+Shift+Z or Ctrl+Alt+Arrow, and the global normalizer may execute the corresponding Pane zoom/navigation action even though the platform shortcut is presented as Cmd+Shift+Z or Cmd+Alt+Arrow. Derive the primary modifier from state.isMac for the whole allowlist, as the backslash branch already does, so macOS Ctrl chords continue reaching the PTY.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dc5c56c. The allowlist now uses Cmd as the primary modifier on macOS and Ctrl elsewhere, with regression coverage for macOS Ctrl+Alt+Arrow passthrough and Cmd+Alt+Arrow release.
| && !isAltGr | ||
| && !event.shiftKey | ||
| && ( | ||
| ['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(event.key) | ||
| || /^Digit[1-9]$/.test(event.code) |
There was a problem hiding this comment.
On Windows/Linux layouts where AltGr is reported as ctrlKey && altKey but getModifierState('AltGraph') is false, AltGr plus a physical digit now matches this branch, is withheld from the TUI, and may trigger an add-tool shortcut instead of entering the layout character. TerminalPanel already accounts for this browser behavior with a Ctrl+Alt fallback for AltGr-produced @; the bypass classifier needs an equivalent fallback based on the produced character or right-Alt state rather than relying exclusively on AltGraph.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dc5c56c. On Windows/Linux, Ctrl+Alt physical-digit events are now rejected from the Pane allowlist when the produced key differs from that digit, covering AltGr character output even when AltGraph is false. The existing explicit AltGraph guard remains, with tests for both paths.
1 participant
Summary
Testing
pnpm --filter frontend exec vitest run src/utils/terminalKeyHandling.test.tspnpm --filter frontend typecheckpnpm --filter frontend lint(passes with existing warnings)pnpm --filter frontend build