Skip to content

Commit 4ae6e9f

Browse files
author
Akos Kitta
committed
fix: obsolete theme cleanup
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 204fe2d commit 4ae6e9f

File tree

6 files changed

+65
-107
lines changed

6 files changed

+65
-107
lines changed

arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -327,11 +327,9 @@ import { NewCloudSketch } from './contributions/new-cloud-sketch';
327327
import { SketchbookCompositeWidget } from './widgets/sketchbook/sketchbook-composite-widget';
328328
import { WindowTitleUpdater } from './theia/core/window-title-updater';
329329
import { WindowTitleUpdater as TheiaWindowTitleUpdater } from '@theia/core/lib/browser/window/window-title-updater';
330-
import { ThemeServiceWithDB } from './theia/core/theming';
331-
import { ThemeServiceWithDB as TheiaThemeServiceWithDB } from '@theia/monaco/lib/browser/monaco-indexed-db';
332330
import {
333331
MonacoThemingService,
334-
CleanupObsoleteIndexedDBThemes,
332+
CleanupObsoleteThemes,
335333
ThemesRegistrationSummary,
336334
MonacoThemeRegistry,
337335
} from './theia/monaco/monaco-theming-service';
@@ -986,16 +984,14 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
986984
rebind(TheiaWindowTitleUpdater).toService(WindowTitleUpdater);
987985

988986
// register Arduino themes
989-
bind(ThemeServiceWithDB).toSelf().inSingletonScope();
990-
rebind(TheiaThemeServiceWithDB).toService(ThemeServiceWithDB);
991987
bind(MonacoThemingService).toSelf().inSingletonScope();
992988
rebind(TheiaMonacoThemingService).toService(MonacoThemingService);
993989

994990
// workaround for themes cannot be removed after registration
995991
// https://github.com/eclipse-theia/theia/issues/11151
996-
bind(CleanupObsoleteIndexedDBThemes).toSelf().inSingletonScope();
992+
bind(CleanupObsoleteThemes).toSelf().inSingletonScope();
997993
bind(FrontendApplicationContribution).toService(
998-
CleanupObsoleteIndexedDBThemes
994+
CleanupObsoleteThemes
999995
);
1000996
bind(ThemesRegistrationSummary).toSelf().inSingletonScope();
1001997
bind(MonacoThemeRegistry).toSelf().inSingletonScope();

arduino-ide-extension/src/browser/dialogs/settings/settings-component.tsx

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ import SettingsStepInput from './settings-step-input';
2626
import { InterfaceScale } from '../../contributions/interface-scale';
2727
import {
2828
userConfigurableThemes,
29-
contributedThemeLabel,
30-
isBuiltInTheme,
29+
themeLabelForSettings,
3130
} from '../../theia/core/theming';
3231

3332
const maxScale = InterfaceScale.ZoomLevel.toPercentage(
@@ -338,27 +337,15 @@ export class SettingsComponent extends React.Component<
338337
);
339338
}
340339

341-
/**
342-
* The label of the currently selected theme if it's a built-in one. Otherwise, the value of the `#contributedThemeLabel`.
343-
*/
344340
private get currentThemeLabel(): string {
345341
const currentTheme = this.props.themeService.getCurrentTheme();
346-
return isBuiltInTheme(currentTheme)
347-
? currentTheme.label
348-
: contributedThemeLabel;
342+
return themeLabelForSettings(currentTheme);
349343
}
350344

351-
/**
352-
* Returns with an array of built-in theme ID/label pairs.
353-
* If the currently selected theme is a 3rd party theme,
354-
* an additional element is added to the end of the result
355-
* array with the ID of the current 3rd party theme as the
356-
* key and the value of the `#contributedThemeLabel` as the label.
357-
*/
358345
private get themeSelectOptions(): { key: string; label: string }[] {
359346
return userConfigurableThemes(this.props.themeService).map((theme) => ({
360347
key: theme.id,
361-
label: isBuiltInTheme(theme) ? theme.label : contributedThemeLabel,
348+
label: themeLabelForSettings(theme),
362349
}));
363350
}
364351

arduino-ide-extension/src/browser/theia/core/theming.ts

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,53 @@
1-
import { Disposable } from '@theia/core';
21
import {
32
BuiltinThemeProvider,
43
ThemeService,
54
} from '@theia/core/lib/browser/theming';
65
import { nls } from '@theia/core/lib/common/nls';
76
import type { Theme, ThemeType } from '@theia/core/lib/common/theme';
8-
import { injectable } from '@theia/core/shared/inversify';
9-
import { ThemeServiceWithDB as TheiaThemeServiceWithDB } from '@theia/monaco/lib/browser/monaco-indexed-db';
107

