Repository: electron/fiddle (ts)

Issue:
Switching Electron versions causes Fiddle to be marked as unsaved
Fiddle's unsaved handler shouldn't trigger when the version changes. This may be caused by us re-loading the editor contents when we change versions.

Pull request:
feat: make document.title track application state
In master, the title is only updated when `replaceFiddle()` is called.

This PR makes title a computed appState property that updates whenever [ gistId, isUnsaved, localPath, templateName ] change. App listens for this to keep document.title in sync with state.title.

This change also has a nice side-effect that loading `Show Me` templates or pressing `New Test` will show the template name.

Fixes #599.

Patch:
diff --git a/src/renderer/app.tsx b/src/renderer/app.tsx
index 5be8256a..6114e1b5 100644
--- a/src/renderer/app.tsx
+++ b/src/renderer/app.tsx
@@ -16,7 +16,6 @@ import { WEBCONTENTS_READY_FOR_IPC_SIGNAL } from '../ipc-events';
 import { updateEditorLayout } from '../utils/editor-layout';
 import { getEditorValue } from '../utils/editor-value';
 import { getPackageJson, PackageJsonOptions } from '../utils/get-package';
-import { getTitle } from '../utils/get-title';
 import { isEditorBackup } from '../utils/type-checks';
 import { EMPTY_EDITOR_CONTENT, SORTED_EDITORS } from './constants';
 import { FileManager } from './file-manager';
@@ -88,8 +87,6 @@ export class App {
     await this.setEditorValues(editorValues);
     this.state.isUnsaved = false;
 
-    document.title = getTitle(this.state);
-
     return true;
   }
 
