fix: obsolete theme cleanup

Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

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

@@ -327,11 +327,9 @@ import { NewCloudSketch } from './contributions/new-cloud-sketch';
 import { SketchbookCompositeWidget } from './widgets/sketchbook/sketchbook-composite-widget';
 import { WindowTitleUpdater } from './theia/core/window-title-updater';
 import { WindowTitleUpdater as TheiaWindowTitleUpdater } from '@theia/core/lib/browser/window/window-title-updater';
-import { ThemeServiceWithDB } from './theia/core/theming';
-import { ThemeServiceWithDB as TheiaThemeServiceWithDB } from '@theia/monaco/lib/browser/monaco-indexed-db';
 import {
   MonacoThemingService,
-  CleanupObsoleteIndexedDBThemes,
+  CleanupObsoleteThemes,
   ThemesRegistrationSummary,
   MonacoThemeRegistry,
 } from './theia/monaco/monaco-theming-service';
@@ -986,16 +984,14 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
   rebind(TheiaWindowTitleUpdater).toService(WindowTitleUpdater);
 
   // register Arduino themes
-  bind(ThemeServiceWithDB).toSelf().inSingletonScope();
-  rebind(TheiaThemeServiceWithDB).toService(ThemeServiceWithDB);
   bind(MonacoThemingService).toSelf().inSingletonScope();
   rebind(TheiaMonacoThemingService).toService(MonacoThemingService);
 
   // workaround for themes cannot be removed after registration
   // https://github.com/eclipse-theia/theia/issues/11151
-  bind(CleanupObsoleteIndexedDBThemes).toSelf().inSingletonScope();
+  bind(CleanupObsoleteThemes).toSelf().inSingletonScope();
   bind(FrontendApplicationContribution).toService(
-    CleanupObsoleteIndexedDBThemes
+    CleanupObsoleteThemes
   );
   bind(ThemesRegistrationSummary).toSelf().inSingletonScope();
   bind(MonacoThemeRegistry).toSelf().inSingletonScope();

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

@@ -26,8 +26,7 @@ import SettingsStepInput from './settings-step-input';
 import { InterfaceScale } from '../../contributions/interface-scale';
 import {
   userConfigurableThemes,
-  contributedThemeLabel,
-  isBuiltInTheme,
+  themeLabelForSettings,
 } from '../../theia/core/theming';
 
 const maxScale = InterfaceScale.ZoomLevel.toPercentage(
@@ -338,27 +337,15 @@ export class SettingsComponent extends React.Component<
     );
   }
 
-  /**
-   * The label of the currently selected theme if it's a built-in one. Otherwise, the value of the `#contributedThemeLabel`.
-   */
   private get currentThemeLabel(): string {
     const currentTheme = this.props.themeService.getCurrentTheme();
-    return isBuiltInTheme(currentTheme)
-      ? currentTheme.label
-      : contributedThemeLabel;
+    return themeLabelForSettings(currentTheme);
   }
 
-  /**
-   * Returns with an array of built-in theme ID/label pairs.
-   * If the currently selected theme is a 3rd party theme,
-   * an additional element is added to the end of the result
-   * array with the ID of the current 3rd party theme as the
-   * key and the value of the `#contributedThemeLabel` as the label.
-   */
   private get themeSelectOptions(): { key: string; label: string }[] {
     return userConfigurableThemes(this.props.themeService).map((theme) => ({
       key: theme.id,
-      label: isBuiltInTheme(theme) ? theme.label : contributedThemeLabel,
+      label: themeLabelForSettings(theme),
     }));
   }
 

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

@@ -1,39 +1,53 @@
-import { Disposable } from '@theia/core';
 import {
   BuiltinThemeProvider,
   ThemeService,
 } from '@theia/core/lib/browser/theming';
 import { nls } from '@theia/core/lib/common/nls';
 import type { Theme, ThemeType } from '@theia/core/lib/common/theme';
-import { injectable } from '@theia/core/shared/inversify';
-import { ThemeServiceWithDB as TheiaThemeServiceWithDB } from '@theia/monaco/lib/browser/monaco-indexed-db';
 
 export namespace ArduinoThemes {
   export const light: Theme = {
     id: 'arduino-theme',
     type: 'light',
-    label: 'Light',
+    label: 'Light (Arduino)',
     editorTheme: 'arduino-theme',
   };
   export const dark: Theme = {
     id: 'arduino-theme-dark',
     type: 'dark',
-    label: 'Dark',
+    label: 'Dark (Arduino)',
     editorTheme: 'arduino-theme-dark',
   };
 }
 
