mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-25 20:44:46 -07:00
bug(ui) fix flicker refreshing background color (#19041)
This commit is contained in:
@@ -612,11 +612,10 @@ export const AppContainer = (props: AppContainerProps) => {
|
|||||||
setThemeError,
|
setThemeError,
|
||||||
historyManager.addItem,
|
historyManager.addItem,
|
||||||
initializationResult.themeError,
|
initializationResult.themeError,
|
||||||
|
refreshStatic,
|
||||||
);
|
);
|
||||||
|
|
||||||
// Poll for terminal background color changes to auto-switch theme
|
// Poll for terminal background color changes to auto-switch theme
|
||||||
useTerminalTheme(handleThemeSelect, config, refreshStatic);
|
useTerminalTheme(handleThemeSelect, config, refreshStatic);
|
||||||
|
|
||||||
const {
|
const {
|
||||||
authState,
|
authState,
|
||||||
setAuthState,
|
setAuthState,
|
||||||
|
|||||||
@@ -79,14 +79,12 @@ describe('ThemeDialog Snapshots', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should call refreshStatic when a theme is selected', async () => {
|
it('should call onSelect when a theme is selected', async () => {
|
||||||
const mockRefreshStatic = vi.fn();
|
|
||||||
const settings = createMockSettings();
|
const settings = createMockSettings();
|
||||||
const { stdin } = renderWithProviders(
|
const { stdin } = renderWithProviders(
|
||||||
<ThemeDialog {...baseProps} settings={settings} />,
|
<ThemeDialog {...baseProps} settings={settings} />,
|
||||||
{
|
{
|
||||||
settings,
|
settings,
|
||||||
uiActions: { refreshStatic: mockRefreshStatic },
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -96,7 +94,6 @@ describe('ThemeDialog Snapshots', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(mockRefreshStatic).toHaveBeenCalled();
|
|
||||||
expect(baseProps.onSelect).toHaveBeenCalled();
|
expect(baseProps.onSelect).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -22,7 +22,6 @@ import { getScopeMessageForSetting } from '../../utils/dialogScopeUtils.js';
|
|||||||
import { useKeypress } from '../hooks/useKeypress.js';
|
import { useKeypress } from '../hooks/useKeypress.js';
|
||||||
import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js';
|
import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js';
|
||||||
import { ScopeSelector } from './shared/ScopeSelector.js';
|
import { ScopeSelector } from './shared/ScopeSelector.js';
|
||||||
import { useUIActions } from '../contexts/UIActionsContext.js';
|
|
||||||
import { useUIState } from '../contexts/UIStateContext.js';
|
import { useUIState } from '../contexts/UIStateContext.js';
|
||||||
|
|
||||||
interface ThemeDialogProps {
|
interface ThemeDialogProps {
|
||||||
@@ -85,7 +84,6 @@ export function ThemeDialog({
|
|||||||
terminalWidth,
|
terminalWidth,
|
||||||
}: ThemeDialogProps): React.JSX.Element {
|
}: ThemeDialogProps): React.JSX.Element {
|
||||||
const isAlternateBuffer = useAlternateBuffer();
|
const isAlternateBuffer = useAlternateBuffer();
|
||||||
const { refreshStatic } = useUIActions();
|
|
||||||
const { terminalBackgroundColor } = useUIState();
|
const { terminalBackgroundColor } = useUIState();
|
||||||
const [selectedScope, setSelectedScope] = useState<LoadableSettingScope>(
|
const [selectedScope, setSelectedScope] = useState<LoadableSettingScope>(
|
||||||
SettingScope.User,
|
SettingScope.User,
|
||||||
@@ -142,9 +140,8 @@ export function ThemeDialog({
|
|||||||
const handleThemeSelect = useCallback(
|
const handleThemeSelect = useCallback(
|
||||||
async (themeName: string) => {
|
async (themeName: string) => {
|
||||||
await onSelect(themeName, selectedScope);
|
await onSelect(themeName, selectedScope);
|
||||||
refreshStatic();
|
|
||||||
},
|
},
|
||||||
[onSelect, selectedScope, refreshStatic],
|
[onSelect, selectedScope],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleThemeHighlight = (themeName: string) => {
|
const handleThemeHighlight = (themeName: string) => {
|
||||||
@@ -159,9 +156,8 @@ export function ThemeDialog({
|
|||||||
const handleScopeSelect = useCallback(
|
const handleScopeSelect = useCallback(
|
||||||
async (scope: LoadableSettingScope) => {
|
async (scope: LoadableSettingScope) => {
|
||||||
await onSelect(highlightedThemeName, scope);
|
await onSelect(highlightedThemeName, scope);
|
||||||
refreshStatic();
|
|
||||||
},
|
},
|
||||||
[onSelect, highlightedThemeName, refreshStatic],
|
[onSelect, highlightedThemeName],
|
||||||
);
|
);
|
||||||
|
|
||||||
const [mode, setMode] = useState<'theme' | 'scope'>('theme');
|
const [mode, setMode] = useState<'theme' | 'scope'>('theme');
|
||||||
|
|||||||
@@ -9,8 +9,8 @@ import { useTerminalTheme } from './useTerminalTheme.js';
|
|||||||
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
|
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||||
import { makeFakeConfig, type Config } from '@google/gemini-cli-core';
|
import { makeFakeConfig, type Config } from '@google/gemini-cli-core';
|
||||||
import os from 'node:os';
|
import os from 'node:os';
|
||||||
|
import { themeManager } from '../themes/theme-manager.js';
|
||||||
|
|
||||||
// Mocks
|
|
||||||
const mockWrite = vi.fn();
|
const mockWrite = vi.fn();
|
||||||
const mockSubscribe = vi.fn();
|
const mockSubscribe = vi.fn();
|
||||||
const mockUnsubscribe = vi.fn();
|
const mockUnsubscribe = vi.fn();
|
||||||
@@ -72,9 +72,7 @@ describe('useTerminalTheme', () => {
|
|||||||
config = makeFakeConfig({
|
config = makeFakeConfig({
|
||||||
targetDir: os.tmpdir(),
|
targetDir: os.tmpdir(),
|
||||||
});
|
});
|
||||||
// Set initial background to ensure the hook passes the startup check.
|
|
||||||
config.setTerminalBackground('#000000');
|
config.setTerminalBackground('#000000');
|
||||||
// Spy on future updates.
|
|
||||||
vi.spyOn(config, 'setTerminalBackground');
|
vi.spyOn(config, 'setTerminalBackground');
|
||||||
|
|
||||||
mockWrite.mockClear();
|
mockWrite.mockClear();
|
||||||
@@ -82,7 +80,7 @@ describe('useTerminalTheme', () => {
|
|||||||
mockUnsubscribe.mockClear();
|
mockUnsubscribe.mockClear();
|
||||||
mockHandleThemeSelect.mockClear();
|
mockHandleThemeSelect.mockClear();
|
||||||
mockQueryTerminalBackground.mockClear();
|
mockQueryTerminalBackground.mockClear();
|
||||||
// Reset any settings modifications
|
vi.mocked(themeManager.setTerminalBackground).mockClear();
|
||||||
mockSettings.merged.ui.autoThemeSwitching = true;
|
mockSettings.merged.ui.autoThemeSwitching = true;
|
||||||
mockSettings.merged.ui.theme = 'default';
|
mockSettings.merged.ui.theme = 'default';
|
||||||
});
|
});
|
||||||
@@ -108,7 +106,6 @@ describe('useTerminalTheme', () => {
|
|||||||
it('should poll for terminal background', () => {
|
it('should poll for terminal background', () => {
|
||||||
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
||||||
|
|
||||||
// Fast-forward time (1 minute)
|
|
||||||
vi.advanceTimersByTime(60000);
|
vi.advanceTimersByTime(60000);
|
||||||
expect(mockQueryTerminalBackground).toHaveBeenCalled();
|
expect(mockQueryTerminalBackground).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
@@ -117,20 +114,23 @@ describe('useTerminalTheme', () => {
|
|||||||
config.getTerminalBackground = vi.fn().mockReturnValue(undefined);
|
config.getTerminalBackground = vi.fn().mockReturnValue(undefined);
|
||||||
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
||||||
|
|
||||||
// Poll should not happen
|
|
||||||
vi.advanceTimersByTime(60000);
|
vi.advanceTimersByTime(60000);
|
||||||
expect(mockQueryTerminalBackground).not.toHaveBeenCalled();
|
expect(mockQueryTerminalBackground).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should switch to light theme when background is light', () => {
|
it('should switch to light theme when background is light and not call refreshStatic directly', () => {
|
||||||
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
const refreshStatic = vi.fn();
|
||||||
|
renderHook(() =>
|
||||||
|
useTerminalTheme(mockHandleThemeSelect, config, refreshStatic),
|
||||||
|
);
|
||||||
|
|
||||||
const handler = mockSubscribe.mock.calls[0][0];
|
const handler = mockSubscribe.mock.calls[0][0];
|
||||||
|
|
||||||
// Simulate light background response (white)
|
|
||||||
handler('rgb:ffff/ffff/ffff');
|
handler('rgb:ffff/ffff/ffff');
|
||||||
|
|
||||||
expect(config.setTerminalBackground).toHaveBeenCalledWith('#ffffff');
|
expect(config.setTerminalBackground).toHaveBeenCalledWith('#ffffff');
|
||||||
|
expect(themeManager.setTerminalBackground).toHaveBeenCalledWith('#ffffff');
|
||||||
|
expect(refreshStatic).not.toHaveBeenCalled();
|
||||||
expect(mockHandleThemeSelect).toHaveBeenCalledWith(
|
expect(mockHandleThemeSelect).toHaveBeenCalledWith(
|
||||||
'default-light',
|
'default-light',
|
||||||
expect.anything(),
|
expect.anything(),
|
||||||
@@ -138,31 +138,51 @@ describe('useTerminalTheme', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('should switch to dark theme when background is dark', () => {
|
it('should switch to dark theme when background is dark', () => {
|
||||||
// Start with light theme
|
|
||||||
mockSettings.merged.ui.theme = 'default-light';
|
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];
|
const handler = mockSubscribe.mock.calls[0][0];
|
||||||
|
|
||||||
// Simulate dark background response (black)
|
|
||||||
handler('rgb:0000/0000/0000');
|
handler('rgb:0000/0000/0000');
|
||||||
|
|
||||||
expect(config.setTerminalBackground).toHaveBeenCalledWith('#000000');
|
expect(config.setTerminalBackground).toHaveBeenCalledWith('#000000');
|
||||||
|
expect(themeManager.setTerminalBackground).toHaveBeenCalledWith('#000000');
|
||||||
|
expect(refreshStatic).not.toHaveBeenCalled();
|
||||||
expect(mockHandleThemeSelect).toHaveBeenCalledWith(
|
expect(mockHandleThemeSelect).toHaveBeenCalledWith(
|
||||||
'default',
|
'default',
|
||||||
expect.anything(),
|
expect.anything(),
|
||||||
);
|
);
|
||||||
|
|
||||||
// Reset theme
|
|
||||||
mockSettings.merged.ui.theme = 'default';
|
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', () => {
|
it('should not switch theme if autoThemeSwitching is disabled', () => {
|
||||||
mockSettings.merged.ui.autoThemeSwitching = false;
|
mockSettings.merged.ui.autoThemeSwitching = false;
|
||||||
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
renderHook(() => useTerminalTheme(mockHandleThemeSelect, config, vi.fn()));
|
||||||
|
|
||||||
// Poll should not happen
|
|
||||||
vi.advanceTimersByTime(60000);
|
vi.advanceTimersByTime(60000);
|
||||||
expect(mockQueryTerminalBackground).not.toHaveBeenCalled();
|
expect(mockQueryTerminalBackground).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
|||||||
@@ -56,11 +56,18 @@ export function useTerminalTheme(
|
|||||||
if (!match) return;
|
if (!match) return;
|
||||||
|
|
||||||
const hexColor = parseColor(match[1], match[2], match[3]);
|
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);
|
config.setTerminalBackground(hexColor);
|
||||||
themeManager.setTerminalBackground(hexColor);
|
themeManager.setTerminalBackground(hexColor);
|
||||||
refreshStatic();
|
|
||||||
|
|
||||||
|
const luminance = getLuminance(hexColor);
|
||||||
const currentThemeName = settings.merged.ui.theme;
|
const currentThemeName = settings.merged.ui.theme;
|
||||||
|
|
||||||
const newTheme = shouldSwitchTheme(
|
const newTheme = shouldSwitchTheme(
|
||||||
@@ -72,6 +79,11 @@ export function useTerminalTheme(
|
|||||||
|
|
||||||
if (newTheme) {
|
if (newTheme) {
|
||||||
void handleThemeSelect(newTheme, SettingScope.User);
|
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();
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ export const useThemeCommand = (
|
|||||||
setThemeError: (error: string | null) => void,
|
setThemeError: (error: string | null) => void,
|
||||||
addItem: UseHistoryManagerReturn['addItem'],
|
addItem: UseHistoryManagerReturn['addItem'],
|
||||||
initialThemeError: string | null,
|
initialThemeError: string | null,
|
||||||
|
refreshStatic: () => void,
|
||||||
): UseThemeCommandReturn => {
|
): UseThemeCommandReturn => {
|
||||||
const [isThemeDialogOpen, setIsThemeDialogOpen] =
|
const [isThemeDialogOpen, setIsThemeDialogOpen] =
|
||||||
useState(!!initialThemeError);
|
useState(!!initialThemeError);
|
||||||
@@ -102,12 +103,13 @@ export const useThemeCommand = (
|
|||||||
themeManager.loadCustomThemes(loadedSettings.merged.ui.customThemes);
|
themeManager.loadCustomThemes(loadedSettings.merged.ui.customThemes);
|
||||||
}
|
}
|
||||||
applyTheme(loadedSettings.merged.ui.theme); // Apply the current theme
|
applyTheme(loadedSettings.merged.ui.theme); // Apply the current theme
|
||||||
|
refreshStatic();
|
||||||
setThemeError(null);
|
setThemeError(null);
|
||||||
} finally {
|
} finally {
|
||||||
setIsThemeDialogOpen(false); // Close the dialog
|
setIsThemeDialogOpen(false); // Close the dialog
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[applyTheme, loadedSettings, setThemeError],
|
[applyTheme, loadedSettings, refreshStatic, setThemeError],
|
||||||
);
|
);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user