Skip to content

Commit

Permalink
fix: improve behavior when regarding sync with system theme
Browse files Browse the repository at this point in the history
  • Loading branch information
dsanders11 committed Dec 6, 2024
1 parent 5232b60 commit cbf60fb
Show file tree
Hide file tree
Showing 12 changed files with 117 additions and 84 deletions.
2 changes: 1 addition & 1 deletion src/ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
8 changes: 2 additions & 6 deletions src/main/themes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import * as namor from 'namor';
import { ipcMainManager } from './ipc';
import { IpcEvents } from '../ipc-events';
import {
DefaultThemes,
FiddleTheme,
LoadedFiddleTheme,
defaultDark,
Expand All @@ -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);

Expand Down Expand Up @@ -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(),
Expand Down
33 changes: 15 additions & 18 deletions src/renderer/app.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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);
}
});
}
Expand All @@ -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) {
Expand Down
6 changes: 2 additions & 4 deletions src/renderer/components/dialog-add-theme.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

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

Expand Down
15 changes: 9 additions & 6 deletions src/renderer/components/settings-general-appearance.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down Expand Up @@ -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 });
},
);
Expand All @@ -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);
Expand Down Expand Up @@ -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">
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)';
4 changes: 2 additions & 2 deletions src/renderer/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
32 changes: 28 additions & 4 deletions src/renderer/themes.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { PREFERS_DARK_MEDIA_QUERY } from './constants';
import { AppState } from './state';
import {
DefaultThemes,
FiddleTheme,
LoadedFiddleTheme,
defaultDark,
defaultLight,
} from '../themes-defaults';

/**
Expand All @@ -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) };
}
Expand Down
24 changes: 2 additions & 22 deletions tests/main/themes-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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({});

Expand All @@ -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 () => {
Expand Down
Loading

0 comments on commit cbf60fb

Please sign in to comment.