From 09f213c48d27ef9fda2f34ea0c00c7846ae0a0be Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:18:03 -0700 Subject: [PATCH] fix(terminal): inset the grid inside the xterm surface (#13252) Padding X/Y was applied as start-edge container margin, so the cell grid stayed flush on the trailing edges and a fractional background opacity stacked a darker gutter around the viewport. Put the setting on .xterm so FitAddon insets both axes and the themed background fills the pad. --- .../terminal-container-geometry.test.ts | 21 ++++++++++++-- src/renderer/src/assets/terminal.css | 28 +++++++++++++------ .../terminal-pane/terminal-appearance.test.ts | 18 ++++++++++++ .../terminal-pane/terminal-appearance.ts | 26 +++++++++-------- 4 files changed, 70 insertions(+), 23 deletions(-) diff --git a/src/renderer/src/assets/terminal-container-geometry.test.ts b/src/renderer/src/assets/terminal-container-geometry.test.ts index db079c9095f..501a31e3e58 100644 --- a/src/renderer/src/assets/terminal-container-geometry.test.ts +++ b/src/renderer/src/assets/terminal-container-geometry.test.ts @@ -5,9 +5,7 @@ const terminalCss = fs.readFileSync(new URL('./terminal.css', import.meta.url), describe('terminal container geometry', () => { it('keeps the hidden link tooltip out of the fitted terminal height', () => { - expect(terminalCss).toMatch( - /\.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--pane-padding-y, 4px\)\);/s - ) + expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*height:\s*100%;/s) expect(terminalCss).toMatch( /\.pane\[data-has-title\] \.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--orca-pane-title-height\)\);/s ) @@ -15,4 +13,21 @@ describe('terminal container geometry', () => { /\.pane-link-tooltip\s*{[^}]*height:\s*var\(--orca-terminal-link-tooltip-height\);/s ) }) + + it('insets the xterm grid on both axes from pane padding', () => { + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-top:\s*var\(--pane-padding-y, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-right:\s*var\(--pane-padding-x, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-bottom:\s*var\(--pane-padding-y, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-left:\s*var\(--pane-padding-x, 4px\);/s + ) + expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*width:\s*100%;/s) + expect(terminalCss).not.toMatch(/\.xterm-container\s*{[^}]*margin-left:/s) + }) }) diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index ee5e0af01f5..86864ffe614 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -30,6 +30,20 @@ width: 100%; } +.xterm-container .xterm { + box-sizing: border-box; + height: 100%; + width: 100%; + /* Why on .xterm, not the container: FitAddon subtracts this element's + padding when proposing cols/rows, and the themed background (including + opacity) fills the inset. Container margins only framed the start edges + and stacked a darker gutter at fractional opacity. */ + padding-top: var(--pane-padding-y, 4px); + padding-right: var(--pane-padding-x, 4px); + padding-bottom: var(--pane-padding-y, 4px); + padding-left: var(--pane-padding-x, 4px); +} + .pane-manager-root .xterm-viewport { /* Why: xterm/VS Code keep the scrollbar gutter stable. `auto` lets Linux DOM scrollbars appear/disappear, which can feed resize/reflow jitter. */ @@ -177,7 +191,7 @@ .pane[data-terminal-attention] .xterm-container::after { content: ''; position: absolute; - inset: 0 4px 4px 0; + inset: 0; z-index: 5; pointer-events: none; border-radius: 2px; @@ -498,16 +512,14 @@ border-color: var(--ring); } -/* Baseline xterm-container layout — padding is driven by CSS custom properties - so Ghostty-imported window-padding-x/y can be applied without inline styles. - The fallback 4px preserves the legacy default. */ +/* Container fills the pane so the .xterm background (and its padding) is the + terminal surface. Title offset stays here because the title overlay is a + sibling of the container, not of .xterm. */ .xterm-container { box-sizing: border-box; position: relative; - width: calc(100% - var(--pane-padding-x, 4px)); - height: calc(100% - var(--pane-padding-y, 4px)); - margin-top: var(--pane-padding-y, 4px); - margin-left: var(--pane-padding-x, 4px); + width: 100%; + height: 100%; } /* When a pane has a title, shift the terminal content down to make room. diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts index 6f7bec8592b..42a16b1613a 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts @@ -360,6 +360,24 @@ describe('applyTerminalAppearance theme assignment', () => { // Latest wins, exactly one write: intermediate hidden values never touch xterm. expect(writes).toEqual([21]) }) + + it('stamps settings-UI padding defaults when padding is unset', () => { + const pane = makePane(1) + const manager = makeManager([pane]) + applyTerminalAppearance( + manager, + getDefaultSettings('/tmp'), + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + expect(manager.setPaneStyleOptions).toHaveBeenCalledWith( + expect.objectContaining({ paddingX: 4, paddingY: 4 }) + ) + }) }) describe('publishTerminalViewAttributesAtAppStart', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.ts index bb61a50fbbc..fbdb99170b3 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -159,6 +159,20 @@ export function applyTerminalAppearance( settings.terminalFontFamily ) + // Why before the pane loop: FitAddon subtracts live .xterm padding, so the + // CSS vars must be stamped before safeFit or a padding shrink leaves a stale grid. + manager.setPaneStyleOptions({ + splitBackground: paneBackground, + paneBackground, + inactivePaneOpacity: paneStyles.inactivePaneOpacity, + activePaneOpacity: paneStyles.activePaneOpacity, + opacityTransitionMs: paneStyles.opacityTransitionMs, + dividerThicknessPx: paneStyles.dividerThicknessPx, + focusFollowsMouse: paneStyles.focusFollowsMouse, + paddingX: settings.terminalPaddingX ?? 4, + paddingY: settings.terminalPaddingY ?? 4 + }) + for (const pane of manager.getPanes()) { // Why value-gated: writing options.theme rebuilds the palette, discarding TUI OSC 4/10/11/12 mutations; skip on no-op change. if (theme && !composedTerminalThemesEqual(pane.terminal.options.theme, theme)) { @@ -227,16 +241,4 @@ export function applyTerminalAppearance( safeFit(pane) } } - - manager.setPaneStyleOptions({ - splitBackground: paneBackground, - paneBackground, - inactivePaneOpacity: paneStyles.inactivePaneOpacity, - activePaneOpacity: paneStyles.activePaneOpacity, - opacityTransitionMs: paneStyles.opacityTransitionMs, - dividerThicknessPx: paneStyles.dividerThicknessPx, - focusFollowsMouse: paneStyles.focusFollowsMouse, - paddingX: settings.terminalPaddingX, - paddingY: settings.terminalPaddingY - }) }