diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 2e53670c31..3b15e6066f 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -83,19 +83,6 @@ describe('SettingsSchema', () => { ).toBe('boolean'); }); - it('should have showTips property', () => { - const showTips = getSettingsSchema().ui?.properties?.showTips; - expect(showTips).toBeDefined(); - expect(showTips?.type).toBe('boolean'); - }); - - it('should have showWit property', () => { - const showWit = getSettingsSchema().ui?.properties?.showWit; - expect(showWit).toBeDefined(); - expect(showWit?.type).toBe('boolean'); - expect(showWit?.default).toBe(true); - }); - it('should have errorVerbosity enum property', () => { const definition = getSettingsSchema().ui?.properties?.errorVerbosity; expect(definition).toBeDefined(); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 989da8d9b1..38b71e433f 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -339,7 +339,7 @@ const SETTINGS_SCHEMA = { label: 'Enable Session Cleanup', category: 'General', requiresRestart: false, - default: false, + default: true as boolean, description: 'Enable automatic session cleanup', showInDialog: true, }, @@ -348,7 +348,7 @@ const SETTINGS_SCHEMA = { label: 'Keep chat history', category: 'General', requiresRestart: false, - default: undefined as string | undefined, + default: '30d' as string, description: 'Automatically delete chats older than this time period (e.g., "30d", "7d", "24h", "1w")', showInDialog: true, @@ -372,16 +372,6 @@ const SETTINGS_SCHEMA = { description: `Minimum retention period (safety limit, defaults to "${DEFAULT_MIN_RETENTION}")`, showInDialog: false, }, - warningAcknowledged: { - type: 'boolean', - label: 'Warning Acknowledged', - category: 'General', - requiresRestart: false, - default: false, - showInDialog: false, - description: - 'INTERNAL: Whether the user has acknowledged the session retention warning', - }, }, description: 'Settings for automatic session cleanup.', }, @@ -619,50 +609,6 @@ const SETTINGS_SCHEMA = { description: 'Hide the footer from the UI', showInDialog: true, }, - collapseDrawerDuringApproval: { - type: 'boolean', - label: 'Collapse Drawer During Approval', - category: 'UI', - requiresRestart: false, - default: true, - description: - 'Collapse the entire drawer (status, context, input, footer) when a tool approval request is displayed.', - showInDialog: true, - }, - newFooterLayout: { - type: 'enum', - label: 'New Footer Layout', - category: 'UI', - requiresRestart: false, - default: 'legacy', - description: 'Use the new 2-row layout with inline tips.', - showInDialog: true, - options: [ - { value: 'legacy', label: 'Legacy' }, - { value: 'new', label: 'New Layout' }, - { value: 'new_divider_down', label: 'New Layout (Divider Down)' }, - ], - }, - showTips: { - type: 'boolean', - label: 'Show Tips', - category: 'UI', - requiresRestart: false, - default: true, - description: - 'Show informative tips on the right side of the status line.', - showInDialog: true, - }, - showWit: { - type: 'boolean', - label: 'Show Witty Phrases', - category: 'UI', - requiresRestart: false, - default: true, - description: 'Show witty phrases while waiting.', - showInDialog: true, - }, - showMemoryUsage: { type: 'boolean', label: 'Show Memory Usage', @@ -747,6 +693,22 @@ const SETTINGS_SCHEMA = { description: 'Show the spinner during operations.', showInDialog: true, }, + loadingPhrases: { + type: 'enum', + label: 'Loading Phrases', + category: 'UI', + requiresRestart: false, + default: 'tips', + description: + 'What to show while the model is working: tips, witty comments, both, or nothing.', + showInDialog: true, + options: [ + { value: 'tips', label: 'Tips' }, + { value: 'witty', label: 'Witty' }, + { value: 'all', label: 'All' }, + { value: 'off', label: 'Off' }, + ], + }, errorVerbosity: { type: 'enum', label: 'Error Verbosity', diff --git a/packages/cli/src/test-utils/AppRig.tsx b/packages/cli/src/test-utils/AppRig.tsx index 55df39e1c7..3ff65c4067 100644 --- a/packages/cli/src/test-utils/AppRig.tsx +++ b/packages/cli/src/test-utils/AppRig.tsx @@ -264,9 +264,6 @@ export class AppRig { enabled: false, hasSeenNudge: true, }, - ui: { - collapseDrawerDuringApproval: false, - }, }, }); } diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 8e84c4c267..9cc65024d0 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -2063,8 +2063,9 @@ Logging in with Google... Restarting Gemini CLI to continue. !!emptyWalletRequest || !!customDialog; - const newLayoutSetting = settings.merged.ui.newFooterLayout; - const isExperimentalLayout = newLayoutSetting !== 'legacy'; + const loadingPhrases = settings.merged.ui.loadingPhrases; + + const isExperimentalLayout = true; const showLoadingIndicator = (!embeddedShellFocused || isBackgroundShellVisible) && streamingState === StreamingState.Responding && @@ -2102,8 +2103,7 @@ Logging in with Google... Restarting Gemini CLI to continue. streamingState, shouldShowFocusHint, retryStatus, - showTips: settings.merged.ui.showTips, - showWit: settings.merged.ui.showWit, + loadingPhrases, customWittyPhrases: settings.merged.ui.customWittyPhrases, errorVerbosity: settings.merged.ui.errorVerbosity, maxLength, diff --git a/packages/cli/src/ui/components/Composer.test.tsx b/packages/cli/src/ui/components/Composer.test.tsx index 3ecc9ba8c5..797203d7cb 100644 --- a/packages/cli/src/ui/components/Composer.test.tsx +++ b/packages/cli/src/ui/components/Composer.test.tsx @@ -250,7 +250,7 @@ const createMockConfig = (overrides = {}): Config => const renderComposer = async ( uiState: UIState, - settings = createMockSettings(), + settings = createMockSettings({ ui: { useLegacyLayout: true } }), config = createMockConfig(), uiActions = createMockUIActions(), ) => { @@ -381,7 +381,7 @@ describe('Composer', () => { }, }); const settings = createMockSettings({ - ui: { inlineThinkingMode: 'full' }, + ui: { inlineThinkingMode: 'full', useLegacyLayout: true }, }); const { lastFrame } = await renderComposer(uiState, settings); @@ -410,7 +410,7 @@ describe('Composer', () => { thought: { subject: 'Hidden', description: 'Should not show' }, }); const settings = createMockSettings({ - merged: { ui: { loadingPhraseLayout: 'none' } }, + merged: { ui: { loadingPhraseLayout: 'none', useLegacyLayout: true } }, }); const { lastFrame } = await renderComposer(uiState, settings); @@ -587,15 +587,16 @@ describe('Composer', () => { const uiState = createMockUIState({ cleanUiDetailsVisible: false, }); + const settings = createMockSettings({ + ui: { useLegacyLayout: true, showShortcutsHint: false }, + }); - const { lastFrame } = await renderComposer(uiState); + const { lastFrame } = await renderComposer(uiState, settings); const output = lastFrame(); - expect(output).toContain('ShortcutsHint'); + expect(output).not.toContain('ShortcutsHint'); expect(output).toContain('InputPrompt'); expect(output).not.toContain('Footer'); - expect(output).not.toContain('ApprovalModeIndicator'); - expect(output).not.toContain('ContextSummaryDisplay'); }); it('renders InputPrompt when input is active', async () => { @@ -744,6 +745,7 @@ describe('Composer', () => { const settings = createMockSettings({ ui: { footer: { hideContextPercentage: false }, + useLegacyLayout: true, }, }); @@ -846,6 +848,7 @@ describe('Composer', () => { const settings = createMockSettings({ ui: { showShortcutsHint: false, + useLegacyLayout: true, }, }); diff --git a/packages/cli/src/ui/components/Composer.tsx b/packages/cli/src/ui/components/Composer.tsx index bb9b504bfc..7a9e9d74e2 100644 --- a/packages/cli/src/ui/components/Composer.tsx +++ b/packages/cli/src/ui/components/Composer.tsx @@ -59,10 +59,14 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { const isAlternateBuffer = useAlternateBuffer(); const { showApprovalModeIndicator } = uiState; - const newLayoutSetting = settings.merged.ui.newFooterLayout; - const { showTips, showWit } = settings.merged.ui; + const loadingPhrases = settings.merged.ui.loadingPhrases; + const showTips = loadingPhrases === 'tips' || loadingPhrases === 'all'; + const showWit = loadingPhrases === 'witty' || loadingPhrases === 'all'; - const isExperimentalLayout = newLayoutSetting !== 'legacy'; + // For this PR we are hardcoding the new experimental layout as the default. + // We allow a hidden setting to override it specifically for existing tests. + const isExperimentalLayout = + (settings.merged.ui as Record)['useLegacyLayout'] !== true; const showUiDetails = uiState.cleanUiDetailsVisible; const suggestionsPosition = isAlternateBuffer ? 'above' : 'below'; const hideContextSummary = @@ -181,10 +185,14 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { return () => clearTimeout(timeout); }, [canShowShortcutsHint]); - if ( - hasPendingActionRequired && - settings.merged.ui.collapseDrawerDuringApproval - ) { + // Use the setting if provided, otherwise default to true for the new UX. + // This allows tests to override the collapse behavior. + const shouldCollapseDuringApproval = + (settings.merged.ui as Record)[ + 'collapseDrawerDuringApproval' + ] !== false; + + if (hasPendingActionRequired && shouldCollapseDuringApproval) { return null; } @@ -632,10 +640,6 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { )} - {showUiDetails && newLayoutSetting === 'new_divider_down' && ( - - )} - {showUiDetails && ( = ({ return |⌐■_■|; } - if ( - settings.merged.ui.newFooterLayout === 'legacy' && - uiState.activeHooks.length > 0 && - settings.merged.hooksConfig.notifications - ) { - return ; - } - if (!settings.merged.ui.hideContextSummary && !hideContextSummary) { return ( { initialStreamingState: StreamingState, initialShouldShowFocusHint: boolean = false, initialRetryStatus: RetryAttemptPayload | null = null, - showTips: boolean = true, - showWit: boolean = true, + loadingPhrases: 'tips' | 'witty' | 'all' | 'off' = 'all', initialErrorVerbosity: 'low' | 'full' = 'full', ) => { let hookResult: ReturnType; @@ -42,23 +41,20 @@ describe('useLoadingIndicator', () => { streamingState, shouldShowFocusHint, retryStatus, - showTips, - showWit, + loadingPhrases, errorVerbosity, }: { streamingState: StreamingState; shouldShowFocusHint?: boolean; retryStatus?: RetryAttemptPayload | null; - showTips?: boolean; - showWit?: boolean; + loadingPhrases?: 'tips' | 'witty' | 'all' | 'off'; errorVerbosity?: 'low' | 'full'; }) { hookResult = useLoadingIndicator({ streamingState, shouldShowFocusHint: !!shouldShowFocusHint, retryStatus: retryStatus || null, - showTips, - showWit, + loadingPhrases, errorVerbosity, }); return null; @@ -68,8 +64,7 @@ describe('useLoadingIndicator', () => { streamingState={initialStreamingState} shouldShowFocusHint={initialShouldShowFocusHint} retryStatus={initialRetryStatus} - showTips={showTips} - showWit={showWit} + loadingPhrases={loadingPhrases} errorVerbosity={initialErrorVerbosity} />, ); @@ -83,14 +78,12 @@ describe('useLoadingIndicator', () => { streamingState: StreamingState; shouldShowFocusHint?: boolean; retryStatus?: RetryAttemptPayload | null; - showTips?: boolean; - showWit?: boolean; + loadingPhrases?: 'tips' | 'witty' | 'all' | 'off'; errorVerbosity?: 'low' | 'full'; }) => rerender( , diff --git a/packages/cli/src/ui/hooks/useLoadingIndicator.ts b/packages/cli/src/ui/hooks/useLoadingIndicator.ts index e3c53cb59c..3af71771c8 100644 --- a/packages/cli/src/ui/hooks/useLoadingIndicator.ts +++ b/packages/cli/src/ui/hooks/useLoadingIndicator.ts @@ -19,8 +19,7 @@ export interface UseLoadingIndicatorProps { streamingState: StreamingState; shouldShowFocusHint: boolean; retryStatus: RetryAttemptPayload | null; - showTips?: boolean; - showWit?: boolean; + loadingPhrases?: 'tips' | 'witty' | 'all' | 'off'; customWittyPhrases?: string[]; errorVerbosity?: 'low' | 'full'; maxLength?: number; @@ -30,8 +29,7 @@ export const useLoadingIndicator = ({ streamingState, shouldShowFocusHint, retryStatus, - showTips = true, - showWit = true, + loadingPhrases = 'tips', customWittyPhrases, errorVerbosity = 'full', maxLength, @@ -43,6 +41,10 @@ export const useLoadingIndicator = ({ const isPhraseCyclingActive = streamingState === StreamingState.Responding; const isWaiting = streamingState === StreamingState.WaitingForConfirmation; + + const showTips = loadingPhrases === 'tips' || loadingPhrases === 'all'; + const showWit = loadingPhrases === 'witty' || loadingPhrases === 'all'; + const { currentTip, currentWittyPhrase } = usePhraseCycler( isPhraseCyclingActive, isWaiting, diff --git a/packages/cli/src/ui/hooks/useSessionRetentionCheck.test.ts b/packages/cli/src/ui/hooks/useSessionRetentionCheck.test.ts index 67e5efbc6b..df1b8ae973 100644 --- a/packages/cli/src/ui/hooks/useSessionRetentionCheck.test.ts +++ b/packages/cli/src/ui/hooks/useSessionRetentionCheck.test.ts @@ -40,59 +40,12 @@ describe('useSessionRetentionCheck', () => { vi.restoreAllMocks(); }); - it('should show warning if enabled is true but maxAge is undefined', async () => { - const settings = { - general: { - sessionRetention: { - enabled: true, - maxAge: undefined, - warningAcknowledged: false, - }, - }, - } as unknown as Settings; - - mockGetAllSessionFiles.mockResolvedValue(['session1.json']); - mockIdentifySessionsToDelete.mockResolvedValue(['session1.json']); - - const { result } = renderHook(() => - useSessionRetentionCheck(mockConfig, settings), - ); - - await waitFor(() => { - expect(result.current.checkComplete).toBe(true); - expect(result.current.shouldShowWarning).toBe(true); - expect(mockGetAllSessionFiles).toHaveBeenCalled(); - expect(mockIdentifySessionsToDelete).toHaveBeenCalled(); - }); - }); - - it('should not show warning if warningAcknowledged is true', async () => { - const settings = { - general: { - sessionRetention: { - warningAcknowledged: true, - }, - }, - } as unknown as Settings; - - const { result } = renderHook(() => - useSessionRetentionCheck(mockConfig, settings), - ); - - await waitFor(() => { - expect(result.current.checkComplete).toBe(true); - expect(result.current.shouldShowWarning).toBe(false); - expect(mockGetAllSessionFiles).not.toHaveBeenCalled(); - expect(mockIdentifySessionsToDelete).not.toHaveBeenCalled(); - }); - }); - it('should not show warning if retention is already enabled', async () => { const settings = { general: { sessionRetention: { enabled: true, - maxAge: '30d', // Explicitly enabled with non-default + maxAge: '30d', // Explicitly enabled }, }, } as unknown as Settings; @@ -114,7 +67,6 @@ describe('useSessionRetentionCheck', () => { general: { sessionRetention: { enabled: false, - warningAcknowledged: false, }, }, } as unknown as Settings; @@ -143,7 +95,6 @@ describe('useSessionRetentionCheck', () => { general: { sessionRetention: { enabled: false, - warningAcknowledged: false, }, }, } as unknown as Settings; @@ -169,7 +120,6 @@ describe('useSessionRetentionCheck', () => { general: { sessionRetention: { enabled: false, - warningAcknowledged: false, }, }, } as unknown as Settings; @@ -198,7 +148,6 @@ describe('useSessionRetentionCheck', () => { general: { sessionRetention: { enabled: false, - warningAcknowledged: false, }, }, } as unknown as Settings; diff --git a/packages/cli/src/ui/hooks/useSessionRetentionCheck.ts b/packages/cli/src/ui/hooks/useSessionRetentionCheck.ts index 99b443cffc..da370b23e0 100644 --- a/packages/cli/src/ui/hooks/useSessionRetentionCheck.ts +++ b/packages/cli/src/ui/hooks/useSessionRetentionCheck.ts @@ -10,7 +10,6 @@ import { type Settings } from '../../config/settings.js'; import { getAllSessionFiles } from '../../utils/sessionUtils.js'; import { identifySessionsToDelete } from '../../utils/sessionCleanup.js'; import path from 'node:path'; - export function useSessionRetentionCheck( config: Config, settings: Settings, @@ -21,11 +20,10 @@ export function useSessionRetentionCheck( const [checkComplete, setCheckComplete] = useState(false); useEffect(() => { - // If warning already acknowledged or retention already enabled, skip check + // If retention already enabled, skip check if ( - settings.general?.sessionRetention?.warningAcknowledged || - (settings.general?.sessionRetention?.enabled && - settings.general?.sessionRetention?.maxAge !== undefined) + settings.general?.sessionRetention?.enabled && + settings.general?.sessionRetention?.maxAge !== undefined ) { setShouldShowWarning(false); setCheckComplete(true);