Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/cli/src/ui/AppContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -609,11 +609,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,
Expand Down
5 changes: 1 addition & 4 deletions packages/cli/src/ui/components/ThemeDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<ThemeDialog {...baseProps} settings={settings} />,
{
settings,
uiActions: { refreshStatic: mockRefreshStatic },
},
);

Expand All @@ -96,7 +94,6 @@ describe('ThemeDialog Snapshots', () => {
});

await waitFor(() => {
expect(mockRefreshStatic).toHaveBeenCalled();
expect(baseProps.onSelect).toHaveBeenCalled();
});
});
Expand Down
8 changes: 2 additions & 6 deletions packages/cli/src/ui/components/ThemeDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<LoadableSettingScope>(
SettingScope.User,
Expand Down Expand Up @@ -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) => {
Expand All @@ -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');
Expand Down
48 changes: 34 additions & 14 deletions packages/cli/src/ui/hooks/useTerminalTheme.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -72,17 +72,15 @@ 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();
mockSubscribe.mockClear();
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';
});
Expand All @@ -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();
});
Expand All @@ -117,52 +114,75 @@ 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(),
);
});

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();

Expand Down
16 changes: 14 additions & 2 deletions packages/cli/src/ui/hooks/useTerminalTheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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();
}
};

Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/ui/hooks/useThemeCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
Loading