Skip to content

Commit

Permalink
fix #8374: set default value of colorTheme and iconTheme preference
Browse files Browse the repository at this point in the history
Also-by: tom-shan <[email protected]>
Signed-off-by: Anton Kosyakov <[email protected]>
  • Loading branch information
akosyakov committed Aug 14, 2020
1 parent 5959399 commit d12c8e5
Show file tree
Hide file tree
Showing 19 changed files with 115 additions and 46 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 10 additions & 3 deletions dev-packages/application-package/src/application-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ export namespace ApplicationProps {
},
frontend: {
config: {
applicationName: 'Eclipse Theia'
applicationName: 'Eclipse Theia',
defaultTheme: 'dark',
defaultIconTheme: 'none'
}
},
generator: {
Expand All @@ -106,9 +108,14 @@ export interface ApplicationConfig {
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`.
* The default theme for the application. If not given, 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 given, defaults to `none`. If invalid theme is given, also defaults to `none`.
*/
readonly defaultIconTheme: string;

/**
* The name of the application. `Eclipse Theia` by default.
Expand Down
15 changes: 15 additions & 0 deletions dev-packages/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
15 changes: 7 additions & 8 deletions packages/core/src/browser/common-frontend-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/browser/connection-status-service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/browser/core-preferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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': {
Expand Down Expand Up @@ -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';
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/browser/icon-theme-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
29 changes: 6 additions & 23 deletions packages/core/src/browser/icon-theme-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
Expand All @@ -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 {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -56,7 +57,8 @@ describe('Preference Proxy', () => {
before(() => {
disableJSDOM = enableJSDOM();
FrontendApplicationConfigProvider.set({
'applicationName': 'test',
...ApplicationProps.DEFAULT.frontend.config,
'applicationName': 'test'
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -56,6 +57,7 @@ describe('Preference Service', () => {
before(() => {
disableJSDOM = enableJSDOM();
FrontendApplicationConfigProvider.set({
...ApplicationProps.DEFAULT.frontend.config,
'applicationName': 'test',
});
});
Expand Down
8 changes: 3 additions & 5 deletions packages/core/src/browser/theming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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];
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
6 changes: 6 additions & 0 deletions packages/git/src/browser/git-repository-provider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
6 changes: 6 additions & 0 deletions packages/navigator/src/browser/navigator-diff.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading

0 comments on commit d12c8e5

Please sign in to comment.