From 9c29ecea0f021aa0d9a796e2f9aefcb3472fe6c6 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:27:13 -0700 Subject: [PATCH] refactor(terminal): extract contrast correction gating into shared helpe Centralizes the minimumContrastRatio gating logic (#7934) into a dedicated module so both the settings preview and live terminal appearance apply the same contrast-by-luminance rule, preventing drift. Adds value-gating to skip redundant writes that would clear xterm's contrast cache on re-applies. --- .../settings/TerminalSettingsPreview.tsx | 13 +++++----- .../terminal-pane/terminal-appearance.test.ts | 24 +++++++++++++++++++ .../terminal-pane/terminal-appearance.ts | 16 ++++++------- .../lib/pane-manager/pane-lifecycle.test.ts | 3 +-- .../lib/pane-manager/pane-terminal-options.ts | 8 +++---- .../src/lib/terminal-contrast-correction.ts | 18 ++++++++++++++ 6 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 src/renderer/src/lib/terminal-contrast-correction.ts diff --git a/src/renderer/src/components/settings/TerminalSettingsPreview.tsx b/src/renderer/src/components/settings/TerminalSettingsPreview.tsx index 2b220c70a1a..d9dee715dd7 100644 --- a/src/renderer/src/components/settings/TerminalSettingsPreview.tsx +++ b/src/renderer/src/components/settings/TerminalSettingsPreview.tsx @@ -8,7 +8,7 @@ import { buildDefaultTerminalOptions } from '@/lib/pane-manager/pane-terminal-op import { buildFontFamily } from '@/components/terminal-pane/layout-serialization' import { composeActiveTerminalTheme } from '@/components/terminal-pane/terminal-appearance' import { clampNumber, resolveEffectiveTerminalAppearance } from '@/lib/terminal-theme' -import { isTerminalBackgroundLight } from '@/lib/terminal-title-contrast' +import { resolveTerminalMinimumContrastRatio } from '@/lib/terminal-contrast-correction' import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { resolveTerminalLigaturesEnabled } from '../../../../shared/terminal-ligatures' import { normalizeTerminalLineHeight } from '../../../../shared/terminal-line-height-settings' @@ -196,12 +196,11 @@ export function TerminalSettingsPreview({ return } terminal.options.theme = composedTheme - // Why: mirror applyTerminalAppearance's background-luminance gating (#7934) so the preview matches live panes. - terminal.options.minimumContrastRatio = isTerminalBackgroundLight(composedTheme.background, { - appSurface: effectiveMode - }) - ? 4.5 - : 1 + // Why: share applyTerminalAppearance's gating helper (#7934) so the preview can't drift from live panes. + terminal.options.minimumContrastRatio = resolveTerminalMinimumContrastRatio( + composedTheme.background, + effectiveMode + ) // Why: xterm renders an alpha-channel background opaque unless allowTransparency is set (matches applyTerminalAppearance). terminal.options.allowTransparency = settings.terminalBackgroundOpacity !== undefined && settings.terminalBackgroundOpacity < 1 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 9c3742febb3..fdf3da8e271 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts @@ -455,6 +455,30 @@ describe('applyTerminalAppearance theme assignment', () => { expect(pane.terminal.options.minimumContrastRatio).toBe(4.5) }) + + it('skips the minimumContrastRatio write on a no-op re-apply (preserves xterm contrast cache)', () => { + const pane = makePane(1) + let writes = 0 + let stored: number | undefined + Object.defineProperty(pane.terminal.options, 'minimumContrastRatio', { + configurable: true, + enumerable: true, + get: () => stored, + set: (value: number) => { + stored = value + writes += 1 + } + }) + const settings = getDefaultSettings('/tmp') + + apply(pane, { ...settings, theme: 'dark' }) + const writesAfterFirst = writes + + apply(pane, { ...settings, theme: 'dark' }) + + // The value-gate must not rewrite an unchanged ratio — each write clears xterm's contrast cache. + expect(writes).toBe(writesAfterFirst) + }) }) 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 eb926740cab..fa93552388a 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -24,7 +24,7 @@ import type { TerminalViewAttributes } from '../../../../shared/terminal-view-at import { publishTerminalViewAttributes } from './terminal-view-attributes-publisher' import { normalizeTerminalLineHeight } from '../../../../shared/terminal-line-height-settings' import { maybePushMode2031Flip } from './terminal-mode-2031-replies' -import { isTerminalBackgroundLight } from '@/lib/terminal-title-contrast' +import { resolveTerminalMinimumContrastRatio } from '@/lib/terminal-contrast-correction' // Why Pick over a hand-rolled type: stays tied to xterm's canonical signature so upstream tightening surfaces here. type Mode2031Parser = Pick @@ -208,15 +208,13 @@ export function applyTerminalAppearance( if (theme && !composedTerminalThemesEqual(pane.terminal.options.theme, theme)) { pane.terminal.options.theme = theme } - // Why gate by background luminance, not appearance.mode (#7934): xterm's contrast correction rescues - // invisible white/bright-white ANSI text on light backgrounds but over-corrects vibrant ANSI colors on - // dark ones — and either theme slot can hold either kind (match-dark-mode, light theme in the dark slot). + // Gate off the configured theme background; the live OSC-11 background is deliberately preserved by the + // theme write above, so a TUI that repaints its background at runtime won't re-gate (known limitation). // Why value-gated: writing minimumContrastRatio clears xterm's contrast cache, so skip on no-op re-applies. - const minimumContrastRatio = isTerminalBackgroundLight(theme?.background, { - appSurface: appearance.mode - }) - ? 4.5 - : 1 + const minimumContrastRatio = resolveTerminalMinimumContrastRatio( + theme?.background, + appearance.mode + ) if (pane.terminal.options.minimumContrastRatio !== minimumContrastRatio) { pane.terminal.options.minimumContrastRatio = minimumContrastRatio } diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index e4c7c87f224..607ad1cdb5d 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -114,8 +114,7 @@ describe('buildDefaultTerminalOptions', () => { expect(normalizeTerminalFastScrollSensitivity(25)).toBe(20) }) - it('starts with light-theme contrast correction so first paint never flashes low-contrast text', () => { - // Initial value is the light-background setting (4.5); applyTerminalAppearance re-gates it by background luminance (dark bg => 1; #7934). + it('defaults minimumContrastRatio to the light-background value (applyTerminalAppearance re-gates it)', () => { expect(buildDefaultTerminalOptions().minimumContrastRatio).toBe(4.5) }) diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-options.ts b/src/renderer/src/lib/pane-manager/pane-terminal-options.ts index b183073ded0..1fc47513483 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-options.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-options.ts @@ -1,5 +1,6 @@ import type { ITerminalOptions } from '@xterm/xterm' import { DESKTOP_TERMINAL_SCROLLBACK_ROWS_DEFAULT } from '../../../../shared/terminal-scrollback-policy' +import { LIGHT_BG_MIN_CONTRAST } from '@/lib/terminal-contrast-correction' type TerminalCursorStyle = NonNullable type TerminalCursorInactiveStyle = NonNullable @@ -47,11 +48,8 @@ export function buildDefaultTerminalOptions(): ITerminalOptions { scrollSensitivity: DEFAULT_TERMINAL_SCROLL_SENSITIVITY, fastScrollSensitivity: DEFAULT_TERMINAL_FAST_SCROLL_SENSITIVITY, allowTransparency: false, - // Why 4.5 as the initial value: agent CLIs render body text with ANSI white/bright-white that - // vanishes on light (white) backgrounds. applyTerminalAppearance re-gates this by the theme's - // background luminance (light bg => 4.5, dark bg => 1; #7934) — start high so the first paint on - // a light theme never flashes low-contrast text before appearance-apply runs. - minimumContrastRatio: 4.5, + // Initial value only; applyTerminalAppearance re-gates by background luminance (#7934) before any content paints. + minimumContrastRatio: LIGHT_BG_MIN_CONTRAST, // Why: on macOS, non-US layouts rely on Option to compose characters like @ and €. macOptionIsMeta: false, macOptionClickForcesSelection: true, diff --git a/src/renderer/src/lib/terminal-contrast-correction.ts b/src/renderer/src/lib/terminal-contrast-correction.ts new file mode 100644 index 00000000000..4cd147af812 --- /dev/null +++ b/src/renderer/src/lib/terminal-contrast-correction.ts @@ -0,0 +1,18 @@ +import { isTerminalBackgroundLight } from '@/lib/terminal-title-contrast' + +// xterm minimumContrastRatio tuning (#7934). Light backgrounds keep WCAG-AA correction so invisible +// white/bright-white ANSI body text stays readable; dark backgrounds disable it (ratio 1) because +// correction over-brightens vibrant ANSI colors. +export const LIGHT_BG_MIN_CONTRAST = 4.5 +export const DARK_BG_MIN_CONTRAST = 1 + +// Why gate by background luminance, not app mode (#7934): either theme slot can hold either kind of +// theme (match-dark-mode, or a light theme in the dark slot), so follow the composed background. +export function resolveTerminalMinimumContrastRatio( + background: string | undefined, + appSurface: 'dark' | 'light' +): number { + return isTerminalBackgroundLight(background, { appSurface }) + ? LIGHT_BG_MIN_CONTRAST + : DARK_BG_MIN_CONTRAST +}