fix #8374: set default value of colorTheme and iconTheme preference

Also-by: tom-shan <swt0008411@163.com> Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
eclipse-theia · Aug 14, 2020 · abfbc87 · abfbc87
1 parent 5959399
commit abfbc87
Show file tree

Hide file tree

Showing 19 changed files with 114 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,20 @@
 <a name="1.5.0_non_blocking_bulk_edit"></a>
 - [[monaco]](#1.5.0_non_blocking_bulk_edit) `MonacoWorkspace.applyBulkEdit` does not open any editors anymore to avoid blocking [#8329](https://github.com/eclipse-theia/theia/pull/8329)
   - Consequently, it does not accept editor opener options, and `MonacoWorkspace.openEditors` and `MonacoWorkspace.toTextEditWithEditor` are removed.
+<a name="1.5.0_declarative_default_themes"></a>
+- [[theming]](#1.5.0_declarative_default_themes) Default color and icon themes should be declared in the application package.json. [#8381](https://github.com/eclipse-theia/theia/pull/8381)
+
+  ```json
+  "theia": {
+    "frontend": {
+      "config": {
+        "defaultTheme": "light",
+        "defaultIconTheme": "vs-seti"
+      }
+    }
+  },
+  ```
+  - Consequently, `ThemeService` and `IconThemeService` don't allow to change the default color or icon theme anymore.
 
 ## v1.4.0
 

diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts
@@ -72,16 +72,15 @@ export class FrontendGenerator extends AbstractGenerator {
 ${this.ifBrowser("require('es6-promise/auto');")}
 require('reflect-metadata');
 const { Container } = require('inversify');
+const { FrontendApplicationConfigProvider } = require('@theia/core/lib/browser/frontend-application-config-provider');
+FrontendApplicationConfigProvider.set(${this.prettyStringify(this.pck.props.frontend.config)});
 const { FrontendApplication } = require('@theia/core/lib/browser');
 const { frontendApplicationModule } = require('@theia/core/lib/browser/frontend-application-module');
 const { messagingFrontendModule } = require('@theia/core/lib/${this.pck.isBrowser()
                 ? 'browser/messaging/messaging-frontend-module'
                 : 'electron-browser/messaging/electron-messaging-frontend-module'}');
 const { loggerFrontendModule } = require('@theia/core/lib/browser/logger-frontend-module');
 const { ThemeService } = require('@theia/core/lib/browser/theming');
-const { FrontendApplicationConfigProvider } = require('@theia/core/lib/browser/frontend-application-config-provider');
-
-FrontendApplicationConfigProvider.set(${this.prettyStringify(this.pck.props.frontend.config)});
 
 const container = new Container();
 container.load(frontendApplicationModule);

diff --git a/dev-packages/application-package/src/application-props.ts b/dev-packages/application-package/src/application-props.ts
@@ -80,7 +80,9 @@ export namespace ApplicationProps {
         },
         frontend: {
             config: {
-                applicationName: 'Eclipse Theia'
+                applicationName: 'Eclipse Theia',
+                defaultTheme: 'dark',
+                defaultIconTheme: 'none'
             }
         },
         generator: {
@@ -108,7 +110,12 @@ export interface FrontendApplicationConfig extends ApplicationConfig {
     /**
      * The default theme for the application. If not give, defaults to `dark`. If invalid theme is given, also defaults to `dark`.
      */
-    readonly defaultTheme?: string;
+    readonly defaultTheme: string;
+
+    /**
+     * The default icon theme for the application. If not give, defaults to `none`. If invalid theme is given, also defaults to `none`.
+     */
+    readonly defaultIconTheme: string;
 
     /**
      * The name of the application. `Eclipse Theia` by default.

diff --git a/dev-packages/cli/README.md b/dev-packages/cli/README.md
@@ -18,6 +18,7 @@
   - [**Build Target**](#build-target)
   - [**Application Properties**](#application-properties)
   - [**Default Preferences**](#default-preferences)
+  - [**Default Theme**](#default-theme)
   - [**Using Latest Builds**](#using-latest-builds)
 - [**Building**](#building)
   - [**Build**](#build)
@@ -94,6 +95,20 @@ For example, an application can update the preference value for `files.enableTra
   },
 ```
 
+### Default Theme
+
+Default color and icon themes can be configured in `theia.frontend.config` section:
+```json
+"theia": {
+    "frontend": {
+      "config": {
+        "defaultTheme": "light",
+        "defaultIconTheme": "vs-seti"
+      }
+    }
+  },
+```
+
 ### Build Target
 
 The following targets are supported: `browser` and `electron`. By default `browser` target is used.

diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts
@@ -361,7 +361,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
     }
 
     protected updateThemePreference(preferenceName: 'workbench.colorTheme' | 'workbench.iconTheme'): void {
-        const inspect = this.preferenceService.inspect<string>(preferenceName);
+        const inspect = this.preferenceService.inspect<string | null>(preferenceName);
         const workspaceValue = inspect && inspect.workspaceValue;
         const userValue = inspect && inspect.globalValue;
         const value = workspaceValue || userValue;
@@ -373,16 +373,15 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
     }
 
     protected updateThemeFromPreference(preferenceName: 'workbench.colorTheme' | 'workbench.iconTheme'): void {
-        const value = this.preferences[preferenceName];
+        const inspect = this.preferenceService.inspect<string | null>(preferenceName);
+        const workspaceValue = inspect && inspect.workspaceValue;
+        const userValue = inspect && inspect.globalValue;
+        const value = workspaceValue || userValue;
         if (value !== undefined) {
             if (preferenceName === 'workbench.colorTheme') {
-                if (!value) {
-                    this.themeService.reset();
-                } else {
-                    this.themeService.setCurrentTheme(value);
-                }
+                this.themeService.setCurrentTheme(value || this.themeService.defaultTheme.id);
             } else {
-                this.iconThemes.current = value || 'none';
+                this.iconThemes.current = value || this.iconThemes.default.id;
             }
         }
     }

diff --git a/packages/core/src/browser/connection-status-service.spec.ts b/packages/core/src/browser/connection-status-service.spec.ts
@@ -18,6 +18,12 @@ import { enableJSDOM } from '../browser/test/jsdom';
 
 let disableJSDOM = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import { expect } from 'chai';
 import { ConnectionStatus } from './connection-status-service';
 import { MockConnectionStatusService } from './test/mock-connection-status-service';

diff --git a/packages/core/src/browser/core-preferences.ts b/packages/core/src/browser/core-preferences.ts
@@ -17,6 +17,7 @@
 import { interfaces } from 'inversify';
 import { createPreferenceProxy, PreferenceProxy, PreferenceService, PreferenceContribution, PreferenceSchema } from './preferences';
 import { SUPPORTED_ENCODINGS } from './supported-encodings';
+import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
 
 export const corePreferenceSchema: PreferenceSchema = {
     'type': 'object',
@@ -53,10 +54,12 @@ export const corePreferenceSchema: PreferenceSchema = {
         },
         'workbench.colorTheme': {
             type: 'string',
+            default: FrontendApplicationConfigProvider.get().defaultTheme,
             description: 'Specifies the color theme used in the workbench.'
         },
         'workbench.iconTheme': {
             type: ['string', 'null'],
+            default: FrontendApplicationConfigProvider.get().defaultIconTheme,
             description: "Specifies the icon theme used in the workbench or 'null' to not show any file icons."
         },
         'workbench.silentNotifications': {
@@ -87,8 +90,8 @@ export interface CoreConfiguration {
     'workbench.list.openMode': 'singleClick' | 'doubleClick';
     'workbench.commandPalette.history': number;
     'workbench.editor.highlightModifiedTabs': boolean;
-    'workbench.colorTheme'?: string;
-    'workbench.iconTheme'?: string | null;
+    'workbench.colorTheme': string;
+    'workbench.iconTheme': string | null;
     'workbench.silentNotifications': boolean;
     'files.encoding': string
     'workbench.tree.renderIndentGuides': 'onHover' | 'none' | 'always';

diff --git a/packages/core/src/browser/icon-theme-contribution.ts b/packages/core/src/browser/icon-theme-contribution.ts
@@ -53,7 +53,6 @@ export class DefaultFileIconThemeContribution implements IconTheme, IconThemeCon
 
     registerIconThemes(iconThemes: IconThemeService): MaybePromise<void> {
         iconThemes.register(this);
-        iconThemes.default = this.id;
     }
 
     /* rely on behaviour before for backward-compatibility */

diff --git a/packages/core/src/browser/icon-theme-service.ts b/packages/core/src/browser/icon-theme-service.ts
@@ -18,6 +18,7 @@ import { injectable, inject, postConstruct } from 'inversify';
 import { Emitter } from '../common/event';
 import { Disposable, DisposableCollection } from '../common/disposable';
 import { LabelProviderContribution, DidChangeLabelEvent } from './label-provider';
+import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
 
 export interface IconThemeDefinition {
     readonly id: string
@@ -94,13 +95,10 @@ export class IconThemeService {
     protected readonly onDidChangeCurrentEmitter = new Emitter<string>();
     readonly onDidChangeCurrent = this.onDidChangeCurrentEmitter.event;
 
-    protected _default: IconTheme;
-
     protected readonly toDeactivate = new DisposableCollection();
 
     @postConstruct()
     protected init(): void {
-        this._default = this.noneIconTheme;
         this.register(this.noneIconTheme);
     }
 
@@ -124,12 +122,9 @@ export class IconThemeService {
             return undefined;
         }
         this._iconThemes.delete(id);
-        if (this._default === iconTheme) {
-            this._default = this.noneIconTheme;
-        }
         if (window.localStorage.getItem('iconTheme') === id) {
             window.localStorage.removeItem('iconTheme');
-            this.onDidChangeCurrentEmitter.fire(this._default.id);
+            this.onDidChangeCurrentEmitter.fire(this.default.id);
         }
         this.onDidChangeEmitter.fire(undefined);
         return iconTheme;
@@ -140,15 +135,15 @@ export class IconThemeService {
     }
 
     set current(id: string) {
-        const newCurrent = this._iconThemes.get(id) || this._default;
+        const newCurrent = this._iconThemes.get(id) || this.default;
         if (this.getCurrent().id !== newCurrent.id) {
             this.setCurrent(newCurrent);
         }
     }
 
     protected getCurrent(): IconTheme {
         const id = window.localStorage.getItem('iconTheme');
-        return id && this._iconThemes.get(id) || this._default;
+        return id && this._iconThemes.get(id) || this.default;
     }
 
     protected setCurrent(current: IconTheme): void {
@@ -158,21 +153,9 @@ export class IconThemeService {
         this.onDidChangeCurrentEmitter.fire(current.id);
     }
 
-    get default(): string {
-        return this._default.id;
+    get default(): IconTheme {
+        return this._iconThemes.get(FrontendApplicationConfigProvider.get().defaultIconTheme) || this.noneIconTheme;
     }
-
-    set default(id: string) {
-        const newDefault = this._iconThemes.get(id) || this.noneIconTheme;
-        if (this._default.id === newDefault.id) {
-            return;
-        }
-        this._default = newDefault;
-        if (!window.localStorage.getItem('iconTheme')) {
-            this.onDidChangeCurrentEmitter.fire(newDefault.id);
-        }
-    }
-
     protected load(): string | undefined {
         return window.localStorage.getItem('iconTheme') || undefined;
     }

diff --git a/packages/core/src/browser/preferences/preference-proxy.spec.ts b/packages/core/src/browser/preferences/preference-proxy.spec.ts
@@ -31,6 +31,7 @@ import { PreferenceScope } from './preference-scope';
 import { PreferenceProvider } from './preference-provider';
 import { FrontendApplicationConfigProvider } from '../frontend-application-config-provider';
 import { createPreferenceProxy, PreferenceProxyOptions, PreferenceProxy, PreferenceChangeEvent } from './preference-proxy';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
 
 disableJSDOM();
 
@@ -56,7 +57,8 @@ describe('Preference Proxy', () => {
     before(() => {
         disableJSDOM = enableJSDOM();
         FrontendApplicationConfigProvider.set({
-            'applicationName': 'test',
+            ...ApplicationProps.DEFAULT.frontend.config,
+            'applicationName': 'test'
         });
     });
 

diff --git a/packages/core/src/browser/preferences/preference-service.spec.ts b/packages/core/src/browser/preferences/preference-service.spec.ts
@@ -31,6 +31,7 @@ import { PreferenceScope } from './preference-scope';
 import { PreferenceProvider } from './preference-provider';
 import { FrontendApplicationConfigProvider } from '../frontend-application-config-provider';
 import { createPreferenceProxy, PreferenceChangeEvent } from './preference-proxy';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
 
 disableJSDOM();
 
@@ -56,6 +57,7 @@ describe('Preference Service', () => {
     before(() => {
         disableJSDOM = enableJSDOM();
         FrontendApplicationConfigProvider.set({
+            ...ApplicationProps.DEFAULT.frontend.config,
             'applicationName': 'test',
         });
     });

diff --git a/packages/core/src/browser/theming.ts b/packages/core/src/browser/theming.ts
@@ -17,6 +17,7 @@
 import { Emitter, Event } from '../common/event';
 import { Disposable } from '../common/disposable';
 import { FrontendApplicationConfigProvider } from './frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
 
 export const ThemeServiceSymbol = Symbol('ThemeService');
 
@@ -50,10 +51,7 @@ export class ThemeService {
         return global[ThemeServiceSymbol] || new ThemeService();
     }
 
-    protected constructor(
-        protected _defaultTheme: string | undefined = FrontendApplicationConfigProvider.get().defaultTheme,
-        protected fallbackTheme: string = 'dark'
-    ) {
+    protected constructor() {
         const global = window as any; // eslint-disable-line @typescript-eslint/no-explicit-any
         global[ThemeServiceSymbol] = this;
     }
@@ -134,7 +132,7 @@ export class ThemeService {
      * The default theme. If that is not applicable, returns with the fallback theme.
      */
     get defaultTheme(): Theme {
-        return this.themes[this._defaultTheme || this.fallbackTheme] || this.themes[this.fallbackTheme];
+        return this.themes[FrontendApplicationConfigProvider.get().defaultTheme] || this.themes[ApplicationProps.DEFAULT.frontend.config.defaultTheme];
     }
 
     /**

diff --git a/packages/editor-preview/src/browser/editor-preview-factory.spec.ts b/packages/editor-preview/src/browser/editor-preview-factory.spec.ts
@@ -21,6 +21,12 @@
 import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
 const disableJsDom = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import { Container } from 'inversify';
 import { WidgetFactory, WidgetManager } from '@theia/core/lib/browser';
 import { EditorWidget, EditorManager } from '@theia/editor/lib/browser';

diff --git a/packages/git/src/browser/git-repository-provider.spec.ts b/packages/git/src/browser/git-repository-provider.spec.ts
@@ -17,6 +17,12 @@
 import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
 let disableJSDOM = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import { Container } from 'inversify';
 import { Git, Repository } from '../common';
 import { DugiteGit } from '../node/dugite-git';

diff --git a/packages/markers/src/browser/marker-tree-label-provider.spec.ts b/packages/markers/src/browser/marker-tree-label-provider.spec.ts
@@ -18,6 +18,12 @@ import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
 
 let disableJSDOM = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import URI from '@theia/core/lib/common/uri';
 import { expect } from 'chai';
 import { Container } from 'inversify';

diff --git a/packages/navigator/src/browser/navigator-diff.spec.ts b/packages/navigator/src/browser/navigator-diff.spec.ts
@@ -17,6 +17,12 @@
 import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
 const disableJSDOM = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import { expect } from 'chai';
 import { NavigatorDiff } from './navigator-diff';
 import * as path from 'path';

diff --git a/packages/preferences/src/browser/util/preference-tree-generator.spec.ts b/packages/preferences/src/browser/util/preference-tree-generator.spec.ts
@@ -19,6 +19,12 @@
 import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
 const disableJSDOM = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import { expect } from 'chai';
 import { Container } from 'inversify';
 import { PreferenceTreeGenerator } from './preference-tree-generator';

diff --git a/packages/preview/src/browser/markdown/markdown-preview-handler.spec.ts b/packages/preview/src/browser/markdown/markdown-preview-handler.spec.ts
@@ -18,6 +18,12 @@ import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
 
 let disableJSDOM = enableJSDOM();
 
+import { FrontendApplicationConfigProvider } from '@theia/core/lib/browser/frontend-application-config-provider';
+import { ApplicationProps } from '@theia/application-package/lib/application-props';
+FrontendApplicationConfigProvider.set({
+    ...ApplicationProps.DEFAULT.frontend.config
+});
+
 import * as chai from 'chai';
 import { expect } from 'chai';
 import URI from '@theia/core/lib/common/uri';