diff --git a/src/vs/platform/theme/electron-main/themeMainService.ts b/src/vs/platform/theme/electron-main/themeMainService.ts index 94c46d84185..c2146132485 100644 --- a/src/vs/platform/theme/electron-main/themeMainService.ts +++ b/src/vs/platform/theme/electron-main/themeMainService.ts @@ -24,11 +24,5 @@ export interface IThemeMainService { getColorScheme(): IColorScheme; - /** - * Whether OS color-scheme auto-detection is active. - * Returns `true` when the `window.autoDetectColorScheme` setting is enabled, - * or for fresh installs where no theme has been stored yet and the user - * has not explicitly configured the setting (e.g. via settings sync). - */ isAutoDetectColorScheme(): boolean; } diff --git a/src/vs/platform/theme/electron-main/themeMainServiceImpl.ts b/src/vs/platform/theme/electron-main/themeMainServiceImpl.ts index 56c103875a6..9e9258c04e8 100644 --- a/src/vs/platform/theme/electron-main/themeMainServiceImpl.ts +++ b/src/vs/platform/theme/electron-main/themeMainServiceImpl.ts @@ -185,12 +185,6 @@ export class ThemeMainService extends Disposable implements IThemeMainService { if (Setting.DETECT_COLOR_SCHEME.getValue(this.configurationService)) { return true; } - // For new installs with no stored theme, auto-detect OS color scheme by default - // unless the user has explicitly configured the setting (e.g. via settings sync) - if (!this.stateService.getItem(THEME_STORAGE_KEY)) { - const { userValue } = this.configurationService.inspect(Setting.DETECT_COLOR_SCHEME.key); - return userValue === undefined; - } return false; } diff --git a/src/vs/platform/windows/electron-main/windowsMainService.ts b/src/vs/platform/windows/electron-main/windowsMainService.ts index 24a1af67f0f..bf6622eb54d 100644 --- a/src/vs/platform/windows/electron-main/windowsMainService.ts +++ b/src/vs/platform/windows/electron-main/windowsMainService.ts @@ -1573,7 +1573,7 @@ export class WindowsMainService extends Disposable implements IWindowsMainServic os: { release: release(), hostname: hostname(), arch: arch() }, autoDetectHighContrast: windowConfig?.autoDetectHighContrast ?? true, - autoDetectColorScheme: windowConfig?.autoDetectColorScheme ?? this.themeMainService.isAutoDetectColorScheme(), + autoDetectColorScheme: windowConfig?.autoDetectColorScheme ?? false, accessibilitySupport: app.accessibilitySupportEnabled, colorScheme: this.themeMainService.getColorScheme(), policiesData: this.policyService.serialize(), diff --git a/src/vs/workbench/services/themes/browser/workbenchThemeService.ts b/src/vs/workbench/services/themes/browser/workbenchThemeService.ts index 20d45050cfa..797fc544193 100644 --- a/src/vs/workbench/services/themes/browser/workbenchThemeService.ts +++ b/src/vs/workbench/services/themes/browser/workbenchThemeService.ts @@ -44,7 +44,8 @@ import { ILanguageService } from '../../../../editor/common/languages/language.j import { mainWindow } from '../../../../base/browser/window.js'; import { generateColorThemeCSS } from './colorThemeCss.js'; import { INotificationService, Severity } from '../../../../platform/notification/common/notification.js'; -import { ICommandService } from '../../../../platform/commands/common/commands.js'; +import { IHostService } from '../../host/browser/host.js'; +import { toAction } from '../../../../base/common/actions.js'; // implementation @@ -114,12 +115,11 @@ export class WorkbenchThemeService extends Disposable implements IWorkbenchTheme @IUserDataInitializationService private readonly userDataInitializationService: IUserDataInitializationService, @ILanguageService private readonly languageService: ILanguageService, @INotificationService private readonly notificationService: INotificationService, - @ICommandService private readonly commandService: ICommandService + @IHostService private readonly hostService: IHostService ) { super(); this.container = layoutService.mainContainer; - const isNewUser = this.storageService.isNew(StorageScope.APPLICATION); - this.settings = new ThemeConfiguration(configurationService, hostColorService, isNewUser); + this.settings = new ThemeConfiguration(configurationService, hostColorService); this.colorThemeRegistry = this._register(new ThemeRegistry(colorThemesExtPoint, ColorThemeData.fromExtensionTheme)); this.colorThemeWatcher = this._register(new ThemeFileWatcher(fileService, environmentService, this.reloadCurrentColorTheme.bind(this))); @@ -146,6 +146,7 @@ export class WorkbenchThemeService extends Disposable implements IWorkbenchTheme // themes are loaded asynchronously, we need to initialize // a color theme document with good defaults until the theme is loaded let themeData: ColorThemeData | undefined = ColorThemeData.fromStorageData(this.storageService); + const previousColorThemeSetting = themeData?.settingsId; const colorThemeSetting = this.settings.colorTheme; if (themeData && colorThemeSetting !== themeData.settingsId) { themeData = undefined; @@ -179,7 +180,7 @@ export class WorkbenchThemeService extends Disposable implements IWorkbenchTheme this.installConfigurationListener(); this.installPreferredSchemeListener(); this.installRegistryListeners(); - this.initialize().catch(errors.onUnexpectedError); + this.initialize(previousColorThemeSetting).catch(errors.onUnexpectedError); }); const codiconStyleSheet = createStyleSheet(); @@ -195,7 +196,7 @@ export class WorkbenchThemeService extends Disposable implements IWorkbenchTheme delayer.schedule(); } - private async initialize(): Promise<[IWorkbenchColorTheme | null, IWorkbenchFileIconTheme | null, IWorkbenchProductIconTheme | null]> { + private async initialize(themePreviousSettingsId: string | undefined): Promise<[IWorkbenchColorTheme | null, IWorkbenchFileIconTheme | null, IWorkbenchProductIconTheme | null]> { const extDevLocs = this.environmentService.extensionDevelopmentLocationURI; const extDevLoc = extDevLocs && extDevLocs.length === 1 ? extDevLocs[0] : undefined; // in dev mode, switch to a theme provided by the extension under dev. @@ -246,34 +247,64 @@ export class WorkbenchThemeService extends Disposable implements IWorkbenchTheme this.migrateColorThemeSettings(); - await this.migrateAutoDetectColorScheme(); const result = await Promise.all([initializeColorTheme(), initializeFileIconTheme(), initializeProductIconTheme()]); - this.showNewDefaultThemeNotification(); + await this.showNewDefaultThemeNotification(themePreviousSettingsId); return result; } private static readonly NEW_THEME_NOTIFICATION_KEY = 'workbench.newDefaultThemeNotification'; - private showNewDefaultThemeNotification(): void { - const newDefaultThemes = new Set([ThemeSettingDefaults.COLOR_THEME_DARK, ThemeSettingDefaults.COLOR_THEME_LIGHT]); - if (newDefaultThemes.has(this.currentColorTheme.settingsId)) { - return; // already using a new default theme - } + private async showNewDefaultThemeNotification(previousSettingsId: string | undefined): Promise { if (this.storageService.getBoolean(WorkbenchThemeService.NEW_THEME_NOTIFICATION_KEY, StorageScope.APPLICATION)) { return; // already shown } - - const handle = this.notificationService.prompt( - Severity.Info, - nls.localize('newDefaultTheme', "New default themes are available for VS Code."), - [{ - label: nls.localize('tryNewTheme', "Try Them Out"), - run: () => this.commandService.executeCommand('workbench.action.tryNewDefaultThemes') - }] - ); - this._register(Event.once(handle.onDidClose)(() => { + if (!(await this.hostService.hadLastFocus()) || this.environmentService.isSessionsWindow) { + return; + } + try { + if (!this.settings.isDefaultColorTheme() || !previousSettingsId) { + return; + } + previousSettingsId = migrateThemeSettingsId(previousSettingsId); + if (!['Dark Modern', 'Light Modern'].includes(previousSettingsId)) { + return; + } + if (![ThemeSettingDefaults.COLOR_THEME_DARK, ThemeSettingDefaults.COLOR_THEME_LIGHT].includes(this.settings.colorTheme)) { + return; + } + } finally { + // remeber to not show the dialog again this.storageService.store(WorkbenchThemeService.NEW_THEME_NOTIFICATION_KEY, true, StorageScope.APPLICATION, StorageTarget.USER); - })); + } + + const keepTheme = await new Promise(resolve => { + this.notificationService.prompt( + Severity.Info, + nls.localize({ key: 'themeUpdatedNotification', comment: ['{0} is the name of the new default theme'] }, "VS Code has a new default theme: '{0}'.", this.getColorTheme().label), + [ + toAction({ + id: 'themeUpdated.tryItOut', + label: nls.localize('tryNewTheme', "Keep It"), + run: () => resolve(true) + }), + toAction({ + id: 'themeUpdated.noThanks', + label: nls.localize('noThanks', "No Thanks"), + run: () => resolve(false) + }) + ], + { + onCancel: () => resolve(false) + } + ); + }); + + if (!keepTheme) { + const previousTheme = this.colorThemeRegistry.findThemeBySettingsId(previousSettingsId); + if (previousTheme) { + this.setColorTheme(previousTheme.id, 'auto'); + } + } } /** @@ -305,29 +336,6 @@ export class WorkbenchThemeService extends Disposable implements IWorkbenchTheme } } - /** - * For new users who haven't explicitly configured `window.autoDetectColorScheme`, - * persist `true` so that auto-detect becomes the default going forward. - */ - private async migrateAutoDetectColorScheme(): Promise { - if (!this.storageService.isNew(StorageScope.APPLICATION)) { - return; - } - - // Ensure that user data (including synced settings) has finished initializing - // so we do not overwrite values that arrive via settings sync. - await this.userDataInitializationService.whenInitializationFinished(); - - const inspection = this.configurationService.inspect(ThemeSettings.DETECT_COLOR_SCHEME); - - // Treat any of userValue, userLocalValue, or userRemoteValue as an explicit configuration. - if (inspection.userValue === undefined - && inspection.userLocalValue === undefined - && inspection.userRemoteValue === undefined) { - await this.configurationService.updateValue(ThemeSettings.DETECT_COLOR_SCHEME, true, ConfigurationTarget.USER); - } - } - private installConfigurationListener() { this._register(this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration(ThemeSettings.COLOR_THEME) diff --git a/src/vs/workbench/services/themes/common/themeConfiguration.ts b/src/vs/workbench/services/themes/common/themeConfiguration.ts index e364109cdae..42b11f4f48f 100644 --- a/src/vs/workbench/services/themes/common/themeConfiguration.ts +++ b/src/vs/workbench/services/themes/common/themeConfiguration.ts @@ -282,19 +282,7 @@ const colorSchemeToPreferred = { }; export class ThemeConfiguration { - constructor(private configurationService: IConfigurationService, private hostColorService: IHostColorSchemeService, private readonly isNewUser: boolean = false) { - } - - private shouldAutoDetectColorScheme(): boolean { - const { value, userValue, userLocalValue, userRemoteValue } = this.configurationService.inspect(ThemeSettings.DETECT_COLOR_SCHEME); - if (value) { - return true; - } - if (this.isNewUser) { - const hasUserScopedValue = userValue !== undefined || userLocalValue !== undefined || userRemoteValue !== undefined; - return !hasUserScopedValue; - } - return false; + constructor(private configurationService: IConfigurationService, private hostColorService: IHostColorSchemeService) { } public get colorTheme(): string { @@ -348,14 +336,14 @@ export class ThemeConfiguration { if (this.configurationService.getValue(ThemeSettings.DETECT_HC) && this.hostColorService.highContrast) { return this.hostColorService.dark ? ColorScheme.HIGH_CONTRAST_DARK : ColorScheme.HIGH_CONTRAST_LIGHT; } - if (this.shouldAutoDetectColorScheme()) { + if (this.isDetectingColorScheme()) { return this.hostColorService.dark ? ColorScheme.DARK : ColorScheme.LIGHT; } return undefined; } public isDetectingColorScheme(): boolean { - return this.shouldAutoDetectColorScheme(); + return this.configurationService.getValue(ThemeSettings.DETECT_COLOR_SCHEME); } public getColorThemeSettingId(): ThemeSettings { diff --git a/src/vs/workbench/services/themes/test/common/workbenchThemeService.test.ts b/src/vs/workbench/services/themes/test/common/workbenchThemeService.test.ts index 2843b3ff46d..07b324d8f56 100644 --- a/src/vs/workbench/services/themes/test/common/workbenchThemeService.test.ts +++ b/src/vs/workbench/services/themes/test/common/workbenchThemeService.test.ts @@ -6,12 +6,6 @@ import assert from 'assert'; import { migrateThemeSettingsId, ThemeSettingDefaults } from '../../common/workbenchThemeService.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { ThemeConfiguration } from '../../common/themeConfiguration.js'; -import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { IHostColorSchemeService } from '../../common/hostColorSchemeService.js'; -import { ColorScheme } from '../../../../../platform/theme/common/theme.js'; -import { Event } from '../../../../../base/common/event.js'; -import { IConfigurationOverrides, IConfigurationValue } from '../../../../../platform/configuration/common/configuration.js'; suite('WorkbenchThemeService', () => { @@ -40,124 +34,4 @@ suite('WorkbenchThemeService', () => { ); }); }); - - suite('ThemeConfiguration', () => { - - function createHostColorSchemeService(dark: boolean, highContrast: boolean = false): IHostColorSchemeService { - return { - _serviceBrand: undefined, - dark, - highContrast, - onDidChangeColorScheme: Event.None, - }; - } - - /** - * Creates a config service that separates the resolved value from the user value, - * matching production behaviour where getValue() returns the schema default - * while inspect().userValue is undefined when no explicit user value is set. - */ - function createConfigServiceWithDefaults( - userConfig: Record, - defaults: Record - ): TestConfigurationService { - const configService = new TestConfigurationService(userConfig); - const originalInspect = configService.inspect.bind(configService); - configService.inspect = (key: string, overrides?: IConfigurationOverrides): IConfigurationValue => { - if (Object.prototype.hasOwnProperty.call(userConfig, key)) { - return originalInspect(key, overrides); - } - // No explicit user value: return the default as the resolved value - const defaultVal = defaults[key] as T; - return { - value: defaultVal, - defaultValue: defaultVal, - userValue: undefined, - userLocalValue: undefined, - }; - }; - const originalGetValue = configService.getValue.bind(configService); - configService.getValue = (arg1?: string | IConfigurationOverrides, arg2?: IConfigurationOverrides): T => { - const result = originalGetValue(arg1, arg2); - if (result === undefined && typeof arg1 === 'string' && Object.prototype.hasOwnProperty.call(defaults, arg1)) { - return defaults[arg1] as T; - } - return result as T; - }; - return configService; - } - - test('new user with no explicit setting gets auto-detect on light OS', () => { - const configService = new TestConfigurationService(); - const hostColor = createHostColorSchemeService(false); - const config = new ThemeConfiguration(configService, hostColor, true); - - assert.deepStrictEqual(config.getPreferredColorScheme(), ColorScheme.LIGHT); - assert.deepStrictEqual(config.isDetectingColorScheme(), true); - }); - - test('new user with no explicit setting gets auto-detect on dark OS', () => { - const configService = new TestConfigurationService(); - const hostColor = createHostColorSchemeService(true); - const config = new ThemeConfiguration(configService, hostColor, true); - - assert.deepStrictEqual(config.getPreferredColorScheme(), ColorScheme.DARK); - assert.deepStrictEqual(config.isDetectingColorScheme(), true); - }); - - test('new user with no explicit setting and schema default false still gets auto-detect', () => { - // Simulates production: getValue() returns false (schema default) but userValue is undefined - const configService = createConfigServiceWithDefaults({}, { 'window.autoDetectColorScheme': false }); - const hostColor = createHostColorSchemeService(false); - const config = new ThemeConfiguration(configService, hostColor, true); - - assert.deepStrictEqual(config.getPreferredColorScheme(), ColorScheme.LIGHT); - assert.deepStrictEqual(config.isDetectingColorScheme(), true); - }); - - test('new user with explicit false does not get auto-detect', () => { - const configService = new TestConfigurationService({ 'window.autoDetectColorScheme': false }); - const hostColor = createHostColorSchemeService(false); - const config = new ThemeConfiguration(configService, hostColor, true); - - assert.deepStrictEqual(config.getPreferredColorScheme(), undefined); - assert.deepStrictEqual(config.isDetectingColorScheme(), false); - }); - - test('existing user without explicit setting does not get auto-detect', () => { - const configService = new TestConfigurationService(); - const hostColor = createHostColorSchemeService(false); - const config = new ThemeConfiguration(configService, hostColor, false); - - assert.deepStrictEqual(config.getPreferredColorScheme(), undefined); - assert.deepStrictEqual(config.isDetectingColorScheme(), false); - }); - - test('existing user with explicit true gets auto-detect', () => { - const configService = new TestConfigurationService({ 'window.autoDetectColorScheme': true }); - const hostColor = createHostColorSchemeService(false); - const config = new ThemeConfiguration(configService, hostColor, false); - - assert.deepStrictEqual(config.getPreferredColorScheme(), ColorScheme.LIGHT); - assert.deepStrictEqual(config.isDetectingColorScheme(), true); - }); - - test('high contrast OS takes priority over auto-detect for new user', () => { - const configService = new TestConfigurationService({ 'window.autoDetectHighContrast': true }); - const hostColor = createHostColorSchemeService(true, true); - const config = new ThemeConfiguration(configService, hostColor, true); - - assert.deepStrictEqual(config.getPreferredColorScheme(), ColorScheme.HIGH_CONTRAST_DARK); - assert.deepStrictEqual(config.isDetectingColorScheme(), true); - }); - - test('high contrast light OS takes priority over auto-detect for new user', () => { - const configService = new TestConfigurationService({ 'window.autoDetectHighContrast': true }); - const hostColor = createHostColorSchemeService(false, true); - const config = new ThemeConfiguration(configService, hostColor, true); - - assert.deepStrictEqual(config.getPreferredColorScheme(), ColorScheme.HIGH_CONTRAST_LIGHT); - assert.deepStrictEqual(config.isDetectingColorScheme(), true); - }); - }); });