@@ -188,6 +185,7 @@ export class App {
 
     this.setupResizeListener();
     this.setupThemeListeners();
+    this.setupTitleListeners();
 
     ipcRenderer.send(WEBCONTENTS_READY_FOR_IPC_SIGNAL);
 
@@ -269,6 +267,25 @@ export class App {
   public setupResizeListener(): void {
     window.addEventListener('resize', updateEditorLayout);
   }
+
+  /**
+   * Have document.title track state.title
+   */
+  public setupTitleListeners() {
+    // the observables used for the title usually change in a batch,
+    // so when setting document title, wait a tick to avoid flicker.
+    let titleIdle: any;
+    reaction(
+      () => this.state.title,
+      (title) => {
+        clearTimeout(titleIdle);
+        titleIdle = setTimeout(() => {
+          document.title = title;
+          titleIdle = null;
+        });
+      },
+    );
+  }
 }
 
 window.ElectronFiddle = window.ElectronFiddle || {};
diff --git a/src/renderer/components/chrome-mac.tsx b/src/renderer/components/chrome-mac.tsx
index 2b9d20cb..ff146a32 100644
--- a/src/renderer/components/chrome-mac.tsx
+++ b/src/renderer/components/chrome-mac.tsx
@@ -2,7 +2,6 @@ import { observer } from 'mobx-react';
 import * as React from 'react';
 import { IpcEvents } from '../../ipc-events';
 
-import { getTitle } from '../../utils/get-title';
 import { ipcRendererManager } from '../ipc';
 import { AppState } from '../state';
 
@@ -21,7 +20,7 @@ export class ChromeMac extends React.Component<ChromeMacProps> {
 
     return (
       <div className="chrome drag" onDoubleClick={this.handleDoubleClick}>
-        <small>{getTitle(this.props.appState)}</small>
+        <small>{this.props.appState.title}</small>
       </div>
     );
   }
diff --git a/src/renderer/components/editors.tsx b/src/renderer/components/editors.tsx
index 7558ff52..30a442b3 100644
--- a/src/renderer/components/editors.tsx
+++ b/src/renderer/components/editors.tsx
@@ -17,7 +17,12 @@ import {
   MosaicWindowProps,
 } from 'react-mosaic-component';
 
-import { EditorId, MosaicId, PanelId } from '../../interfaces';
+import {
+  EditorId,
+  MosaicId,
+  PanelId,
+  SetFiddleOptions,
+} from '../../interfaces';
 import { IpcEvents } from '../../ipc-events';
 import { updateEditorLayout } from '../../utils/editor-layout';
 import { getFocusedEditor } from '../../utils/focused-editor';
@@ -101,12 +106,14 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
     ipcRendererManager.on(IpcEvents.FS_NEW_FIDDLE, async (_event) => {
       const { version } = this.props.appState;
       const fiddle = await getTemplate(version);
-      await window.ElectronFiddle.app.replaceFiddle(fiddle, {});
+      const options: SetFiddleOptions = { templateName: version };
+      await window.ElectronFiddle.app.replaceFiddle(fiddle, options);
     });
 
     ipcRendererManager.on(IpcEvents.FS_NEW_TEST, async (_event) => {
       const fiddle = await getTestTemplate();
-      await window.ElectronFiddle.app.replaceFiddle(fiddle, {});
+      const options: SetFiddleOptions = { templateName: 'Test' };
+      await window.ElectronFiddle.app.replaceFiddle(fiddle, options);
     });
 
     ipcRendererManager.on(
diff --git a/src/renderer/state.ts b/src/renderer/state.ts
index b79484d3..84c88f1f 100644
--- a/src/renderer/state.ts
+++ b/src/renderer/state.ts
@@ -16,6 +16,7 @@ import {
   OutputEntry,
   OutputOptions,
   RunnableVersion,
+  SetFiddleOptions,
   Version,
   VersionSource,
   VersionState,
@@ -27,7 +28,7 @@ import {
   createMosaicArrangement,
   getVisibleMosaics,
 } from '../utils/editors-mosaic-arrangement';
-import { getName } from '../utils/get-title';
+import { getName } from '../utils/get-name';
 import { normalizeVersion } from '../utils/normalize-version';
 import { isEditorBackup, isEditorId, isPanelId } from '../utils/type-checks';
 import {
@@ -298,6 +299,33 @@ export class AppState {
     ]);
   }
 
+  /**
+   * @returns {string} the title, e.g. appname, fiddle name, state
+   */
+  @computed get title(): string {
+    const { gistId, isUnsaved, localPath, templateName } = this;
+    const tokens = [];
+
+    if (localPath) {
+      tokens.push(localPath);
+    } else if (templateName) {
+      tokens.push(templateName);
+    } else if (gistId) {
+      tokens.push(`gist.github.com/${gistId}`);
+    }
+
+    if (isUnsaved) {
+      tokens.push('Unsaved');
+    }
+
+    if (tokens.length > 0) {
+      tokens.unshift('-');
+    }
+
+    tokens.unshift('Electron Fiddle');
+    return tokens.join(' ');
+  }
+
   /**
    * Returns the current RunnableVersion or the first
    * one that can be found.
@@ -357,6 +385,11 @@ export class AppState {
     this.isUpdatingElectronVersions = false;
   }
 
+  public async getName() {
+    this.name ||= await getName(this);
+    return this.name;
+  }
+
   @action public hideChannels(channels: Array<ElectronReleaseChannel>) {
     this.channelsToShow = this.channelsToShow.filter(
       (ch) => !channels.includes(ch),
@@ -371,14 +404,6 @@ export class AppState {
     this.channelsToShow = [...s.values()];
   }
 
-  @action public async getName() {
-    if (!this.name) {
-      this.name = await getName(this);
-    }
-
-    return this.name;
-  }
-
   @action public toggleConsole() {
     this.isConsoleShowing = !this.isConsoleShowing;
   }
@@ -561,7 +586,8 @@ export class AppState {
     // Should we update the editor?
     if (await isContentUnchanged(EditorId.main, this.version)) {
       const editorValues = await getTemplate(version);
-      await window.ElectronFiddle.app.replaceFiddle(editorValues, {});
+      const options: SetFiddleOptions = { templateName: version };
+      await window.ElectronFiddle.app.replaceFiddle(editorValues, options);
     }
 
     this.version = version;
diff --git a/src/utils/get-name.ts b/src/utils/get-name.ts
new file mode 100644
index 00000000..b16a1456
--- /dev/null
+++ b/src/utils/get-name.ts
@@ -0,0 +1,17 @@
+import { AppState } from '../renderer/state';
+
+/**
+ * Returns a name for this project
+ *
+ * @param {AppState} appState
+ * @returns {Promise<string>}
+ */
+export async function getName(appState: AppState): Promise<string> {
+  if (appState.localPath) {
+    const path = await import('path');
+    return path.basename(appState.localPath);
+  } else {
+    const namor = await import('namor');
+    return namor.generate({ words: 3, numbers: 0 });
+  }
+}
diff --git a/src/utils/get-title.ts b/src/utils/get-title.ts
deleted file mode 100644
index 5b959347..00000000
--- a/src/utils/get-title.ts
+++ /dev/null
@@ -1,33 +0,0 @@
-import { AppState } from '../renderer/state';
-
-/**
- * Returns the window title
- *
- * @param {AppState} appState
- * @returns {string}
- */
-export function getTitle(appState: AppState): string {
-  const { gistId, localPath } = appState;
-  const gist = gistId ? `gist.github.com/${appState.gistId}` : '';
-  const local = localPath ? localPath : '';
-  const separator = gist && local ? ' ' : '';
-  const unsaved = !gist && !local ? 'Unsaved' : '';
-
-  return `Electron Fiddle - ${gist}${separator}${local}${unsaved}`;
-}
-
-/**
- * Returns a name for this project
- *
- * @param {AppState} appState
- * @returns {Promise<string>}
- */
-export async function getName(appState: AppState): Promise<string> {
-  if (appState.localPath) {
-    const path = await import('path');
-    return path.basename(appState.localPath);
-  } else {
-    const namor = await import('namor');
-    return namor.generate({ words: 3, numbers: 0 });
-  }
-}


Test patch:
diff --git a/tests/mocks/state.ts b/tests/mocks/state.ts
index 947e2f03..3aa0f646 100644
--- a/tests/mocks/state.ts
+++ b/tests/mocks/state.ts
@@ -6,6 +6,7 @@ export class MockState {
   @observable public isConsoleShowing = true;
   @observable public isGenericDialogShowing = false;
   @observable public isUsingSystemTheme = true;
+  @observable public title = 'Electron Fiddle';
   @observable public output = [];
   @observable public versionsToShow = [];
 
diff --git a/tests/renderer/app-spec.tsx b/tests/renderer/app-spec.tsx
index 6f089e3d..376d8ac0 100644
--- a/tests/renderer/app-spec.tsx
+++ b/tests/renderer/app-spec.tsx
@@ -1,6 +1,7 @@
 import { App } from '../../src/renderer/app';
 import { AppState } from '../../src/renderer/state';
 import { EditorBackup } from '../../src/utils/editor-backup';
+import { waitFor } from '../../src/utils/wait-for';
 import { ElectronFiddleMock } from '../mocks/electron-fiddle';
 import { MockState } from '../mocks/state';
 import { EditorId } from '../../src/interfaces';
@@ -551,4 +552,21 @@ describe('App component', () => {
       });
     });
   });