118
export namespace ArduinoThemes {
129
export const light: Theme = {
1310
id: 'arduino-theme',
1411
type: 'light',
15-
label: 'Light',
12+
label: 'Light (Arduino)',
1613
editorTheme: 'arduino-theme',
1714
};
1815
export const dark: Theme = {
1916
id: 'arduino-theme-dark',
2017
type: 'dark',
21-
label: 'Dark',
18+
label: 'Dark (Arduino)',
2219
editorTheme: 'arduino-theme-dark',
2320
};
2421
}
2522

26-
const builtInThemeIds = new Set(
23+
const officialThemeIds = new Set(
2724
[
2825
ArduinoThemes.light,
2926
ArduinoThemes.dark,
3027
BuiltinThemeProvider.hcTheme,
3128
// TODO: add the HC light theme after Theia 1.36
3229
].map(({ id }) => id)
3330
);
34-
export function isBuiltInTheme(theme: Theme | string): boolean {
31+
export function isOfficialTheme(theme: Theme | string): boolean {
3532
const themeId = typeof theme === 'string' ? theme : theme.id;
36-
return builtInThemeIds.has(themeId);
33+
return officialThemeIds.has(themeId);
34+
}
35+
36+
export function themeLabelForSettings(theme: Theme): string {
37+
switch (theme.id) {
38+
case ArduinoThemes.light.id:
39+
return nls.localize('arduino/theme/light', 'Light');
40+
case ArduinoThemes.dark.id:
41+
return nls.localize('arduino/theme/dark', 'Dark');
42+
case BuiltinThemeProvider.hcTheme.id:
43+
return nls.localize('arduino/theme/hc', 'High Contrast');
44+
default:
45+
return nls.localize(
46+
'arduino/theme/unofficialTheme',
47+
'Unofficial - {0}',
48+
theme.label
49+
);
50+
}
3751
}
3852

3953
export function compatibleBuiltInTheme(theme: Theme): Theme {
@@ -53,33 +67,7 @@ export function compatibleBuiltInTheme(theme: Theme): Theme {
5367
}
5468
}
5569

56-
export const contributedThemeLabel = nls.localize(
57-
'arduino/theme/contributedTheme',
58-
'Contributed Theme'
59-
);
60-
61-
@injectable()
62-
export class ThemeServiceWithDB extends TheiaThemeServiceWithDB {
63-
protected override init(): void {
64-
// this.register(ArduinoThemes.Light, ArduinoThemes.Dark); // TODO: check if needed
65-
super.init();
66-
}
67-
68-
override register(...themes: Theme[]): Disposable {
69-
const hcThemeIndex = themes.findIndex(
70-
(theme) => theme.id === BuiltinThemeProvider.hcTheme.id
71-
);
72-
if (hcThemeIndex >= 0) {
73-
const hcTheme = themes[hcThemeIndex];
74-
themes.splice(hcThemeIndex, 1, {
75-
...hcTheme,
76-
label: nls.localize('arduino/theme/hcDark', 'High Contrast (Dark)'),
77-
});
78-
}
79-
return super.register(...themes);
80-
}
81-
}
82-
70+
// For tests without DI
8371
interface ThemeProvider {
8472
themes(): Theme[];
8573
currentTheme(): Theme;
@@ -104,10 +92,10 @@ export function userConfigurableThemes(
10492
const currentTheme = provider.currentTheme();
10593
return provider
10694
.themes()
107-
.filter((theme) => isBuiltInTheme(theme) || currentTheme.id === theme.id)
95+
.filter((theme) => isOfficialTheme(theme) || currentTheme.id === theme.id)
10896
.sort((left, right) => {
109-
const leftBuiltIn = isBuiltInTheme(left);
110-
const rightBuiltIn = isBuiltInTheme(right);
97+
const leftBuiltIn = isOfficialTheme(left);
98+
const rightBuiltIn = isOfficialTheme(right);
11199
if (leftBuiltIn === rightBuiltIn) {
112100
return themeTypeComparator(left, right);
113101
}

arduino-ide-extension/src/browser/theia/monaco/monaco-theming-service.ts

Lines changed: 26 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
import { CommonCommands } from '@theia/core/lib/browser/common-frontend-contribution';
21
import { FrontendApplicationContribution } from '@theia/core/lib/browser/frontend-application';
32
import { ThemeService } from '@theia/core/lib/browser/theming';
4-
import { CommandService } from '@theia/core/lib/common/command';
53
import {
64
Disposable,
75
DisposableCollection,
@@ -148,9 +146,7 @@ export class MonacoThemingService extends TheiaMonacoThemingService {
148146
* Workaround for removing VSIX themes from the indexedDB if they were not loaded during the app startup.
149147
*/
150148
@injectable()
151-
export class CleanupObsoleteIndexedDBThemes
152-
implements FrontendApplicationContribution
153-
{
149+
export class CleanupObsoleteThemes implements FrontendApplicationContribution {
154150
@inject(HostedPluginSupport)
155151
private readonly hostedPlugin: HostedPluginSupport;
156152
@inject(ThemesRegistrationSummary)
@@ -159,8 +155,6 @@ export class CleanupObsoleteIndexedDBThemes
159155
private readonly themeService: ThemeService;
160156
@inject(MessageService)
161157
private readonly messageService: MessageService;
162-
@inject(CommandService)
163-
private readonly commandService: CommandService;
164158
@inject(WindowServiceExt)
165159
private readonly windowService: WindowServiceExt;
166160

@@ -177,47 +171,39 @@ export class CleanupObsoleteIndexedDBThemes
177171
if (!obsoleteThemeIds.length) {
178172
return;
179173
}
180-
await obsoleteThemeIds.reduce(async (previousTask, themeId) => {
181-
await previousTask;
182-
return deleteThemeFromIndexedDB(themeId);
183-
}, Promise.resolve());
184-
185174
const firstWindow = await this.windowService.isFirstWindow();
186175
if (firstWindow) {
187-
return this.switchToCompatibleBuiltInTheme(obsoleteThemeIds);
176+
await this.removeObsoleteThemesFromIndexedDB(obsoleteThemeIds);
177+
this.unregisterObsoleteThemes(obsoleteThemeIds);
188178
}
189179
}
190180

191-
private async switchToCompatibleBuiltInTheme(
192-
obsoleteThemeIds: string[]
193-
): Promise<void> {
181+
private removeObsoleteThemesFromIndexedDB(themeIds: string[]): Promise<void> {
182+
return themeIds.reduce(async (previousTask, themeId) => {
183+
await previousTask;
184+
return deleteThemeFromIndexedDB(themeId);
185+
}, Promise.resolve());
186+
}
187+
188+
private unregisterObsoleteThemes(themeIds: string[]): void {
194189
const currentTheme = this.themeService.getCurrentTheme();
195-
if (obsoleteThemeIds.includes(currentTheme.id)) {
196-
const message = nls.localize(
197-
'arduino/theme/couldNotLoadTheme',
198-
'Could not load your currently selected theme: {0}. Do you want to automatically select a compatible theme?',
199-
currentTheme.label
200-
);
201-
const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
202-
const selectManually = nls.localize(
203-
'arduino/theme/selectManually',
204-
'Select Manually'
190+
const switchToCompatibleTheme = themeIds.includes(currentTheme.id);
191+
for (const themeId of themeIds) {
192+
delete this.themeService['themes'][themeId];
193+
}
194+
this.themeService['doUpdateColorThemePreference']();
195+
if (switchToCompatibleTheme) {
196+
this.themeService.setCurrentTheme(
197+
compatibleBuiltInTheme(currentTheme).id,
198+
true
205199
);
206-
const answer = await this.messageService.info(
207-
message,
208-
selectManually,
209-
yes
200+
this.messageService.info(
201+
nls.localize(
202+
'arduino/theme/currentThemeNotFound',
203+
'Could not find the currently selected theme: {0}. Arduino IDE has selected a compatible official theme.',
204+
currentTheme.label
205+
)
210206
);
211-
if (answer === yes) {
212-
this.themeService.setCurrentTheme(
213-
compatibleBuiltInTheme(currentTheme).id,
214-
true
215-
);
216-
} else if (answer === selectManually) {
217-
return this.commandService.executeCommand(
218-
CommonCommands.SELECT_COLOR_THEME.id
219-
);
220-
}
221207
}
222208
}
223209
}

arduino-ide-extension/src/test/browser/theming.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Theme } from '@theia/core/lib/common/theme';
66
import { expect } from 'chai';
77
import {
88
ArduinoThemes,
9-
isBuiltInTheme,
9+
isOfficialTheme,
1010
userConfigurableThemes,
1111
} from '../../browser/theia/core/theming';
1212

@@ -75,7 +75,7 @@ describe('theming', () => {
7575
it(`should${expected ? '' : ' not'} treat '${
7676
theme.id
7777
}' theme as built-in`, () =>
78-
expect(isBuiltInTheme(theme)).to.be.equal(expected))
78+
expect(isOfficialTheme(theme)).to.be.equal(expected))
7979
);
8080
});
8181
});

i18n/en.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -467,10 +467,11 @@
467467
"surveyMessage": "Please help us improve by answering this super short survey. We value our community and would like to get to know our supporters a little better."
468468
},
469469
"theme": {
470-
"contributedTheme": "Contributed Theme",
471-
"couldNotLoadTheme": "Could not load your currently selected theme: {0}. Do you want to automatically select a compatible theme?",
472-
"hcDark": "High Contrast (Dark)",
473-
"selectManually": "Select Manually"
470+
"currentThemeNotFound": "Could not find the currently selected theme: {0}. Arduino IDE has selected a compatible official theme.",
471+
"dark": "Dark",
472+
"hc": "High Contrast",
473+
"light": "Light",
474+
"unofficialTheme": "Unofficial - {0}"
474475
},
475476
"title": {
476477
"cloud": "Cloud"

0 commit comments

Comments
 (0)