fix: improve behavior with regard to sync with system theme (#1659)

src/ambient.d.ts

@@ -154,7 +154,7 @@ declare global {
       pathExists(path: string): boolean;
       platform: string;
       pushOutputEntry(entry: OutputEntry): void;
-      readThemeFile(name?: string): Promise<LoadedFiddleTheme | null>;
+      readThemeFile(name: string): Promise<LoadedFiddleTheme | null>;
       reloadWindows(): void;
       removeAllListeners(type: FiddleEvent): void;
       removeVersion(version: string): Promise<InstallState>;

src/main/themes.ts

@@ -7,7 +7,6 @@ import * as namor from 'namor';
 import { ipcMainManager } from './ipc';
 import { IpcEvents } from '../ipc-events';
 import {
-  DefaultThemes,
   FiddleTheme,
   LoadedFiddleTheme,
   defaultDark,
@@ -21,11 +20,8 @@ export const THEMES_PATH = path.join(CONFIG_PATH, 'themes');
  * Read in a theme file.
  */
 export async function readThemeFile(
-  name?: string,
+  name: string,
 ): Promise<LoadedFiddleTheme | null> {
-  if (!name || name === DefaultThemes.DARK) return defaultDark;
-  if (name === DefaultThemes.LIGHT) return defaultLight;
-
   const file = name.endsWith('.json') ? name : `${name}.json`;
   const themePath = path.join(THEMES_PATH, file);
 
@@ -117,7 +113,7 @@ export async function openThemeFolder() {
 export function setupThemes() {
   ipcMainManager.handle(
     IpcEvents.READ_THEME_FILE,
-    (_: IpcMainEvent, name?: string) => readThemeFile(name),
+    (_: IpcMainEvent, name: string) => readThemeFile(name),
   );
   ipcMainManager.handle(IpcEvents.GET_AVAILABLE_THEMES, (_: IpcMainEvent) =>
     getAvailableThemes(),

src/renderer/app.tsx

@@ -1,12 +1,13 @@
 import { autorun, reaction, when } from 'mobx';
 
+import { PREFERS_DARK_MEDIA_QUERY } from './constants';
 import { ElectronTypes } from './electron-types';
 import { FileManager } from './file-manager';
 import { RemoteLoader } from './remote-loader';
 import { Runner } from './runner';
 import { AppState } from './state';
 import { TaskRunner } from './task-runner';
-import { activateTheme, getTheme } from './themes';
+import { activateTheme, getCurrentTheme, getTheme } from './themes';
 import { getPackageJson } from './utils/get-package';
 import { getElectronVersions } from './versions';
 import {
@@ -101,7 +102,11 @@ export class App {
    * render process.
    */
   public async setup(): Promise<void | Element | React.Component> {
-    await this.loadTheme(this.state.theme || '');
+    if (this.state.isUsingSystemTheme) {
+      await this.loadTheme(getCurrentTheme().file);
+    } else {
+      await this.loadTheme(this.state.theme);
+    }
 
     const [
       { default: React },
@@ -158,33 +163,25 @@ export class App {
   }
 
   public async setupThemeListeners() {
-    const setSystemTheme = (prefersDark: boolean) => {
-      if (prefersDark) {
-        this.state.setTheme(defaultDark.file);
-      } else {
-        this.state.setTheme(defaultLight.file);
-      }
-    };
-
     // match theme to system when box is ticked
     reaction(
       () => this.state.isUsingSystemTheme,
       () => {
         if (this.state.isUsingSystemTheme) {
           window.ElectronFiddle.setNativeTheme('system');
-
-          const { matches } = window.matchMedia('(prefers-color-scheme: dark)');
-          setSystemTheme(matches);
+          this.loadTheme(getCurrentTheme().file);
+        } else {
+          this.loadTheme(this.state.theme);
         }
       },
     );
 
     // change theme when system theme changes
     window
-      .matchMedia('(prefers-color-scheme: dark)')
-      .addEventListener('change', ({ matches }) => {
+      .matchMedia(PREFERS_DARK_MEDIA_QUERY)
+      .addEventListener('change', ({ matches: prefersDark }) => {
         if (this.state.isUsingSystemTheme) {
-          setSystemTheme(matches);
+          this.loadTheme((prefersDark ? defaultDark : defaultLight).file);
         }
       });
   }
@@ -209,10 +206,10 @@ export class App {
   /**
    * Loads theme CSS into the HTML document.
    */
-  public async loadTheme(name: string): Promise<void> {
+  public async loadTheme(name: string | null): Promise<void> {
     const tag: HTMLStyleElement | null =
       document.querySelector('style#fiddle-theme');
-    const theme = await getTheme(name);
+    const theme = await getTheme(this.state, name);
     activateTheme(theme);
 
     if (tag && theme.css) {

src/renderer/components/dialog-add-theme.tsx

@@ -4,7 +4,7 @@ import { Button, Dialog, FileInput } from '@blueprintjs/core';
 import { observer } from 'mobx-react';
 import * as MonacoType from 'monaco-editor';
 
-import { FiddleTheme, defaultDark } from '../../themes-defaults';
+import { FiddleTheme } from '../../themes-defaults';
 import { AppState } from '../state';
 import { getTheme } from '../themes';
 
@@ -52,9 +52,7 @@ export const AddThemeDialog = observer(
       const { file } = this.state;
       const { appState } = this.props;
 
-      const defaultTheme = !!appState.theme
-        ? await getTheme(appState.theme)
-        : defaultDark;
+      const defaultTheme = await getTheme(appState, appState.theme);
 
       if (!file) return;
 

src/renderer/components/settings-general-appearance.tsx

@@ -13,7 +13,7 @@ import { observer } from 'mobx-react';
 
 import { LoadedFiddleTheme } from '../../themes-defaults';
 import { AppState } from '../state';
-import { getTheme } from '../themes';
+import { getCurrentTheme, getTheme } from '../themes';
 import { highlightText } from '../utils/highlight-text';
 
 const ThemeSelect = Select.ofType<LoadedFiddleTheme>();
@@ -86,15 +86,19 @@ export const AppearanceSettings = observer(
       window.ElectronFiddle.getAvailableThemes().then((themes) => {
         const { theme } = this.props.appState;
         const selectedTheme =
-          (theme && themes.find(({ file }) => file === theme)) || themes[0];
+          (theme && themes.find(({ file }) => file === theme)) ||
+          getCurrentTheme();
 
         this.setState({ themes, selectedTheme });
 
         // set up mobx so that changes from system sync are reflected in picker
         reaction(
           () => this.props.appState.theme,
           async () => {
-            const selectedTheme = await getTheme(this.props.appState.theme);
+            const selectedTheme = await getTheme(
+              this.props.appState,
+              this.props.appState.theme,
+            );
             this.setState({ selectedTheme });
           },
         );
@@ -119,7 +123,7 @@ export const AppearanceSettings = observer(
      */
     public async createNewThemeFromCurrent(): Promise<boolean> {
       const { appState } = this.props;
-      const theme = await getTheme(appState.theme);
+      const theme = await getTheme(appState, appState.theme);
 
       try {
         await window.ElectronFiddle.createThemeFile(theme);
@@ -172,8 +176,7 @@ export const AppearanceSettings = observer(
     public render() {
       const { selectedTheme } = this.state;
       const { isUsingSystemTheme } = this.props.appState;
-      const selectedName =
-        (selectedTheme && selectedTheme.name) || 'Select a theme';
+      const selectedName = selectedTheme?.name || 'Select a theme';
 
       return (
         <div className="settings-appearance">

src/renderer/constants.ts

@@ -2,3 +2,5 @@ export const ELECTRON_ORG = 'electron';
 export const ELECTRON_REPO = 'electron';
 
 export const FIDDLE_GIST_DESCRIPTION_PLACEHOLDER = 'Electron Fiddle Gist';
+
+export const PREFERS_DARK_MEDIA_QUERY = '(prefers-color-scheme: dark)';

src/renderer/state.ts

@@ -737,8 +737,8 @@ export class AppState {
     this.resetView({ isTourShowing: true });
   }
 
-  public setTheme(fileName?: string) {
-    this.theme = fileName || '';
+  public setTheme(fileName: string | null) {
+    this.theme = fileName;
     window.app.loadTheme(this.theme);
   }
 

src/renderer/themes.ts

@@ -1,7 +1,11 @@
+import { PREFERS_DARK_MEDIA_QUERY } from './constants';
+import { AppState } from './state';
 import {
+  DefaultThemes,
   FiddleTheme,
   LoadedFiddleTheme,
   defaultDark,
+  defaultLight,
 } from '../themes-defaults';
 
 /**
@@ -13,17 +17,37 @@ export function activateTheme(theme: LoadedFiddleTheme) {
   monaco.editor.setTheme('main');
 }
 
+export function getCurrentTheme(): LoadedFiddleTheme {
+  return window.matchMedia(PREFERS_DARK_MEDIA_QUERY).matches
+    ? defaultDark
+    : defaultLight;
+}
+
 /**
  * Returns a Fiddle theme, either a default one or by checking
  * the disk for a JSON file.
  */
 export async function getTheme(
-  name?: string | null,
+  appState: AppState,
+  name: string | null,
 ): Promise<LoadedFiddleTheme> {
   console.log(`Themes: getTheme() loading ${name || 'default'}`);
-  const theme =
-    (await window.ElectronFiddle.readThemeFile(name || undefined)) ||
-    defaultDark;
+
+  let theme: LoadedFiddleTheme | null = null;
+
+  if (name === DefaultThemes.LIGHT) {
+    theme = defaultLight;
+  } else if (name === DefaultThemes.DARK) {
+    theme = defaultDark;
+  } else if (name) {
+    theme = await window.ElectronFiddle.readThemeFile(name);
+  }
+
+  // If there is no theme, default to the current system theme
+  // if the app is using system theme, otherwise default to dark
+  if (!theme) {
+    theme = appState.isUsingSystemTheme ? getCurrentTheme() : defaultDark;
+  }
 
   return { ...theme, css: await getCssStringForTheme(theme) };
 }

tests/main/themes-spec.ts

@@ -11,7 +11,7 @@ import {
   openThemeFolder,
   readThemeFile,
 } from '../../src/main/themes';
-import { DefaultThemes, LoadedFiddleTheme } from '../../src/themes-defaults';
+import { LoadedFiddleTheme, defaultDark } from '../../src/themes-defaults';
 
 jest.mock('fs-extra');
 
@@ -82,21 +82,6 @@ describe('themes', () => {
   });
 
   describe('readThemeFile()', () => {
-    it('returns the default (light) theme', async () => {
-      const theme = await readThemeFile(DefaultThemes.LIGHT);
-      expect(theme!.name).toBe('Fiddle (Light)');
-    });
-
-    it('returns the default (Dark) theme', async () => {
-      const theme = await readThemeFile(DefaultThemes.DARK);
-      expect(theme!.name).toBe('Fiddle (Dark)');
-    });
-
-    it('returns the default (Dark) theme if no name provided', async () => {
-      const theme = await readThemeFile();
-      expect(theme!.name).toBe('Fiddle (Dark)');
-    });
-
     it('reads the right file if ends with .json', async () => {
       (fs.readJSON as jest.Mock).mockResolvedValueOnce({});
 
@@ -122,12 +107,7 @@ describe('themes', () => {
     let defaultTheme: LoadedFiddleTheme;
 
     beforeEach(async () => {
-      const theme = await readThemeFile();
-      if (theme) {
-        defaultTheme = theme;
-      } else {
-        fail('Error loading default theme');
-      }
+      defaultTheme = defaultDark;
     });
 
     it('filters out file and css keys', async () => {