+
+  describe('setupTitleListeners()', () => {
+    let app: App;
+
+    beforeEach(() => {
+      app = new App();
+      (app.state as Partial<AppState>) = new MockState();
+    });
+
+    it('updates the document title when state.title changes', async () => {
+      const title = 'Hello, World!';
+      app.setupTitleListeners();
+      (app.state.title as any) = title;
+      await waitFor(() => document.title?.length > 0);
+      expect(document.title).toMatch(title);
+    });
+  });
 });
diff --git a/tests/renderer/components/__snapshots__/chrome-mac-spec.tsx.snap b/tests/renderer/components/__snapshots__/chrome-mac-spec.tsx.snap
index fc0b5065..f0897fa3 100644
--- a/tests/renderer/components/__snapshots__/chrome-mac-spec.tsx.snap
+++ b/tests/renderer/components/__snapshots__/chrome-mac-spec.tsx.snap
@@ -6,7 +6,7 @@ exports[`Chrome-Mac component renders 1`] = `
   onDoubleClick={[Function]}
 >
   <small>
-    Electron Fiddle - Unsaved
+    Electron Fiddle
   </small>
 </div>
 `;
diff --git a/tests/renderer/components/chrome-mac-spec.tsx b/tests/renderer/components/chrome-mac-spec.tsx
index 14e11c89..80fd3f46 100644
--- a/tests/renderer/components/chrome-mac-spec.tsx
+++ b/tests/renderer/components/chrome-mac-spec.tsx
@@ -1,4 +1,5 @@
 import { mount, shallow } from 'enzyme';
+import { MockState } from '../../mocks/state';
 import * as React from 'react';
 import { IpcEvents } from '../../../src/ipc-events';
 
