feat: add terminal font size zoom with Cmd+/- shortcuts#5
Open
stromseth wants to merge 1 commit into
Open
Conversation
Adds keyboard shortcuts to increase/decrease terminal font size: - Cmd+= or Cmd++ to increase font size - Cmd+- to decrease font size - Cmd+0 to reset to default (13px) Both = and + keys are registered for zoom-in to support Norwegian and other keyboard layouts where + is a standalone key. Font size is persisted via localStorage (range: 8-32px) and applied to all active terminal instances including the code review terminal. The PTY is resized after each font change to keep cols/rows in sync. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds global terminal font-size zoom support (Cmd/Ctrl =, +, -, 0) with persistence, and wires it into terminal creation so new terminals start at the configured size.
Changes:
- Introduces
TerminalFontSizeservice to persist/clamp font size and register global zoom shortcuts. - Propagates font size into terminal creation (
TerminalPool, code review terminal) and allows zoom keys to bubble past xterm key handling. - Updates ignore rules to exclude
.worktrees.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/terminal/terminal-utils.ts | Adds fontSize parameter to createTerminal() and lets zoom key combos pass through xterm’s custom key handler. |
| frontend/src/services/TerminalPool.ts | Uses the current global terminal font size when creating pooled terminals. |
| frontend/src/services/TerminalFontSize.ts | New global font size state + localStorage persistence + shortcut registration and fan-out to active terminals. |
| frontend/src/hooks/useCodeReviewTerminal.ts | Initializes code-review terminal with global font size and reacts to font-size changes. |
| frontend/src/App.tsx | Side-effect import to register terminal zoom shortcuts at startup. |
| .gitignore | Ignores .worktrees. |
Comments suppressed due to low confidence (1)
frontend/src/hooks/useCodeReviewTerminal.ts:69
- The terminal created here doesn’t attach the shared
attachKeyHandlers/attachCustomKeyEventHandlerlogic that preventsCmd/Ctrl+=/-/0from being processed by xterm. With the new global zoom shortcuts, keydown events will reach xterm before the window-levelKeyboardShortcutManagerhandler, so the zoom keystrokes may still be sent to the PTY while also changing font size. Consider reusingattachKeyHandlers(or at least adding aattachCustomKeyEventHandlerguard for these combos) for this terminal too.
terminal = new Terminal({
theme: {
background: '#111111',
foreground: '#e6e6e6',
},
fontFamily: 'Menlo, Monaco, "Courier New", monospace',
fontSize: getTerminalFontSize(),
cursorBlink: true,
allowProposedApi: true, // Required for Unicode11 addon
});
fitAddon = new FitAddon();
terminal.loadAddon(fitAddon);
terminal.open(container);
loadAddons(terminal);
fitAddon.fit();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+9
| import { createSignal } from 'solid-js'; | ||
| import { terminalPool } from './TerminalPool'; | ||
| import { keyboardShortcutManager } from './KeyboardShortcutManager'; |
There was a problem hiding this comment.
TerminalFontSize imports terminalPool, while TerminalPool imports getTerminalFontSize from TerminalFontSize, creating a circular module dependency. This is fragile (can yield partially-initialized exports under different import orders/HMR/tests) and also couples the font-size service to the pool implementation. Consider breaking the cycle by having TerminalPool observe terminalFontSize() (e.g., a Solid effect in the pool module) and apply updates to its handles, or by introducing a lightweight callback/subscription interface so TerminalFontSize doesn’t import TerminalPool directly.
Comment on lines
+11
to
+116
| const STORAGE_KEY = 'codelane-terminal-font-size'; | ||
| const DEFAULT_FONT_SIZE = 13; | ||
| const MIN_FONT_SIZE = 8; | ||
| const MAX_FONT_SIZE = 32; | ||
| const STEP = 1; | ||
|
|
||
| function loadFontSize(): number { | ||
| try { | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| if (stored) { | ||
| const parsed = parseInt(stored, 10); | ||
| if (!isNaN(parsed) && parsed >= MIN_FONT_SIZE && parsed <= MAX_FONT_SIZE) { | ||
| return parsed; | ||
| } | ||
| } | ||
| } catch { | ||
| // localStorage unavailable | ||
| } | ||
| return DEFAULT_FONT_SIZE; | ||
| } | ||
|
|
||
| function saveFontSize(size: number): void { | ||
| try { | ||
| localStorage.setItem(STORAGE_KEY, String(size)); | ||
| } catch { | ||
| // localStorage unavailable | ||
| } | ||
| } | ||
|
|
||
| const [fontSize, setFontSizeSignal] = createSignal(loadFontSize()); | ||
|
|
||
| function applyToAllTerminals(size: number): void { | ||
| for (const handle of terminalPool.getAllHandles()) { | ||
| handle.terminal.options.fontSize = size; | ||
| try { | ||
| handle.fitAddon.fit(); | ||
| // Resize PTY to match new cols/rows after font size change | ||
| terminalPool.resize(handle.id, handle.terminal.cols, handle.terminal.rows).catch(() => {}); | ||
| } catch { | ||
| // terminal may not be attached yet | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function getTerminalFontSize(): number { | ||
| return fontSize(); | ||
| } | ||
|
|
||
| /** Reactive signal accessor for use in SolidJS effects */ | ||
| export const terminalFontSize = fontSize; | ||
|
|
||
| export function setTerminalFontSize(size: number): void { | ||
| const clamped = Math.max(MIN_FONT_SIZE, Math.min(MAX_FONT_SIZE, size)); | ||
| setFontSizeSignal(clamped); | ||
| saveFontSize(clamped); | ||
| applyToAllTerminals(clamped); | ||
| } | ||
|
|
||
| export function increaseTerminalFontSize(): void { | ||
| setTerminalFontSize(fontSize() + STEP); | ||
| } | ||
|
|
||
| export function decreaseTerminalFontSize(): void { | ||
| setTerminalFontSize(fontSize() - STEP); | ||
| } | ||
|
|
||
| export function resetTerminalFontSize(): void { | ||
| setTerminalFontSize(DEFAULT_FONT_SIZE); | ||
| } | ||
|
|
||
| // Register global keyboard shortcuts for terminal zoom | ||
| keyboardShortcutManager.register({ | ||
| id: 'terminalZoomIn', | ||
| description: 'Increase terminal font size', | ||
| key: '=', | ||
| modifiers: { cmdOrCtrl: true }, | ||
| handler: () => increaseTerminalFontSize(), | ||
| scope: 'global', | ||
| }); | ||
|
|
||
| keyboardShortcutManager.register({ | ||
| id: 'terminalZoomInPlus', | ||
| description: 'Increase terminal font size (+)', | ||
| key: '+', | ||
| modifiers: { cmdOrCtrl: true }, | ||
| handler: () => increaseTerminalFontSize(), | ||
| scope: 'global', | ||
| }); | ||
|
|
||
| keyboardShortcutManager.register({ | ||
| id: 'terminalZoomOut', | ||
| description: 'Decrease terminal font size', | ||
| key: '-', | ||
| modifiers: { cmdOrCtrl: true }, | ||
| handler: () => decreaseTerminalFontSize(), | ||
| scope: 'global', | ||
| }); | ||
|
|
||
| keyboardShortcutManager.register({ | ||
| id: 'terminalZoomReset', | ||
| description: 'Reset terminal font size', | ||
| key: '0', | ||
| modifiers: { cmdOrCtrl: true }, | ||
| handler: () => resetTerminalFontSize(), | ||
| scope: 'global', | ||
| }); |
There was a problem hiding this comment.
This file registers global zoom shortcuts, but there are no unit tests covering the persistence/clamping behavior (8–32px) or that the Cmd/Ctrl shortcuts actually update state. Since the repo already has service-level tests for KeyboardShortcutManager, consider adding tests for TerminalFontSize (e.g., mocking localStorage and verifying increase/decrease/reset + shortcut dispatch update the stored value).
Owner
|
Hello @stromseth thanks for your contribution, could you add some unit tests and resolve the comments above? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
=and+keys registered for zoom-in to support Norwegian and other keyboard layouts where+is a standalone key (not Shift+=)Implementation
New file:
frontend/src/services/TerminalFontSize.tsTerminalPool.getAllHandles()fitAddon.fit()to keep cols/rows in syncKeyboardShortcutManagerModified files:
packages/shared/src/terminal/terminal-utils.ts—createTerminal()accepts optionalfontSizeparam;attachCustomKeyEventHandlerlets Cmd+/-/0/= bubble to DOM instead of being sent to the PTYfrontend/src/services/TerminalPool.ts— Uses current font size when creating new terminalsfrontend/src/hooks/useCodeReviewTerminal.ts— Uses current font size at creation and reacts to runtime changes viacreateEffectfrontend/src/App.tsx— Side-effect import to register shortcuts at startupTest plan
Cmd+=increases font size in active terminalCmd++increases font size (Norwegian/non-US keyboards)Cmd+-decreases font sizeCmd+0resets to defaultcd frontend && pnpm test