From 7c069eec6a291bb18c51494333beefc715b2afc0 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Sat, 12 Sep 2026 13:02:43 +0200 Subject: [PATCH 1/2] fix(tui): repair light/dark theme switching artifacts - Keep structural containers (pickerBox, inputBoxStyle) transparent: nested styled spans emit ANSI resets that punch through inherited parent backgrounds, causing gray/black cutout blocks - Force full repaint (tea.ClearScreen) on theme-picker navigation and Escape-cancel so stale cells from the previous polarity cannot linger - Replace background regression tests: containers must be transparent, leaf surfaces (status bar, metrics) stay solid - Add dark-to-light-to-dark preview transition test asserting repaint --- internal/tui/theme.go | 2 -- internal/tui/theme_test.go | 50 +++++++++++++++++++++++++++++++------ internal/tui/tui_backend.go | 4 ++- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/internal/tui/theme.go b/internal/tui/theme.go index c403c8c..ef17545 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -370,7 +370,6 @@ func ApplyTheme(t Theme) { pickerBox = lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(lipgloss.Color(t.SurfaceBorder)). - Background(lipgloss.Color(t.SurfaceDark)). Padding(0, 1) pickerSectionHeader = lipgloss.NewStyle(). Foreground(lipgloss.Color(t.TextDim)). @@ -441,7 +440,6 @@ func ApplyTheme(t Theme) { inputBoxStyle = lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(lipgloss.Color(t.SurfaceBorder)). - Background(lipgloss.Color(t.SurfaceDark)). Padding(0, 1) statusMetricsStyle = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextSubtle)).Background(lipgloss.Color(t.SurfaceDark)) diff --git a/internal/tui/theme_test.go b/internal/tui/theme_test.go index 7260f60..6265112 100644 --- a/internal/tui/theme_test.go +++ b/internal/tui/theme_test.go @@ -1,9 +1,11 @@ package tui import ( + "reflect" "strconv" "testing" + tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" ) @@ -129,25 +131,34 @@ func TestApplyTheme_SetsPolarity(t *testing.T) { ApplyTheme(ThemeByName("historic")) } -// TestApplyTheme_BackgroundsFollowTheme guards the invisible-text bug: after -// ApplyTheme, surfaces that promise a solid background must actually carry -// the theme's SurfaceDark color, so menus stay readable when the terminal -// background polarity mismatches the selected theme. -func TestApplyTheme_BackgroundsFollowTheme(t *testing.T) { +// TestApplyTheme_BackgroundOwnership guards against ANSI cutouts. Structural +// containers stay transparent because nested styled spans emit reset sequences +// that interrupt inherited parent backgrounds. Leaf surfaces may be solid. +func TestApplyTheme_BackgroundOwnership(t *testing.T) { for _, name := range ThemeNames() { theme := ThemeByName(name) ApplyTheme(theme) surface := lipgloss.Color(theme.SurfaceDark) - cases := []struct { + transparent := []struct { label string got lipgloss.TerminalColor }{ {"pickerBox", pickerBox.GetBackground()}, {"inputBoxStyle", inputBoxStyle.GetBackground()}, + } + for _, c := range transparent { + if _, ok := c.got.(lipgloss.NoColor); !ok { + t.Errorf("ApplyTheme(%q): %s background = %v, want transparent", name, c.label, c.got) + } + } + solid := []struct { + label string + got lipgloss.TerminalColor + }{ {"statusBarStyle", statusBarStyle.GetBackground()}, {"statusMetricsStyle", statusMetricsStyle.GetBackground()}, } - for _, c := range cases { + for _, c := range solid { if c.got != surface { t.Errorf("ApplyTheme(%q): %s background = %v, want %v", name, c.label, c.got, surface) } @@ -161,7 +172,7 @@ func TestApplyTheme_BackgroundsFollowTheme(t *testing.T) { // TestThemePalette_SelectionContrastsSurface guards the picker selection // highlight: SurfaceSelect must be visibly distinct from SurfaceDark now -// that pickerBox paints its own background behind every row. +// selected rows still need a visible state transition on either polarity. func TestThemePalette_SelectionContrastsSurface(t *testing.T) { for _, name := range ThemeNames() { theme := ThemeByName(name) @@ -221,3 +232,26 @@ func TestApplyTheme_RefreshesLiveTerminalRenderer(t *testing.T) { t.Error("ApplyTheme(historic) left the renderer on light style; want dark") } } + +func TestThemePicker_PolarityTransitionsRequestFullRepaint(t *testing.T) { + m := newThemePickerModel("aurora") // last dark theme; next is light + + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + m = updated.(themePickerModel) + if got := m.themes[m.cursor]; got != "paper" { + t.Fatalf("down transition selected %q, want paper", got) + } + if cmd == nil || reflect.TypeOf(cmd()) != reflect.TypeOf(tea.ClearScreen()) { + t.Fatal("dark-to-light preview did not request a full-screen repaint") + } + + updated, cmd = m.Update(tea.KeyMsg{Type: tea.KeyUp}) + m = updated.(themePickerModel) + if got := m.themes[m.cursor]; got != "aurora" { + t.Fatalf("up transition selected %q, want aurora", got) + } + if cmd == nil || reflect.TypeOf(cmd()) != reflect.TypeOf(tea.ClearScreen()) { + t.Fatal("light-to-dark preview did not request a full-screen repaint") + } + ApplyTheme(ThemeByName("historic")) +} diff --git a/internal/tui/tui_backend.go b/internal/tui/tui_backend.go index 0ea8b79..573c7fe 100644 --- a/internal/tui/tui_backend.go +++ b/internal/tui/tui_backend.go @@ -755,18 +755,20 @@ func (m themePickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "ctrl+c", "esc", "q": m.cancelled = true ApplyTheme(ThemeByName(m.originalTheme)) - return m, tea.Quit + return m, tea.Sequence(tea.ClearScreen, tea.Quit) case "up", "k": if m.cursor > 0 { m.cursor-- ApplyTheme(ThemeByName(m.themes[m.cursor])) + return m, tea.ClearScreen } case "down", "j": if m.cursor < len(m.themes)-1 { m.cursor++ ApplyTheme(ThemeByName(m.themes[m.cursor])) + return m, tea.ClearScreen } case "enter", " ": From f805c7cf2e1f7be78abd27649c2799604026d1f3 Mon Sep 17 00:00:00 2001 From: Ruslan Strazhnyk Date: Sat, 12 Sep 2026 13:44:22 +0200 Subject: [PATCH 2/2] fix(tui): keep picker chrome readable on polarity mismatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial-review follow-ups: - Foreground-only picker chrome (labels, headers, footers, dividers, hints) now keys off the terminal's actual background polarity instead of the previewed theme's. Previewing a light theme on a dark terminal (or vice versa) previously rendered near-black labels on a dark background — the exact invisible-text bug #195 tried to fix. Values match same-polarity themes exactly, so matched setups are unchanged. - Gate tea.ClearScreen on actual polarity flips: clearing erases the visible screenful in place, so every arrow key was destroying the on-screen transcript. Same-polarity moves rely on the renderer's line diff; cancel only clears when the restore flips polarity. - Skip glamour renderer rebuilds when polarity is unchanged. - Tests: same-polarity move must not clear, no-flip cancel must quit without clear, chrome polarity adaptation, renderer rebuild elision. --- internal/tui/terminal.go | 9 +++- internal/tui/theme.go | 36 ++++++++++++---- internal/tui/theme_test.go | 84 +++++++++++++++++++++++++++++++++++++ internal/tui/tui_backend.go | 19 +++++++-- 4 files changed, 135 insertions(+), 13 deletions(-) diff --git a/internal/tui/terminal.go b/internal/tui/terminal.go index ec7686c..2cf3165 100644 --- a/internal/tui/terminal.go +++ b/internal/tui/terminal.go @@ -285,8 +285,15 @@ func NewTerminal() *Terminal { // setMarkdownStyle rebuilds the glamour renderer for the given background // polarity. It keeps the previous renderer when construction fails so a -// failed swap never disables markdown rendering. +// failed swap never disables markdown rendering, and skips the rebuild when +// the polarity is unchanged. func (t *Terminal) setMarkdownStyle(dark bool) { + t.rendMu.Lock() + unchanged := t.renderer != nil && dark == t.markdownDark + t.rendMu.Unlock() + if unchanged { + return + } gStyle := "light" if dark { gStyle = "dark" diff --git a/internal/tui/theme.go b/internal/tui/theme.go index ef17545..1f2b2dd 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -344,12 +344,30 @@ var ( ThemeIsDark = true ) +// terminalDark captures the terminal's actual background polarity once at +// package init, before any ApplyTheme call overrides lipgloss's detection +// with the selected theme's polarity. Foreground-only picker chrome keys off +// this so opposite-polarity previews (e.g. a light theme on a dark terminal) +// stay readable on the live terminal. +var terminalDark = lipgloss.HasDarkBackground() + // ApplyTheme rebuilds all lipgloss styles and ANSI prompt vars from t. // Must be called before NewTerminal() and ShowModelPicker(). func ApplyTheme(t Theme) { ThemeIsDark = t.Dark lipgloss.SetHasDarkBackground(t.Dark) + // Neutral foreground-only chrome follows the terminal's real polarity, + // not the theme's: these styles carry no background, so a theme/terminal + // polarity mismatch would render its own neutrals invisible (near-black + // labels on a dark terminal while previewing a light theme, and vice + // versa). The values are identical to same-polarity themes, so matched + // setups render exactly as before. + labelFg, dimFg, subtleFg, sepFg := lipgloss.Color("252"), lipgloss.Color("242"), lipgloss.Color("240"), lipgloss.Color("237") + if !terminalDark { + labelFg, dimFg, subtleFg, sepFg = lipgloss.Color("236"), lipgloss.Color("243"), lipgloss.Color("247"), lipgloss.Color("252") + } + // ANSI prompt/banner vars themePromptName = t.ANSIPromptName themePromptArrow = t.ANSIPromptArrow @@ -372,7 +390,7 @@ func ApplyTheme(t Theme) { BorderForeground(lipgloss.Color(t.SurfaceBorder)). Padding(0, 1) pickerSectionHeader = lipgloss.NewStyle(). - Foreground(lipgloss.Color(t.TextDim)). + Foreground(dimFg). PaddingTop(1) pickerRowSelected = lipgloss.NewStyle(). Background(lipgloss.Color(t.SurfaceSelect)). @@ -381,7 +399,7 @@ func ApplyTheme(t Theme) { pickerIcon = lipgloss.NewStyle().Foreground(lipgloss.Color(t.Brand)) pickerIconCodex = lipgloss.NewStyle().Foreground(lipgloss.Color(t.IconCodex)) pickerIconAPI = lipgloss.NewStyle().Foreground(lipgloss.Color(t.IconAPI)) - pickerLabel = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextNormal)) + pickerLabel = lipgloss.NewStyle().Foreground(labelFg) pickerLabelSel = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextBright)).Bold(true) pickerBadgeNew = lipgloss.NewStyle(). Foreground(lipgloss.Color("0")). @@ -390,19 +408,19 @@ func ApplyTheme(t Theme) { PaddingLeft(1).PaddingRight(1) pickerBadgeCurrent = lipgloss.NewStyle().Foreground(lipgloss.Color(t.Success)).Bold(true) pickerBadgeStar = lipgloss.NewStyle().Foreground(lipgloss.Color(t.Accent)) - pickerBadgeExt = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextDim)) - pickerShortcut = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextSubtle)) - pickerDivider = lipgloss.NewStyle().Foreground(lipgloss.Color(t.SurfaceSep)) + pickerBadgeExt = lipgloss.NewStyle().Foreground(dimFg) + pickerShortcut = lipgloss.NewStyle().Foreground(subtleFg) + pickerDivider = lipgloss.NewStyle().Foreground(sepFg) effortLabelActive = lipgloss.NewStyle(). Foreground(lipgloss.Color("0")). Background(lipgloss.Color(t.Brand)). Bold(true). PaddingLeft(2).PaddingRight(2) effortLabelInactive = lipgloss.NewStyle(). - Foreground(lipgloss.Color(t.TextDim)). + Foreground(dimFg). PaddingLeft(2).PaddingRight(2) pickerFooter = lipgloss.NewStyle(). - Foreground(lipgloss.Color(t.TextSubtle)). + Foreground(subtleFg). PaddingTop(1) pickerStatusBar = lipgloss.NewStyle(). Foreground(lipgloss.Color(t.TextNormal)). @@ -430,11 +448,11 @@ func ApplyTheme(t Theme) { menuItemStyle = lipgloss.NewStyle(). Foreground(lipgloss.Color(t.MenuItem)). PaddingLeft(1) - menuDescStyle = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextDim)) + menuDescStyle = lipgloss.NewStyle().Foreground(dimFg) menuDescSelSty = lipgloss.NewStyle(). Foreground(lipgloss.Color("0")). Background(lipgloss.Color(t.Brand)) - menuHintStyle = lipgloss.NewStyle().Foreground(lipgloss.Color(t.TextSubtle)) + menuHintStyle = lipgloss.NewStyle().Foreground(subtleFg) filterStyle = lipgloss.NewStyle().Foreground(lipgloss.Color(t.Accent)).Bold(true) inputBoxStyle = lipgloss.NewStyle(). diff --git a/internal/tui/theme_test.go b/internal/tui/theme_test.go index 6265112..bf903fa 100644 --- a/internal/tui/theme_test.go +++ b/internal/tui/theme_test.go @@ -245,6 +245,20 @@ func TestThemePicker_PolarityTransitionsRequestFullRepaint(t *testing.T) { t.Fatal("dark-to-light preview did not request a full-screen repaint") } + updated, cmd = m.Update(tea.KeyMsg{Type: tea.KeyDown}) + m = updated.(themePickerModel) + if got := m.themes[m.cursor]; got != "sky" { + t.Fatalf("same-polarity down selected %q, want sky", got) + } + if cmd != nil { + t.Fatal("same-polarity move requested a full-screen repaint; clearing erases the visible transcript and must be gated on polarity flips") + } + + updated, cmd = m.Update(tea.KeyMsg{Type: tea.KeyUp}) + m = updated.(themePickerModel) + if got := m.themes[m.cursor]; got != "paper" { + t.Fatalf("up selected %q, want paper", got) + } updated, cmd = m.Update(tea.KeyMsg{Type: tea.KeyUp}) m = updated.(themePickerModel) if got := m.themes[m.cursor]; got != "aurora" { @@ -253,5 +267,75 @@ func TestThemePicker_PolarityTransitionsRequestFullRepaint(t *testing.T) { if cmd == nil || reflect.TypeOf(cmd()) != reflect.TypeOf(tea.ClearScreen()) { t.Fatal("light-to-dark preview did not request a full-screen repaint") } + + m = newThemePickerModel("historic") + updated, cmd = m.Update(tea.KeyMsg{Type: tea.KeyEsc}) + m = updated.(themePickerModel) + if !m.cancelled { + t.Fatal("esc did not cancel") + } + if cmd == nil { + t.Fatal("esc produced no command") + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Fatal("cancel without a polarity change must quit without a screen clear") + } ApplyTheme(ThemeByName("historic")) } + +// TestApplyTheme_PickerChromeFollowsTerminalPolarity guards the invisible- +// chrome regression: foreground-only picker styles must key off the +// terminal's real polarity, so previewing (or running) an opposite-polarity +// theme keeps labels readable on the live terminal background. +func TestApplyTheme_PickerChromeFollowsTerminalPolarity(t *testing.T) { + orig := terminalDark + defer func() { + terminalDark = orig + ApplyTheme(ThemeByName("historic")) + }() + + terminalDark = true + ApplyTheme(ThemeByName("paper")) // light theme previewed on a dark terminal + if got := pickerLabel.GetForeground(); got != lipgloss.Color("252") { + t.Errorf("pickerLabel on dark terminal = %v, want light label 252", got) + } + if got := pickerFooter.GetForeground(); got != lipgloss.Color("240") { + t.Errorf("pickerFooter on dark terminal = %v, want subtle 240", got) + } + if got := pickerDivider.GetForeground(); got != lipgloss.Color("237") { + t.Errorf("pickerDivider on dark terminal = %v, want 237", got) + } + + terminalDark = false + ApplyTheme(ThemeByName("historic")) // dark theme on a light terminal + if got := pickerLabel.GetForeground(); got != lipgloss.Color("236") { + t.Errorf("pickerLabel on light terminal = %v, want dark label 236", got) + } + if got := pickerFooter.GetForeground(); got != lipgloss.Color("247") { + t.Errorf("pickerFooter on light terminal = %v, want subtle 247", got) + } +} + +// TestSetMarkdownStyle_SkipsRedundantRebuild ensures a same-polarity +// ApplyTheme does not churn the glamour renderer. +func TestSetMarkdownStyle_SkipsRedundantRebuild(t *testing.T) { + term := &Terminal{} + term.setMarkdownStyle(true) + if term.renderer == nil { + t.Fatal("initial build did not produce a renderer") + } + first := term.renderer + + term.setMarkdownStyle(true) + if term.renderer != first { + t.Error("same-polarity call rebuilt the renderer") + } + + term.setMarkdownStyle(false) + if term.renderer == first { + t.Error("polarity flip did not rebuild the renderer") + } + if term.markdownDark { + t.Error("markdownDark not updated after flip") + } +} diff --git a/internal/tui/tui_backend.go b/internal/tui/tui_backend.go index 573c7fe..aa484da 100644 --- a/internal/tui/tui_backend.go +++ b/internal/tui/tui_backend.go @@ -754,21 +754,34 @@ func (m themePickerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg.String() { case "ctrl+c", "esc", "q": m.cancelled = true + previewed := allThemes[m.themes[m.cursor]] ApplyTheme(ThemeByName(m.originalTheme)) - return m, tea.Sequence(tea.ClearScreen, tea.Quit) + // Clearing erases the visible screenful in place, so only pay + // that cost when the restore actually flips polarity; otherwise + // the renderer's line diff is enough. + if allThemes[m.originalTheme].Dark != previewed.Dark { + return m, tea.Sequence(tea.ClearScreen, tea.Quit) + } + return m, tea.Quit case "up", "k": if m.cursor > 0 { + prev := allThemes[m.themes[m.cursor]] m.cursor-- ApplyTheme(ThemeByName(m.themes[m.cursor])) - return m, tea.ClearScreen + if allThemes[m.themes[m.cursor]].Dark != prev.Dark { + return m, tea.ClearScreen + } } case "down", "j": if m.cursor < len(m.themes)-1 { + prev := allThemes[m.themes[m.cursor]] m.cursor++ ApplyTheme(ThemeByName(m.themes[m.cursor])) - return m, tea.ClearScreen + if allThemes[m.themes[m.cursor]].Dark != prev.Dark { + return m, tea.ClearScreen + } } case "enter", " ":