Repository: electron/fiddle
Pull request #624 (2021-03-27 07:56:38)

fix: inconsistent sorting of editor windows
I thought Fiddle hacking might help with some insomnia, but apparently not :slightly_smiling_face: 

The bug: On startup, the editor windows are sorted one way. Then if you hit 'New Fiddle' the preload and html editors will swap places. The root cause is that constants.ts's DEFAULT_MOSAIC_ARRANGEMENT and the 'order' array in app.tsx's replaceFiddle() do not have the same order.  Looks like I [introduced](https://github.com/electron/fiddle/commit/19acc0b1279b7aedb4e51a939bc49609fa4cebe1#diff-6fd15f0cf5192de646d9ba7274a882bb6c33406232567e8949e06f372320629bL21) this regression a couple of weeks ago when adding preload to DEFAULT_MOSAIC_ARRANGEMENT for v12.0.0.

This PR builds DEFAULT_MOSAIC_ARRANGEMENT from the sorted array so they will always match.

*edit:*  There are no new uncovered LOC or branches. coveralls dropped because this PR removes more code than it adds.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/38303646/badge)](https://coveralls.io/builds/38303646)

Coverage decreased (-0.003%) to 93.339% when pulling **329a99b047cb9726d80b5473754ec096fa0f592b on fix/inconsistent-editor-sorting** into **c6edb73f35b4ea06cc6adcac075acf7953ad4581 on master**.


Patch:
diff --git a/src/renderer/app.tsx b/src/renderer/app.tsx
index 3b6b39cd..62e3b2a0 100644
--- a/src/renderer/app.tsx
+++ b/src/renderer/app.tsx
@@ -18,7 +18,7 @@ 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 } from './constants';
+import { EMPTY_EDITOR_CONTENT, SORTED_EDITORS } from './constants';
 import { FileManager } from './file-manager';
 import { RemoteLoader } from './remote-loader';
 import { Runner } from './runner';
@@ -64,33 +64,16 @@ export class App {
       }
     }
 
-    // display all editors that have content
-    const visibleEditors = [];
-
-    // ensure consistent sorting of mosaics
-    const sortedEditors = Object.keys(editorValues).sort((a, b) => {
-      const order: Array<string> = [
-        'main',
-        'renderer',
-        'html',
-        'preload',
-        'css',
-      ];
-
-      return order.indexOf(a) - order.indexOf(b);
-    });
-
-    for (const id of sortedEditors) {
-      const content = editorValues[id];
-
-      // if the gist content is empty or matches the empty file output, don't show it
-      if (
-        content.length > 0 &&
-        !Object.values(EMPTY_EDITOR_CONTENT).includes(content)
-      ) {
-        visibleEditors.push(id as EditorId);
-      }
-    }
+    // if the gist content is empty or matches the empty file output, don't show it
+    const EMPTIES = Object.values(EMPTY_EDITOR_CONTENT);
+    const shouldShowContent = (content?: string) =>
+      content && content.length > 0 && !EMPTIES.includes(content);
+
+    // sort and display all editors that have content
+    const visibleEditors: EditorId[] = Object.entries(editorValues)
+      .filter(([_id, content]) => shouldShowContent(content))
+      .map(([id]) => id as EditorId)
+      .sort((a, b) => SORTED_EDITORS.indexOf(a) - SORTED_EDITORS.indexOf(b));
 
     this.state.gistId = gistId || '';
     this.state.localPath = filePath;
diff --git a/src/renderer/constants.ts b/src/renderer/constants.ts
index e21488f8..fb5aa4e6 100644
--- a/src/renderer/constants.ts
+++ b/src/renderer/constants.ts
@@ -10,17 +10,25 @@ export const CONFIG_PATH = path.join(
   '.electron-fiddle',
 );
 
+export const SORTED_EDITORS = Object.freeze([
+  EditorId.main,
+  EditorId.renderer,
+  EditorId.html,
+  EditorId.preload,
+  EditorId.css,
+]);
+
 export const DEFAULT_MOSAIC_ARRANGEMENT: MosaicNode<MosaicId> = {
   direction: 'row',
   first: {
     direction: 'column',
-    first: EditorId.main,
-    second: EditorId.renderer,
+    first: SORTED_EDITORS[0],
+    second: SORTED_EDITORS[1],
   },
   second: {
     direction: 'column',
-    first: EditorId.preload,
-    second: EditorId.html,
+    first: SORTED_EDITORS[2],
+    second: SORTED_EDITORS[3],
   },
 };
 
diff --git a/src/shared-constants.ts b/src/shared-constants.ts
index 2181d0f0..3bbc7fe7 100644
--- a/src/shared-constants.ts
+++ b/src/shared-constants.ts
@@ -1,3 +1,5 @@
+import { EditorId } from './interfaces';
+
 export const INDEX_HTML_NAME = 'index.html';
 export const MAIN_JS_NAME = 'main.js';
 export const RENDERER_JS_NAME = 'renderer.js';