-const builtInThemeIds = new Set(
+const officialThemeIds = new Set(
   [
     ArduinoThemes.light,
     ArduinoThemes.dark,
     BuiltinThemeProvider.hcTheme,
     // TODO: add the HC light theme after Theia 1.36
   ].map(({ id }) => id)
 );
-export function isBuiltInTheme(theme: Theme | string): boolean {
+export function isOfficialTheme(theme: Theme | string): boolean {
   const themeId = typeof theme === 'string' ? theme : theme.id;
-  return builtInThemeIds.has(themeId);
+  return officialThemeIds.has(themeId);
+}
+
+export function themeLabelForSettings(theme: Theme): string {
+  switch (theme.id) {
+    case ArduinoThemes.light.id:
+      return nls.localize('arduino/theme/light', 'Light');
+    case ArduinoThemes.dark.id:
+      return nls.localize('arduino/theme/dark', 'Dark');
+    case BuiltinThemeProvider.hcTheme.id:
+      return nls.localize('arduino/theme/hc', 'High Contrast');
+    default:
+      return nls.localize(
+        'arduino/theme/unofficialTheme',
+        'Unofficial - {0}',
+        theme.label
+      );
+  }
 }
 
 export function compatibleBuiltInTheme(theme: Theme): Theme {
@@ -53,33 +67,7 @@ export function compatibleBuiltInTheme(theme: Theme): Theme {
   }
 }
 
-export const contributedThemeLabel = nls.localize(
-  'arduino/theme/contributedTheme',
-  'Contributed Theme'
-);
-
-@injectable()
-export class ThemeServiceWithDB extends TheiaThemeServiceWithDB {
-  protected override init(): void {
-    // this.register(ArduinoThemes.Light, ArduinoThemes.Dark); // TODO: check if needed
-    super.init();
-  }
-
-  override register(...themes: Theme[]): Disposable {
-    const hcThemeIndex = themes.findIndex(
-      (theme) => theme.id === BuiltinThemeProvider.hcTheme.id
-    );
-    if (hcThemeIndex >= 0) {
-      const hcTheme = themes[hcThemeIndex];
-      themes.splice(hcThemeIndex, 1, {
-        ...hcTheme,
-        label: nls.localize('arduino/theme/hcDark', 'High Contrast (Dark)'),
-      });
-    }
-    return super.register(...themes);
-  }
-}
-
+// For tests without DI
 interface ThemeProvider {
   themes(): Theme[];
   currentTheme(): Theme;
@@ -104,10 +92,10 @@ export function userConfigurableThemes(
   const currentTheme = provider.currentTheme();
   return provider
     .themes()
-    .filter((theme) => isBuiltInTheme(theme) || currentTheme.id === theme.id)
+    .filter((theme) => isOfficialTheme(theme) || currentTheme.id === theme.id)
     .sort((left, right) => {
-      const leftBuiltIn = isBuiltInTheme(left);
-      const rightBuiltIn = isBuiltInTheme(right);
+      const leftBuiltIn = isOfficialTheme(left);
+      const rightBuiltIn = isOfficialTheme(right);
       if (leftBuiltIn === rightBuiltIn) {
         return themeTypeComparator(left, right);
       }

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

@@ -1,7 +1,5 @@
-import { CommonCommands } from '@theia/core/lib/browser/common-frontend-contribution';
 import { FrontendApplicationContribution } from '@theia/core/lib/browser/frontend-application';
 import { ThemeService } from '@theia/core/lib/browser/theming';
-import { CommandService } from '@theia/core/lib/common/command';
 import {
   Disposable,
   DisposableCollection,
@@ -148,9 +146,7 @@ export class MonacoThemingService extends TheiaMonacoThemingService {
  * Workaround for removing VSIX themes from the indexedDB if they were not loaded during the app startup.
  */
 @injectable()
-export class CleanupObsoleteIndexedDBThemes
-  implements FrontendApplicationContribution
-{
+export class CleanupObsoleteThemes implements FrontendApplicationContribution {
   @inject(HostedPluginSupport)
   private readonly hostedPlugin: HostedPluginSupport;
   @inject(ThemesRegistrationSummary)
@@ -159,8 +155,6 @@ export class CleanupObsoleteIndexedDBThemes
   private readonly themeService: ThemeService;
   @inject(MessageService)
   private readonly messageService: MessageService;
-  @inject(CommandService)
-  private readonly commandService: CommandService;
   @inject(WindowServiceExt)
   private readonly windowService: WindowServiceExt;
 
@@ -177,47 +171,39 @@ export class CleanupObsoleteIndexedDBThemes
     if (!obsoleteThemeIds.length) {
       return;
     }
-    await obsoleteThemeIds.reduce(async (previousTask, themeId) => {
-      await previousTask;
-      return deleteThemeFromIndexedDB(themeId);
-    }, Promise.resolve());
-
     const firstWindow = await this.windowService.isFirstWindow();
     if (firstWindow) {
-      return this.switchToCompatibleBuiltInTheme(obsoleteThemeIds);
+      await this.removeObsoleteThemesFromIndexedDB(obsoleteThemeIds);
+      this.unregisterObsoleteThemes(obsoleteThemeIds);
     }
   }
 
-  private async switchToCompatibleBuiltInTheme(
-    obsoleteThemeIds: string[]
-  ): Promise<void> {
+  private removeObsoleteThemesFromIndexedDB(themeIds: string[]): Promise<void> {
+    return themeIds.reduce(async (previousTask, themeId) => {
+      await previousTask;
+      return deleteThemeFromIndexedDB(themeId);
+    }, Promise.resolve());
+  }
+
+  private unregisterObsoleteThemes(themeIds: string[]): void {
     const currentTheme = this.themeService.getCurrentTheme();
-    if (obsoleteThemeIds.includes(currentTheme.id)) {
-      const message = nls.localize(
-        'arduino/theme/couldNotLoadTheme',
-        'Could not load your currently selected theme: {0}. Do you want to automatically select a compatible theme?',
-        currentTheme.label
-      );
-      const yes = nls.localize('vscode/extensionsUtils/yes', 'Yes');
-      const selectManually = nls.localize(
-        'arduino/theme/selectManually',
-        'Select Manually'
+    const switchToCompatibleTheme = themeIds.includes(currentTheme.id);
+    for (const themeId of themeIds) {
+      delete this.themeService['themes'][themeId];
+    }
+    this.themeService['doUpdateColorThemePreference']();
+    if (switchToCompatibleTheme) {
+      this.themeService.setCurrentTheme(
+        compatibleBuiltInTheme(currentTheme).id,
+        true
       );
-      const answer = await this.messageService.info(
-        message,
-        selectManually,
-        yes
+      this.messageService.info(
+        nls.localize(
+          'arduino/theme/currentThemeNotFound',
+          'Could not find the currently selected theme: {0}. Arduino IDE has selected a compatible official theme.',
+          currentTheme.label
+        )
       );
-      if (answer === yes) {
-        this.themeService.setCurrentTheme(
-          compatibleBuiltInTheme(currentTheme).id,
-          true
-        );
-      } else if (answer === selectManually) {
-        return this.commandService.executeCommand(
-          CommonCommands.SELECT_COLOR_THEME.id
-        );
-      }
     }
   }
 }

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

@@ -6,7 +6,7 @@ import { Theme } from '@theia/core/lib/common/theme';
 import { expect } from 'chai';
 import {
   ArduinoThemes,
-  isBuiltInTheme,
+  isOfficialTheme,
   userConfigurableThemes,
 } from '../../browser/theia/core/theming';
 
@@ -75,7 +75,7 @@ describe('theming', () => {
       it(`should${expected ? '' : ' not'} treat '${
         theme.id
       }' theme as built-in`, () =>
-        expect(isBuiltInTheme(theme)).to.be.equal(expected))
+        expect(isOfficialTheme(theme)).to.be.equal(expected))
     );
   });
 });

i18n/en.json

@@ -467,10 +467,11 @@
       "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."
     },
     "theme": {
-      "contributedTheme": "Contributed Theme",
-      "couldNotLoadTheme": "Could not load your currently selected theme: {0}. Do you want to automatically select a compatible theme?",
-      "hcDark": "High Contrast (Dark)",
-      "selectManually": "Select Manually"
+      "currentThemeNotFound": "Could not find the currently selected theme: {0}. Arduino IDE has selected a compatible official theme.",
+      "dark": "Dark",
+      "hc": "High Contrast",
+      "light": "Light",
+      "unofficialTheme": "Unofficial - {0}"
     },
     "title": {
       "cloud": "Cloud"