@@ -7,7 +8,7 @@ import { ipcRendererManager } from '../../../src/renderer/ipc';
 import { overridePlatform, resetPlatform } from '../../utils';
 
 describe('Chrome-Mac component', () => {
-  const store: any = {};
+  const store: any = new MockState();
 
   afterEach(() => resetPlatform());
 
diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts
index 585f6c20..23698945 100644
--- a/tests/renderer/state-spec.ts
+++ b/tests/renderer/state-spec.ts
@@ -31,7 +31,7 @@ import {
 } from '../../src/renderer/versions';
 import { createMosaicArrangement } from '../../src/utils/editors-mosaic-arrangement';
 import { waitFor } from '../../src/utils/wait-for';
-import { getName } from '../../src/utils/get-title';
+import { getName } from '../../src/utils/get-name';
 import { mockVersions, mockVersionsArray } from '../mocks/electron-versions';
 import { overridePlatform, resetPlatform } from '../utils';
 
@@ -65,7 +65,7 @@ jest.mock('../../src/renderer/versions', () => {
     getReleaseChannel,
   };
 });
-jest.mock('../../src/utils/get-title', () => ({
+jest.mock('../../src/utils/get-name', () => ({
   getName: jest.fn(),
 }));
 jest.mock('../../src/renderer/ipc');
@@ -717,4 +717,55 @@ describe('AppState', () => {
       expect(appState.acceleratorsToBlock).toEqual([]);
     });
   });
+
+  describe('title', () => {
+    const APPNAME = 'Electron Fiddle';
+
+    it('defaults to the appname', () => {
+      const expected = APPNAME;
+      const actual = appState.title;
+      expect(actual).toBe(expected);
+    });
+
+    it('shows the name of local fiddles', () => {
+      const localPath = 'path/to/fiddle';
+      const expected = `${APPNAME} - ${localPath}`;
+      appState.localPath = localPath;
+      const actual = appState.title;
+      expect(actual).toBe(expected);
+    });
+
+    it('shows the name of gist fiddles', () => {
+      const gistId = 'abcdef';
+      const expected = `${APPNAME} - gist.github.com/${gistId}`;
+      appState.gistId = gistId;
+      const actual = appState.title;
+      expect(actual).toBe(expected);
+    });
+
+    it('prefers to display localPath', () => {
+      const gistId = 'abcdef';
+      const templateName = 'BrowserWindow';
+      const localPath = 'path/to/fiddle';
+      const expected = `${APPNAME} - ${localPath}`;
+      Object.assign(appState, { gistId, localPath, templateName });
+      const actual = appState.title;
+      expect(actual).toBe(expected);
+    });
+
+    it('shows the name of template fiddles', () => {
+      const templateName = 'BrowserWindow';
+      const expected = `${APPNAME} - ${templateName}`;
+      appState.templateName = templateName;
+      const actual = appState.title;
+      expect(actual).toBe(expected);
+    });
+
+    it('flags unsaved fiddles', () => {
+      const expected = `${APPNAME} - Unsaved`;
+      appState.isUnsaved = true;
+      const actual = appState.title;
+      expect(actual).toBe(expected);
+    });
+  });
 });
diff --git a/tests/utils/get-name-spec.ts b/tests/utils/get-name-spec.ts
new file mode 100644
index 00000000..cdfc3a4b
--- /dev/null
+++ b/tests/utils/get-name-spec.ts
@@ -0,0 +1,15 @@
+import { getName } from '../../src/utils/get-name';
+
+describe('get-name', () => {
+  describe('getName()', () => {
+    it('returns a random name', async () => {
+      const result = await getName({} as any);
+      expect(result).toBeTruthy();
+    });
+
+    it('returns a name from the local path if saved', async () => {
+      const result = await getName({ localPath: 'a/b/myFiddle' } as any);
+      expect(result).toBe('myFiddle');
+    });
+  });
+});
diff --git a/tests/utils/get-title-spec.ts b/tests/utils/get-title-spec.ts
deleted file mode 100644
index a615448b..00000000
--- a/tests/utils/get-title-spec.ts
+++ /dev/null
@@ -1,40 +0,0 @@
-import { getName, getTitle } from '../../src/utils/get-title';
-
-describe('get-title', () => {
-  describe('getTitle()', () => {
-    it('returns a solid default title', () => {
-      const result = getTitle({} as any);
-      expect(result).toBe('Electron Fiddle - Unsaved');
-    });
-
-    it('returns a title for a Gist', () => {
-      const result = getTitle({ gistId: '123' } as any);
-      expect(result).toBe('Electron Fiddle - gist.github.com/123');
-    });
-
-    it('returns a title for a local fiddle', () => {
-      const result = getTitle({ localPath: 'a/b/fiddle' } as any);
-      expect(result).toBe('Electron Fiddle - a/b/fiddle');
-    });
-
-    it('returns a title for a local & gist fiddle', () => {
-      const result = getTitle({
-        localPath: 'a/b/fiddle',
-        gistId: '123',
-      } as any);
-      expect(result).toBe('Electron Fiddle - gist.github.com/123 a/b/fiddle');
-    });
-  });
-
-  describe('getName()', () => {
-    it('returns a random name', async () => {
-      const result = await getName({} as any);
-      expect(result).toBeTruthy();
-    });
-
-    it('returns a name from the local path if saved', async () => {
-      const result = await getName({ localPath: 'a/b/myFiddle' } as any);
-      expect(result).toBe('myFiddle');
-    });
-  });
-});