From 401bef1d2b8dfc3e173ca4b4d4f867264d126e60 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Fri, 13 Feb 2026 15:33:02 -0800 Subject: [PATCH] bug(ui) fix flicker refreshing background color (#19041) --- packages/cli/src/ui/AppContainer.tsx | 3 +- .../src/ui/components/ThemeDialog.test.tsx | 5 +- .../cli/src/ui/components/ThemeDialog.tsx | 8 +--- .../src/ui/hooks/useTerminalTheme.test.tsx | 48 +++++++++++++------ packages/cli/src/ui/hooks/useTerminalTheme.ts | 16 ++++++- packages/cli/src/ui/hooks/useThemeCommand.ts | 4 +- 6 files changed, 55 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 31e90f824e..5b33013846 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -612,11 +612,10 @@ export const AppContainer = (props: AppContainerProps) => { setThemeError, historyManager.addItem, initializationResult.themeError, + refreshStatic, ); - // Poll for terminal background color changes to auto-switch theme useTerminalTheme(handleThemeSelect, config, refreshStatic); - const { authState, setAuthState, diff --git a/packages/cli/src/ui/components/ThemeDialog.test.tsx b/packages/cli/src/ui/components/ThemeDialog.test.tsx index 165d4a52a2..21ceb8dacc 100644 --- a/packages/cli/src/ui/components/ThemeDialog.test.tsx +++ b/packages/cli/src/ui/components/ThemeDialog.test.tsx @@ -79,14 +79,12 @@ describe('ThemeDialog Snapshots', () => { }); }); - it('should call refreshStatic when a theme is selected', async () => { - const mockRefreshStatic = vi.fn(); + it('should call onSelect when a theme is selected', async () => { const settings = createMockSettings(); const { stdin } = renderWithProviders( , { settings, - uiActions: { refreshStatic: mockRefreshStatic }, }, ); @@ -96,7 +94,6 @@ describe('ThemeDialog Snapshots', () => { }); await waitFor(() => { - expect(mockRefreshStatic).toHaveBeenCalled(); expect(baseProps.onSelect).toHaveBeenCalled(); }); }); diff --git a/packages/cli/src/ui/components/ThemeDialog.tsx b/packages/cli/src/ui/components/ThemeDialog.tsx index 65e26aae49..9356b81e3b 100644 --- a/packages/cli/src/ui/components/ThemeDialog.tsx +++ b/packages/cli/src/ui/components/ThemeDialog.tsx @@ -22,7 +22,6 @@ import { getScopeMessageForSetting } from '../../utils/dialogScopeUtils.js'; import { useKeypress } from '../hooks/useKeypress.js'; import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; import { ScopeSelector } from './shared/ScopeSelector.js'; -import { useUIActions } from '../contexts/UIActionsContext.js'; import { useUIState } from '../contexts/UIStateContext.js'; interface ThemeDialogProps { @@ -85,7 +84,6 @@ export function ThemeDialog({ terminalWidth, }: ThemeDialogProps): React.JSX.Element { const isAlternateBuffer = useAlternateBuffer(); - const { refreshStatic } = useUIActions(); const { terminalBackgroundColor } = useUIState(); const [selectedScope, setSelectedScope] = useState( SettingScope.User, @@ -142,9 +140,8 @@ export function ThemeDialog({ const handleThemeSelect = useCallback( async (themeName: string) => { await onSelect(themeName, selectedScope); - refreshStatic(); }, - [onSelect, selectedScope, refreshStatic], + [onSelect, selectedScope], ); const handleThemeHighlight = (themeName: string) => { @@ -159,9 +156,8 @@ export function ThemeDialog({ const handleScopeSelect = useCallback( async (scope: LoadableSettingScope) => { await onSelect(highlightedThemeName, scope); - refreshStatic(); }, - [onSelect, highlightedThemeName, refreshStatic], + [onSelect, highlightedThemeName], ); const [mode, setMode] = useState<'theme' | 'scope'>('theme'); diff --git a/packages/cli/src/ui/hooks/useTerminalTheme.test.tsx b/packages/cli/src/ui/hooks/useTerminalTheme.test.tsx index 21eabba6cc..654102a740 100644 --- a/packages/cli/src/ui/hooks/useTerminalTheme.test.tsx +++ b/packages/cli/src/ui/hooks/useTerminalTheme.test.tsx @@ -9,8 +9,8 @@ import { useTerminalTheme } from './useTerminalTheme.js'; import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; import { makeFakeConfig, type Config } from '@google/gemini-cli-core'; import os from 'node:os'; +import { themeManager } from '../themes/theme-manager.js'; -// Mocks const mockWrite = vi.fn(); const mockSubscribe = vi.fn(); const mockUnsubscribe = vi.fn(); @@ -72,9 +72,7 @@ describe('useTerminalTheme', () => { config = makeFakeConfig({ targetDir: os.tmpdir(), }); - // Set initial background to ensure the hook passes the startup check. config.setTerminalBackground('#000000'); - // Spy on future updates. vi.spyOn(config, 'setTerminalBackground'); mockWrite.mockClear(); @@ -82,7 +80,7 @@ describe('useTerminalTheme', () => { mockUnsubscribe.mockClear(); mockHandleThemeSelect.mockClear(); mockQueryTerminalBackground.mockClear(); - // Reset any settings modifications + vi.mocked(themeManager.setTerminalBackground).mockClear(); mockSettings.merged.ui.autoThemeSwitching = true; mockSettings.merged.ui.theme = 'default'; }); @@ -108,7 +106,6 @@ describe('useTerminalTheme', () => { it('should poll for terminal background', () => { renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn())); - // Fast-forward time (1 minute) vi.advanceTimersByTime(60000); expect(mockQueryTerminalBackground).toHaveBeenCalled(); }); @@ -117,20 +114,23 @@ describe('useTerminalTheme', () => { config.getTerminalBackground = vi.fn().mockReturnValue(undefined); renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn())); - // Poll should not happen vi.advanceTimersByTime(60000); expect(mockQueryTerminalBackground).not.toHaveBeenCalled(); }); - it('should switch to light theme when background is light', () => { - renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn())); + it('should switch to light theme when background is light and not call refreshStatic directly', () => { + const refreshStatic = vi.fn(); + renderHook(() => + useTerminalTheme(mockHandleThemeSelect, config, refreshStatic), + ); const handler = mockSubscribe.mock.calls[0][0]; - // Simulate light background response (white) handler('rgb:ffff/ffff/ffff'); expect(config.setTerminalBackground).toHaveBeenCalledWith('#ffffff'); + expect(themeManager.setTerminalBackground).toHaveBeenCalledWith('#ffffff'); + expect(refreshStatic).not.toHaveBeenCalled(); expect(mockHandleThemeSelect).toHaveBeenCalledWith( 'default-light', expect.anything(), @@ -138,31 +138,51 @@ describe('useTerminalTheme', () => { }); it('should switch to dark theme when background is dark', () => { - // Start with light theme mockSettings.merged.ui.theme = 'default-light'; - renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn())); + config.setTerminalBackground('#ffffff'); + + const refreshStatic = vi.fn(); + renderHook(() => + useTerminalTheme(mockHandleThemeSelect, config, refreshStatic), + ); const handler = mockSubscribe.mock.calls[0][0]; - // Simulate dark background response (black) handler('rgb:0000/0000/0000'); expect(config.setTerminalBackground).toHaveBeenCalledWith('#000000'); + expect(themeManager.setTerminalBackground).toHaveBeenCalledWith('#000000'); + expect(refreshStatic).not.toHaveBeenCalled(); expect(mockHandleThemeSelect).toHaveBeenCalledWith( 'default', expect.anything(), ); - // Reset theme mockSettings.merged.ui.theme = 'default'; }); + it('should not update config or call refreshStatic on repeated identical background reports', () => { + const refreshStatic = vi.fn(); + renderHook(() => + useTerminalTheme(mockHandleThemeSelect, config, refreshStatic), + ); + + const handler = mockSubscribe.mock.calls[0][0]; + + handler('rgb:0000/0000/0000'); + + expect(config.setTerminalBackground).not.toHaveBeenCalled(); + expect(themeManager.setTerminalBackground).not.toHaveBeenCalled(); + expect(refreshStatic).not.toHaveBeenCalled(); + + expect(mockHandleThemeSelect).not.toHaveBeenCalled(); + }); + it('should not switch theme if autoThemeSwitching is disabled', () => { mockSettings.merged.ui.autoThemeSwitching = false; renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn())); - // Poll should not happen vi.advanceTimersByTime(60000); expect(mockQueryTerminalBackground).not.toHaveBeenCalled(); diff --git a/packages/cli/src/ui/hooks/useTerminalTheme.ts b/packages/cli/src/ui/hooks/useTerminalTheme.ts index b3ac7522bc..9927847758 100644 --- a/packages/cli/src/ui/hooks/useTerminalTheme.ts +++ b/packages/cli/src/ui/hooks/useTerminalTheme.ts @@ -56,11 +56,18 @@ export function useTerminalTheme( if (!match) return; const hexColor = parseColor(match[1], match[2], match[3]); - const luminance = getLuminance(hexColor); + if (!hexColor) return; + + const previousColor = config.getTerminalBackground(); + + if (previousColor === hexColor) { + return; + } + config.setTerminalBackground(hexColor); themeManager.setTerminalBackground(hexColor); - refreshStatic(); + const luminance = getLuminance(hexColor); const currentThemeName = settings.merged.ui.theme; const newTheme = shouldSwitchTheme( @@ -72,6 +79,11 @@ export function useTerminalTheme( if (newTheme) { void handleThemeSelect(newTheme, SettingScope.User); + } else { + // The existing theme had its background changed so refresh because + // there may be existing static UI rendered that relies on the old + // background color. + refreshStatic(); } }; diff --git a/packages/cli/src/ui/hooks/useThemeCommand.ts b/packages/cli/src/ui/hooks/useThemeCommand.ts index d1d17da428..15eaa875a2 100644 --- a/packages/cli/src/ui/hooks/useThemeCommand.ts +++ b/packages/cli/src/ui/hooks/useThemeCommand.ts @@ -31,6 +31,7 @@ export const useThemeCommand = ( setThemeError: (error: string | null) => void, addItem: UseHistoryManagerReturn['addItem'], initialThemeError: string | null, + refreshStatic: () => void, ): UseThemeCommandReturn => { const [isThemeDialogOpen, setIsThemeDialogOpen] = useState(!!initialThemeError); @@ -102,12 +103,13 @@ export const useThemeCommand = ( themeManager.loadCustomThemes(loadedSettings.merged.ui.customThemes); } applyTheme(loadedSettings.merged.ui.theme); // Apply the current theme + refreshStatic(); setThemeError(null); } finally { setIsThemeDialogOpen(false); // Close the dialog } }, - [applyTheme, loadedSettings, setThemeError], + [applyTheme, loadedSettings, refreshStatic, setThemeError], ); return {