diff --git a/package-lock.json b/package-lock.json index 0b15b98..09a95a3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "agentrium", - "version": "1.32.2", + "version": "1.33.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "agentrium", - "version": "1.32.2", + "version": "1.33.3", "dependencies": { "@monaco-editor/react": "^4.6.0", "@tauri-apps/api": "^2.0.0", diff --git a/src/App.tsx b/src/App.tsx index 2ca310c..b502a25 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -793,8 +793,16 @@ function App() { return (
{/* Launch splash - logo reveal video + loading bar, main window only. */} diff --git a/src/components/NewTerminalModal.tsx b/src/components/NewTerminalModal.tsx index 8149115..3068b5c 100644 --- a/src/components/NewTerminalModal.tsx +++ b/src/components/NewTerminalModal.tsx @@ -434,6 +434,20 @@ export function NewTerminalModal() { } closeNewTerminalModal(); + + // Move focus into the newly-created terminal's xterm. Without this the + // browser returns focus to the sidebar's New Session button (the trigger + // that opened this modal), and a subsequent Enter press re-fires that + // button as a synthetic click, re-opening the modal instead of reaching + // the terminal. See #58. TerminalTabs also has a rAF focus effect keyed + // on activeTerminalId, but the effect runs BEFORE the terminal's canvas + // is fully mounted on first render - so mirror the focus here after two + // rAFs to make it stick. + requestAnimationFrame(() => { + requestAnimationFrame(() => { + useTerminalStore.getState().terminals.get(newTerminalId)?.xterm?.focus(); + }); + }); } catch (err) { setError(String(err)); reportInvokeFailure('create_terminal', err); diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index b50dd66..461742e 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -140,7 +140,17 @@ export function Sidebar() { {/* Prominent primary action + settings */}
+ {/* Close active session (#59). Only visible while a real terminal + is focused - hidden while browsing a file, and hidden entirely + when there is no active session. Mirrors the sidebar card X, + which also closes without a confirm. */} + {activeConfig && !activeFilePath && ( + + + + )}
)} diff --git a/src/components/TerminalView.tsx b/src/components/TerminalView.tsx index 30b02e1..9b55921 100644 --- a/src/components/TerminalView.tsx +++ b/src/components/TerminalView.tsx @@ -18,6 +18,7 @@ import { reportInvokeFailure } from '../lib/errorReporter'; import { classifyPasteInput } from '../lib/pasteWarning'; import { toBracketedPaste } from '../lib/bracketedPaste'; import { decideCtrlC } from '../lib/ctrlCAction'; +import { matchesKeyCode } from '../lib/keymap'; import { resolveAppTheme, prefersLightScheme } from '../lib/appTheme'; import { isVisibilityHidden } from '../utils/dragDrop'; import { TerminalSearch } from './TerminalSearch'; @@ -298,25 +299,29 @@ export function TerminalView({ terminalId }: TerminalViewProps) { container.addEventListener('mousedown', swallowRightButton, true); container.addEventListener('mouseup', swallowRightButton, true); - // Handle Ctrl+C (copy) and Ctrl+V (paste) keyboard shortcuts + // Handle Ctrl+C (copy) and Ctrl+V (paste) keyboard shortcuts. + // + // Letter accelerators use `matchesKeyCode` (from lib/keymap.ts) so they + // still fire under non-Latin keyboard layouts (Hebrew/Cyrillic/Arabic/...). + // `KeyboardEvent.key` returns the LOCALIZED character produced by the + // physical key - on Hebrew, physical V yields `e.key === 'ה'`, so a + // `key === 'v'` comparison never matched, quietly breaking every Ctrl+letter + // shortcut for those users (see #57). `KeyboardEvent.code` reports the + // physical key position (`'KeyV'`) regardless of layout, which is what we + // want. CapsLock no longer needs a `.toLowerCase()` guard for the same + // reason: `e.code` is case-invariant. terminal.attachCustomKeyEventHandler((e: KeyboardEvent) => { const isCtrl = e.ctrlKey || e.metaKey; - // Normalize the key so CapsLock doesn't break these shortcuts: with - // CapsLock on, an unshifted letter arrives as uppercase (e.g. 'V'), which - // would otherwise miss the lowercase comparisons below and fall through to - // xterm's raw control byte (no paste/copy). Shift is still distinguished - // via e.shiftKey, so Ctrl+Shift+V stays handled by the global handler. - const key = e.key.toLowerCase(); // Ctrl+F: Toggle in-terminal search (Ctrl+Shift+F is reserved for the // global file/content search - see useKeyboardShortcuts). - if (isCtrl && !e.shiftKey && key === 'f' && e.type === 'keydown') { + if (isCtrl && !e.shiftKey && matchesKeyCode(e, 'F') && e.type === 'keydown') { e.preventDefault(); toggleSearch(); return false; } - if (isCtrl && !e.shiftKey && key === 'c' && e.type === 'keydown') { + if (isCtrl && !e.shiftKey && matchesKeyCode(e, 'C') && e.type === 'keydown') { const action = decideCtrlC({ hasSelection: terminal.hasSelection(), copyOnSelect: useAppStore.getState().terminalCopyOnSelect, @@ -343,9 +348,9 @@ export function TerminalView({ terminalId }: TerminalViewProps) { // Ctrl+V control byte (0x16) to the program. Pasting // is then done via right-click or Ctrl+Shift+V // ("Paste as file"), which is unaffected here. - // Ctrl+Shift+V (e.key === 'V') is intentionally not matched - it's - // handled by the global shortcut handler. - if (isCtrl && !e.shiftKey && key === 'v') { + // Ctrl+Shift+V is intentionally not matched here - it's handled by the + // global shortcut handler. + if (isCtrl && !e.shiftKey && matchesKeyCode(e, 'V')) { if (useAppStore.getState().terminalPasteShortcut === 'ctrl+v') { return false; } @@ -362,7 +367,7 @@ export function TerminalView({ terminalId }: TerminalViewProps) { // actual undo binding. Claude Code binds undo to Ctrl+_ (byte 0x1f), NOT // Ctrl+Z - a raw 0x1a is SIGTSTP/suspend and does nothing useful in the // prompt. So we send 0x1f. (Also prevents any browser-level undo.) - if (isCtrl && !e.shiftKey && key === 'z') { + if (isCtrl && !e.shiftKey && matchesKeyCode(e, 'Z')) { if (e.type === 'keydown') { e.preventDefault(); writeToTerminal(terminalId, '\x1f').catch((err) => { @@ -383,8 +388,9 @@ export function TerminalView({ terminalId }: TerminalViewProps) { // not cancel) from leaking a stray \x1b[Z into the PTY. // // Plain Tab is deliberately untouched - Claude Code needs it for - // autocomplete and mode switching. - if (isCtrl && key === 'tab') { + // autocomplete and mode switching. `e.key === 'Tab'` is safe across + // layouts (Tab isn't remapped by non-Latin layouts). + if (isCtrl && e.key === 'Tab') { return false; } diff --git a/src/components/TitleBar.tsx b/src/components/TitleBar.tsx index 909b8b2..cf78cc8 100644 --- a/src/components/TitleBar.tsx +++ b/src/components/TitleBar.tsx @@ -215,7 +215,12 @@ export function TitleBar() { {branchMenuOpen && (
diff --git a/src/hooks/useKeyboardShortcuts.ts b/src/hooks/useKeyboardShortcuts.ts index 219d5eb..c666a43 100644 --- a/src/hooks/useKeyboardShortcuts.ts +++ b/src/hooks/useKeyboardShortcuts.ts @@ -7,6 +7,7 @@ import { captureClaudeInput } from '../lib/terminalInput'; import { reportInvokeFailure } from '../lib/errorReporter'; import { readClipboardText } from '../lib/clipboard'; import { cyclableTabIds, nextTabId } from '../lib/tabCycle'; +import { matchesKeyCode } from '../lib/keymap'; /** * Return true when the focused element is an editable surface that is NOT @@ -63,7 +64,11 @@ export function useKeyboardShortcuts() { // top-level document, which throws away all open terminals (they // aren't persisted). If the preview panel is open, route to the // preview reload instead. - if (e.key === 'F5' || (ctrl && !shift && (e.key === 'r' || e.key === 'R'))) { + // + // Letter accelerators use `matchesKeyCode` so they still fire on + // non-Latin layouts (Hebrew/Russian/Arabic/...), where `e.key` would + // hold the localized character instead of 'r'. See lib/keymap.ts. + if (e.key === 'F5' || (ctrl && !shift && matchesKeyCode(e, 'R'))) { e.preventDefault(); const previewOpen = usePreviewStore.getState().globalOpen; const activeId = activeIdRef.current; @@ -73,19 +78,19 @@ export function useKeyboardShortcuts() { return; } - if (ctrl && shift && e.key === 'N') { + if (ctrl && shift && matchesKeyCode(e, 'N')) { e.preventDefault(); useAppStore.getState().openNewTerminalModal(); } // Command Palette: Ctrl+P - if (ctrl && e.key === 'p') { + if (ctrl && !shift && matchesKeyCode(e, 'P')) { e.preventDefault(); useAppStore.getState().toggleCommandPalette(); } // Snippets: Ctrl+Shift+S - if (ctrl && shift && e.key === 'S') { + if (ctrl && shift && matchesKeyCode(e, 'S')) { e.preventDefault(); useAppStore.getState().openSnippetsModal(); } @@ -93,7 +98,7 @@ export function useKeyboardShortcuts() { // Prompt Editor: Ctrl+Shift+E - compose a prompt for the active terminal, // seeded with whatever is already typed in its input line. Can be turned // off in Settings (the status-bar pencil still works). - if (ctrl && shift && e.key === 'E' && useAppStore.getState().promptEditorShortcutEnabled) { + if (ctrl && shift && matchesKeyCode(e, 'E') && useAppStore.getState().promptEditorShortcutEnabled) { e.preventDefault(); const activeId = activeIdRef.current; const term = activeId ? terminalsRef.current.get(activeId)?.xterm : undefined; @@ -102,7 +107,7 @@ export function useKeyboardShortcuts() { } // Paste as file: Ctrl+Shift+V - if (ctrl && shift && e.key === 'V') { + if (ctrl && shift && matchesKeyCode(e, 'V')) { e.preventDefault(); const activeId = activeIdRef.current; (async () => { @@ -116,7 +121,7 @@ export function useKeyboardShortcuts() { } // Preview panel toggle: Ctrl+Alt+P (Ctrl+Shift+V and Ctrl+Shift+G are taken). - if (ctrl && e.altKey && !shift && (e.key === 'p' || e.key === 'P')) { + if (ctrl && e.altKey && !shift && matchesKeyCode(e, 'P')) { e.preventDefault(); usePreviewStore.getState().toggleGlobal(); } @@ -142,17 +147,17 @@ export function useKeyboardShortcuts() { } // Global file/content search (VS Code style): Ctrl+Shift+F - if (ctrl && shift && e.key === 'F') { + if (ctrl && shift && matchesKeyCode(e, 'F')) { e.preventDefault(); useAppStore.getState().toggleGlobalSearch(); } - if (ctrl && e.key === 'b') { + if (ctrl && !shift && matchesKeyCode(e, 'B')) { e.preventDefault(); useAppStore.getState().toggleSidebar(); } - if (ctrl && e.key === 'w') { + if (ctrl && !shift && matchesKeyCode(e, 'W')) { e.preventDefault(); const activeId = activeIdRef.current; if (activeId) { @@ -164,7 +169,7 @@ export function useKeyboardShortcuts() { } // Duplicate active terminal: Ctrl+Shift+D - if (ctrl && shift && e.key === 'D') { + if (ctrl && shift && matchesKeyCode(e, 'D')) { e.preventDefault(); const activeId = activeIdRef.current; if (activeId) { @@ -242,13 +247,13 @@ export function useKeyboardShortcuts() { } // Toggle Grid Mode: Ctrl+G - if (ctrl && e.key === 'g') { + if (ctrl && !shift && matchesKeyCode(e, 'G')) { e.preventDefault(); useAppStore.getState().toggleGridMode(); } // Push modal: Ctrl+Shift+K (IntelliJ parity) - if (ctrl && shift && e.key === 'K') { + if (ctrl && shift && matchesKeyCode(e, 'K')) { e.preventDefault(); const activeId = activeIdRef.current; if (!activeId) { @@ -270,7 +275,7 @@ export function useKeyboardShortcuts() { } // Worktree Modal: Ctrl+Shift+W - if (ctrl && shift && e.key === 'W') { + if (ctrl && shift && matchesKeyCode(e, 'W')) { e.preventDefault(); const activeId = activeIdRef.current; if (activeId) { @@ -286,7 +291,7 @@ export function useKeyboardShortcuts() { } // Add current terminal to grid: Ctrl+Shift+G - if (ctrl && shift && e.key === 'G') { + if (ctrl && shift && matchesKeyCode(e, 'G')) { e.preventDefault(); const activeId = activeIdRef.current; if (activeId) { @@ -319,7 +324,12 @@ export function useKeyboardShortcuts() { // Terminal font zoom. Ctrl+= / Ctrl++ / Ctrl+- / Ctrl+0. // Skip when the user is in a non-terminal editable surface so that // e.g. Ctrl+- in a Settings input still selects characters natively. - if (ctrl && !shift && (e.key === '=' || e.key === '-' || e.key === '0')) { + // + // Digit 0 uses `matchesKeyCode` so Ctrl+0 zooms-reset even under a layout + // that remaps the digit row. `=` and `-` stay on `e.key` because their + // physical location varies across layouts (US Equal vs. AZERTY), but + // `e.key` reports the intended character reliably. + if (ctrl && !shift && (e.key === '=' || e.key === '-' || matchesKeyCode(e, '0'))) { if (isFocusInNonTerminalEditable()) return; e.preventDefault(); const { terminalFontSize, setTerminalFontSize } = useAppStore.getState(); diff --git a/src/lib/keymap.test.ts b/src/lib/keymap.test.ts index 5cbe9f2..7ded92e 100644 --- a/src/lib/keymap.test.ts +++ b/src/lib/keymap.test.ts @@ -1,5 +1,14 @@ import { describe, expect, it } from 'vitest'; -import { KEYMAP, keymapByGroup } from './keymap'; +import { KEYMAP, keymapByGroup, matchesKeyCode } from './keymap'; + +// Build a synthetic KeyboardEvent-like object with just the fields +// `matchesKeyCode` reads. Using a plain object keeps the tests running under +// jsdom (real KeyboardEvent needs a Window) and mirrors the shape of what the +// browser dispatches. Adding `type: 'keydown'` isn't required by the helper +// but documents intent. +function evt(code: string, key = ''): KeyboardEvent { + return { code, key, type: 'keydown' } as unknown as KeyboardEvent; +} describe('keymap', () => { it('every entry has the required fields', () => { @@ -40,3 +49,64 @@ describe('keymap', () => { expect(zoomIn?.shortcut).toMatch(/^(Ctrl|Cmd)\+=$/); }); }); + +// #57 - Ctrl+letter shortcuts silently broke under non-Latin layouts because +// the handlers compared `e.key` (the LOCALIZED character) against a Latin +// letter. `matchesKeyCode` compares `e.code` (the PHYSICAL key) instead, so +// Ctrl+V works whether the user is on English, Hebrew, Cyrillic, Arabic, etc. +describe('matchesKeyCode', () => { + it('matches the physical letter key regardless of what character the layout produces', () => { + // The physical V key. `e.key` differs per layout: + // English → 'v' + // Hebrew → 'ה' (Heh) + // Cyrillic → 'м' (Cyrillic em) + // Arabic → 'ر' (Reh) + // Greek → 'ω' (Omega) + // Only the physical position ('KeyV') is layout-invariant. + expect(matchesKeyCode(evt('KeyV', 'v'), 'V')).toBe(true); + expect(matchesKeyCode(evt('KeyV', 'ה'), 'V')).toBe(true); + expect(matchesKeyCode(evt('KeyV', 'м'), 'V')).toBe(true); + expect(matchesKeyCode(evt('KeyV', 'ر'), 'V')).toBe(true); + expect(matchesKeyCode(evt('KeyV', 'ω'), 'V')).toBe(true); + }); + + it('accepts both lowercase and uppercase letter arguments', () => { + // Ergonomic - callers can write matchesKeyCode(e, 'v') or 'V' equivalently. + expect(matchesKeyCode(evt('KeyC'), 'c')).toBe(true); + expect(matchesKeyCode(evt('KeyC'), 'C')).toBe(true); + }); + + it('is case-invariant on the code side - CapsLock does not break it', () => { + // Under a real CapsLock the browser still emits code 'KeyV'; the change + // is in `e.key` only, which the helper does not read. This is why the old + // `key.toLowerCase()` hack in TerminalView is no longer necessary. + expect(matchesKeyCode(evt('KeyV', 'V'), 'V')).toBe(true); + expect(matchesKeyCode(evt('KeyV', 'v'), 'V')).toBe(true); + }); + + it('matches digits via DigitN codes', () => { + // Ctrl+0 zoom-reset. Physical digit row still reports DigitN codes even + // when a layout remaps the shifted characters. + expect(matchesKeyCode(evt('Digit0'), '0')).toBe(true); + expect(matchesKeyCode(evt('Digit9'), '9')).toBe(true); + }); + + it('does not match other physical keys with the same character', () => { + // If a layout happens to place 'V' on a different physical key (or the + // user hits a different key producing 'v'), the helper must NOT match - + // that would defeat the whole point of using `code`. + expect(matchesKeyCode(evt('KeyB', 'v'), 'V')).toBe(false); + expect(matchesKeyCode(evt('KeyC', 'v'), 'V')).toBe(false); + expect(matchesKeyCode(evt('Numpad0'), '0')).toBe(false); + }); + + it('rejects non-letter/non-digit arguments defensively', () => { + // Callers should use e.key for symbols/function keys; the helper only + // handles letters + digits. Returning false (not throwing) keeps the + // dispatcher safe if someone passes an unexpected string. + expect(matchesKeyCode(evt('KeyV'), 'VV')).toBe(false); + expect(matchesKeyCode(evt('KeyV'), '')).toBe(false); + expect(matchesKeyCode(evt('Comma', ','), ',')).toBe(false); + expect(matchesKeyCode(evt('F1'), 'F1')).toBe(false); + }); +}); diff --git a/src/lib/keymap.ts b/src/lib/keymap.ts index 28008ac..9c76965 100644 --- a/src/lib/keymap.ts +++ b/src/lib/keymap.ts @@ -1,6 +1,26 @@ // Shared keymap definitions. Hooks/useKeyboardShortcuts.ts is the actual handler; // this file is the single source of truth for displayed labels and groups. +// Layout-independent Ctrl+letter matcher. `KeyboardEvent.key` returns the +// character produced by the CURRENT keyboard layout - on a Hebrew (or Cyrillic, +// Arabic, Greek, ...) layout the physical V key produces `e.key === 'ה'`, so +// `key === 'v'` never matches and every Ctrl+letter accelerator (paste, copy, +// close, sidebar, ...) silently breaks. `KeyboardEvent.code` reports the +// physical key's position on a US-QWERTY layout (`'KeyV'`, `'Digit0'`), which +// is stable across layouts and matches how users think about shortcuts (they +// find them by physical position, same as VS Code / browsers). +// +// Use for letters and digits only - non-letter accelerators (Tab, F1..F8, ,, +// =, -, \, +) should keep reading `e.key`, whose semantic identity is layout- +// independent for those keys. +export function matchesKeyCode(e: KeyboardEvent, letterOrDigit: string): boolean { + const s = letterOrDigit.toUpperCase(); + if (s.length !== 1) return false; + if (s >= 'A' && s <= 'Z') return e.code === `Key${s}`; + if (s >= '0' && s <= '9') return e.code === `Digit${s}`; + return false; +} + export interface KeymapEntry { id: string; label: string;