new theme notification (#306827)

* new theme notification

* update

* update

* update

* update description

* add colon

Author: aeschli

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;
 }

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<boolean>(Setting.DETECT_COLOR_SCHEME.key);
-			return userValue === undefined;
-		}
 		return false;
 	}
 

src/vs/platform/windows/electron-main/windowsMainService.ts

@@ -1561,7 +1561,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(),

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<void> {
 		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<void> {
-		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<boolean>(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)

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<boolean>(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 {

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<string, unknown>,
-			defaults: Record<string, unknown>
-		): TestConfigurationService {
-			const configService = new TestConfigurationService(userConfig);
-			const originalInspect = configService.inspect.bind(configService);
-			configService.inspect = <T>(key: string, overrides?: IConfigurationOverrides): IConfigurationValue<T> => {
-				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 = <T>(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);
-		});
-	});
 });