@@ -6,9 +8,9 @@ export const STYLES_CSS_NAME = 'styles.css';
 export const PACKAGE_NAME = 'package.json';
 
 export const FILENAME_KEYS = Object.freeze({
-  [INDEX_HTML_NAME]: 'html',
-  [MAIN_JS_NAME]: 'main',
-  [PRELOAD_JS_NAME]: 'preload',
-  [RENDERER_JS_NAME]: 'renderer',
-  [STYLES_CSS_NAME]: 'css',
+  [INDEX_HTML_NAME]: EditorId.html,
+  [MAIN_JS_NAME]: EditorId.main,
+  [PRELOAD_JS_NAME]: EditorId.preload,
+  [RENDERER_JS_NAME]: EditorId.renderer,
+  [STYLES_CSS_NAME]: EditorId.css,
 });


Test patch:
diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts
index 79791803..bf9ac40b 100644
--- a/tests/renderer/state-spec.ts
+++ b/tests/renderer/state-spec.ts
@@ -17,7 +17,10 @@ import {
   setupBinary,
 } from '../../src/renderer/binary';
 import { Bisector } from '../../src/renderer/bisect';
-import { DEFAULT_MOSAIC_ARRANGEMENT } from '../../src/renderer/constants';
+import {
+  DEFAULT_MOSAIC_ARRANGEMENT,
+  SORTED_EDITORS,
+} from '../../src/renderer/constants';
 import { getTemplate, isContentUnchanged } from '../../src/renderer/content';
 import { ipcRendererManager } from '../../src/renderer/ipc';
 import { AppState } from '../../src/renderer/state';
@@ -603,15 +606,15 @@ describe('AppState', () => {
     it('hides a given editor and creates a backup', () => {
       appState.mosaicArrangement = DEFAULT_MOSAIC_ARRANGEMENT;
       appState.closedPanels = {};
-      appState.hideAndBackupMosaic(EditorId.main);
+      appState.hideAndBackupMosaic(SORTED_EDITORS[0]);
 
       expect(appState.mosaicArrangement).toEqual({
         direction: 'row',
-        first: EditorId.renderer,
+        first: SORTED_EDITORS[1],
         second: {
           direction: 'column',
-          first: EditorId.preload,
-          second: EditorId.html,
+          first: SORTED_EDITORS[2],
+          second: SORTED_EDITORS[3],
         },
       });
       expect(appState.closedPanels[EditorId.main]).toBeTruthy();
@@ -635,15 +638,15 @@ describe('AppState', () => {
 
   describe('resetEditorLayout()', () => {
     it('Puts editors in default arrangement', () => {
-      appState.hideAndBackupMosaic(EditorId.main);
+      appState.hideAndBackupMosaic(SORTED_EDITORS[0]);
 
       expect(appState.mosaicArrangement).toEqual({
         direction: 'row',
-        first: EditorId.renderer,
+        first: SORTED_EDITORS[1],
         second: {
           direction: 'column',
-          first: EditorId.preload,
-          second: EditorId.html,
+          first: SORTED_EDITORS[2],
+          second: SORTED_EDITORS[3],
         },
       });
 
diff --git a/tests/utils/editors-mosaic-arrangement-spec.ts b/tests/utils/editors-mosaic-arrangement-spec.ts
index cd9225d1..90651118 100644
--- a/tests/utils/editors-mosaic-arrangement-spec.ts
+++ b/tests/utils/editors-mosaic-arrangement-spec.ts
@@ -1,5 +1,8 @@
 import { EditorId } from '../../src/interfaces';
-import { DEFAULT_MOSAIC_ARRANGEMENT } from '../../src/renderer/constants';
+import {
+  DEFAULT_MOSAIC_ARRANGEMENT,
+  SORTED_EDITORS,
+} from '../../src/renderer/constants';
 import {
   createMosaicArrangement,
   getVisibleMosaics,
@@ -27,12 +30,7 @@ describe('Mosaic Arrangement Utilities', () => {
     });
 
     it('creates the correct arrangement for the default visible panels', () => {
-      const result = createMosaicArrangement([
-        EditorId.main,
-        EditorId.renderer,
-        EditorId.preload,
-        EditorId.html,
-      ]);
+      const result = createMosaicArrangement(SORTED_EDITORS.slice(0, 4));
 
       expect(result).toEqual(DEFAULT_MOSAIC_ARRANGEMENT);
     });
@@ -64,7 +62,7 @@ describe('Mosaic Arrangement Utilities', () => {
     it('returns the correct array for the default mosaic', () => {
       const result = getVisibleMosaics(DEFAULT_MOSAIC_ARRANGEMENT);
 
-      expect(result).toEqual(['main', 'renderer', 'preload', 'html']);
+      expect(result).toEqual(SORTED_EDITORS.slice(0, 4));
     });
   });
 });