Repository: electron/fiddle
Pull request #598 (2021-03-06 02:06:59)

refactor: remove remote from `chrome-mac.tsx`
Ref #418

Moves the click handler for the macOS titlebar into the main process.


Patch:
diff --git a/README.md b/README.md
index 2fca018e..4b7003d6 100644
--- a/README.md
+++ b/README.md
@@ -47,11 +47,11 @@ address bar.
     </td>
     <td>
       <span>Intel Processor</span>
-      <a href="https://github.com/electron/fiddle/releases/download/v0.20.0/Electron.Fiddle-darwin-arm64-0.20.0.zip">
+      <a href="https://github.com/electron/fiddle/releases/download/v0.20.0/Electron.Fiddle-darwin-x64-0.20.0.zip">
         📦 Standalone Zip
       </a><br />
       <span>Apple Silicon Processor</span>
-      <a href="https://github.com/electron/fiddle/releases/download/v0.20.0/Electron.Fiddle-darwin-x64-0.20.0.zip">
+      <a href="https://github.com/electron/fiddle/releases/download/v0.20.0/Electron.Fiddle-darwin-arm64-0.20.0.zip">
         📦 Standalone Zip
       </a><br />
       <span>
diff --git a/src/ipc-events.ts b/src/ipc-events.ts
index c7e0166b..be7d7734 100644
--- a/src/ipc-events.ts
+++ b/src/ipc-events.ts
@@ -29,6 +29,7 @@ export enum IpcEvents {
   SET_APPDATA_DIR = 'SET_APPDATA_DIR',
   SELECT_ALL_IN_EDITOR = 'SELECT_ALL_IN_EDITOR',
   BLOCK_ACCELERATORS = 'BLOCK_ACCELERATORS',
+  CLICK_TITLEBAR_MAC = 'CLICK_TITLEBAR_MAC',
 }
 
 export const ipcMainEvents = [
@@ -40,6 +41,7 @@ export const ipcMainEvents = [
   IpcEvents.SHOW_INACTIVE,
   IpcEvents.CONFIRM_QUIT,
   IpcEvents.BLOCK_ACCELERATORS,
+  IpcEvents.CLICK_TITLEBAR_MAC,
 ];
 
 export const ipcRendererEvents = [
diff --git a/src/main/main.ts b/src/main/main.ts
index 7c3c11b3..dd80cedc 100644
--- a/src/main/main.ts
+++ b/src/main/main.ts
@@ -1,7 +1,7 @@
 import { initSentry } from '../sentry';
 initSentry();
 
-import { app } from 'electron';
+import { app, BrowserWindow, systemPreferences } from 'electron';
 
 import { IpcEvents } from '../ipc-events';
 import { isDevMode } from '../utils/devmode';
@@ -14,6 +14,7 @@ import { listenForProtocolHandler, setupProtocolHandler } from './protocol';
 import { shouldQuit } from './squirrel';
 import { setupUpdates } from './update';
 import { getOrCreateMainWindow } from './windows';
+import { IpcMainEvent } from 'electron/main';
 
 /**
  * Handle the app's "ready" event. This is essentially
@@ -36,6 +37,7 @@ export async function onReady() {
   setupUpdates();
   setupDialogs();
   setupDevTools();
+  setupTitleBarClickMac();
 }
 
 /**
@@ -57,6 +59,34 @@ export function setupMenuHandler() {
   );
 }
 
+/**
+ * On macOS, set up the custom titlebar click handler.
+ */
+export function setupTitleBarClickMac() {
+  if (process.platform !== 'darwin') {
+    return;
+  }
+
+  ipcMainManager.on(IpcEvents.CLICK_TITLEBAR_MAC, (event: IpcMainEvent) => {
+    const doubleClickAction = systemPreferences.getUserDefault(
+      'AppleActionOnDoubleClick',
+      'string',
+    );
+    const win = BrowserWindow.fromWebContents(event.sender);
+    if (win) {
+      if (doubleClickAction === 'Minimize') {
+        win.minimize();
+      } else if (doubleClickAction === 'Maximize') {
+        if (!win.isMaximized()) {
+          win.maximize();
+        } else {
+          win.unmaximize();
+        }
+      }
+    }
+  });
+}
+
 /**
  * All windows have been closed, quit on anything but
  * macOS.
diff --git a/src/renderer/components/chrome-mac.tsx b/src/renderer/components/chrome-mac.tsx
index 6664ecf3..70237e0e 100644
--- a/src/renderer/components/chrome-mac.tsx
+++ b/src/renderer/components/chrome-mac.tsx
@@ -1,8 +1,9 @@
-import { remote } from 'electron';
 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';
 
 export interface ChromeMacProps {
@@ -12,20 +13,7 @@ export interface ChromeMacProps {
 @observer
 export class ChromeMac extends React.Component<ChromeMacProps> {
   public handleDoubleClick = () => {
-    const doubleClickAction = remote.systemPreferences.getUserDefault(
-      'AppleActionOnDoubleClick',
-      'string',
-    );
-    const win = remote.getCurrentWindow();
-    if (doubleClickAction === 'Minimize') {
-      win.minimize();
-    } else if (doubleClickAction === 'Maximize') {
-      if (!win.isMaximized()) {
-        win.maximize();
-      } else {
-        win.unmaximize();
-      }
-    }
+    ipcRendererManager.send(IpcEvents.CLICK_TITLEBAR_MAC);
   };
 
   public render() {


Test patch:
diff --git a/tests/main/main-spec.ts b/tests/main/main-spec.ts
index 083815d3..963aa2be 100644
--- a/tests/main/main-spec.ts
+++ b/tests/main/main-spec.ts
@@ -2,7 +2,7 @@
  * @jest-environment node
  */
 
-import { app } from 'electron';
+import { app, BrowserWindow, systemPreferences } from 'electron';
 
 import { IpcEvents } from '../../src/ipc-events';
 import { ipcMainManager } from '../../src/main/ipc';
@@ -12,11 +12,14 @@ import {
   onReady,
   onWindowsAllClosed,
   setupMenuHandler,
+  setupTitleBarClickMac,
 } from '../../src/main/main';
 import { shouldQuit } from '../../src/main/squirrel';
 import { setupUpdates } from '../../src/main/update';
 import { getOrCreateMainWindow } from '../../src/main/windows';
 import { setupAboutPanel } from '../../src/main/about-panel';
+import { overridePlatform } from '../utils';
+import { MockBrowserWindow } from '../mocks/browser-window';
 
 jest.mock('../../src/main/windows', () => ({
   getOrCreateMainWindow: jest.fn(),
@@ -122,4 +125,107 @@ describe('main', () => {
       );
     });
   });
+
+  describe('setupTitleBarClickMac()', () => {
+    it('should do nothing on non-macOS platforms', () => {
+      overridePlatform('win32');
+      setupTitleBarClickMac();
+
+      expect(ipcMainManager.on).not.toHaveBeenCalled();
+    });
+
+    describe('on macOS', () => {
+      beforeEach(() => {
+        overridePlatform('darwin');
+        // Since ipcMainManager is mocked, we can't just .emit to trigger
+        // the event. Instead, call the callback as soon as the listener
+        // is instantiated.
+        (ipcMainManager.on as jest.Mock).mockImplementationOnce(
+          (channel, callback) => {
+            if (channel === IpcEvents.CLICK_TITLEBAR_MAC) {
+              callback({});
+            }
+          },
+        );
+      });
+
+      it('should minimize the window if AppleActionOnDoubleClick is minimize', () => {
+        const mockWindow = new MockBrowserWindow();
+        (BrowserWindow.fromWebContents as jest.Mock).mockReturnValue(
+          mockWindow,
+        );
+        (systemPreferences.getUserDefault as jest.Mock).mockReturnValue(
+          'Minimize',
+        );
+
+        setupTitleBarClickMac();
+
+        expect(mockWindow.minimize).toHaveBeenCalled();
+      });
+
+      it('should minimize the window if AppleActionOnDoubleClick is minimize', () => {
+        const mockWindow = new MockBrowserWindow();
+        (BrowserWindow.fromWebContents as jest.Mock).mockReturnValue(
+          mockWindow,
+        );
+        (systemPreferences.getUserDefault as jest.Mock).mockReturnValue(
+          'Minimize',
+        );
+
+        setupTitleBarClickMac();
+
+        expect(mockWindow.minimize).toHaveBeenCalled();
+        expect(mockWindow.maximize).not.toHaveBeenCalled();
+        expect(mockWindow.unmaximize).not.toHaveBeenCalled();
+      });
+
+      it('should maximize the window if AppleActionOnDoubleClick is maximize and the window is not maximized', () => {
+        const mockWindow = new MockBrowserWindow();
+        mockWindow.isMaximized.mockReturnValue(false);
+        (BrowserWindow.fromWebContents as jest.Mock).mockReturnValue(
+          mockWindow,
+        );
+        (systemPreferences.getUserDefault as jest.Mock).mockReturnValue(
+          'Maximize',
+        );
+
+        setupTitleBarClickMac();
+
+        expect(mockWindow.minimize).not.toHaveBeenCalled();
+        expect(mockWindow.maximize).toHaveBeenCalled();
+        expect(mockWindow.unmaximize).not.toHaveBeenCalled();
+      });
+
+      it('should unmaximize the window if AppleActionOnDoubleClick is maximize and the window is maximized', () => {
+        const mockWindow = new MockBrowserWindow();
+        mockWindow.isMaximized.mockReturnValue(true);
+        (BrowserWindow.fromWebContents as jest.Mock).mockReturnValue(
+          mockWindow,
+        );
+        (systemPreferences.getUserDefault as jest.Mock).mockReturnValue(
+          'Maximize',
+        );
+
+        setupTitleBarClickMac();
+
+        expect(mockWindow.minimize).not.toHaveBeenCalled();
+        expect(mockWindow.maximize).not.toHaveBeenCalled();
+        expect(mockWindow.unmaximize).toHaveBeenCalled();
+      });
+
+      it('should do nothing if AppleActionOnDoubleClick is an unknown value', () => {
+        const mockWindow = new MockBrowserWindow();
+        (BrowserWindow.fromWebContents as jest.Mock).mockReturnValue(
+          mockWindow,
+        );
+        (systemPreferences.getUserDefault as jest.Mock).mockReturnValue(
+          undefined,
+        );
+        setupTitleBarClickMac();
+        expect(mockWindow.minimize).not.toHaveBeenCalled();
+        expect(mockWindow.maximize).not.toHaveBeenCalled();
+        expect(mockWindow.unmaximize).not.toHaveBeenCalled();
+      });
+    });
+  });
 });
diff --git a/tests/mocks/electron.ts b/tests/mocks/electron.ts
index b03bc7d1..e9ada0fa 100644
--- a/tests/mocks/electron.ts
+++ b/tests/mocks/electron.ts
@@ -221,6 +221,7 @@ const electronMock = {
 
 electronMock.BrowserWindow.getAllWindows.mockReturnValue([]);
 electronMock.BrowserWindow.fromId.mockReturnValue(mainWindowStub);
+electronMock.BrowserWindow.fromWebContents.mockReturnValue(mainWindowStub);
 electronMock.BrowserWindow.getFocusedWindow.mockReturnValue(focusedWindowStub);
 electronMock.remote.getCurrentWindow.mockReturnValue(mainWindowStub);
 
diff --git a/tests/renderer/binary-spec.ts b/tests/renderer/binary-spec.ts
index dc466b5b..36b34b88 100644
--- a/tests/renderer/binary-spec.ts
+++ b/tests/renderer/binary-spec.ts
@@ -162,7 +162,7 @@ describe('binary', () => {
     });
 
     it('throws on other platforms', () => {
-      overridePlatform('bleepbloop');
+      overridePlatform('android');
 
       expect(() => getElectronBinaryPath('v3.0.0')).toThrow();
     });
diff --git a/tests/renderer/components/chrome-mac-spec.tsx b/tests/renderer/components/chrome-mac-spec.tsx
index 69b0e30e..14e11c89 100644
--- a/tests/renderer/components/chrome-mac-spec.tsx
+++ b/tests/renderer/components/chrome-mac-spec.tsx
@@ -1,9 +1,9 @@
-import * as electron from 'electron';
 import { mount, shallow } from 'enzyme';
 import * as React from 'react';
+import { IpcEvents } from '../../../src/ipc-events';
 
 import { ChromeMac } from '../../../src/renderer/components/chrome-mac';
-import { MockBrowserWindow } from '../../mocks/browser-window';
+import { ipcRendererManager } from '../../../src/renderer/ipc';
 import { overridePlatform, resetPlatform } from '../../utils';
 
 describe('Chrome-Mac component', () => {
@@ -26,86 +26,15 @@ describe('Chrome-Mac component', () => {
   });
 
   describe('handleDoubleClick', () => {
-    it('should minimize the window if AppleActionOnDoubleClick is minimize', () => {
+    it('should send a message to the main process', () => {
       const wrapper = mount(<ChromeMac appState={store} />);
       const chrome = wrapper.instance() as ChromeMac;
-      const fakeWindow = new MockBrowserWindow();
-      (electron.remote.systemPreferences
-        .getUserDefault as jest.Mock).mockReturnValue('Minimize');
-      (electron.remote.getCurrentWindow as jest.Mock).mockReturnValue(
-        fakeWindow,
-      );
+      ipcRendererManager.send = jest.fn();
 
       chrome.handleDoubleClick();
-
-      expect(
-        electron.remote.systemPreferences.getUserDefault,
-      ).toHaveBeenCalled();
-      expect(fakeWindow.minimize).toHaveBeenCalled();
-      expect(fakeWindow.maximize).not.toHaveBeenCalled();
-      expect(fakeWindow.unmaximize).not.toHaveBeenCalled();
-    });
-
-    it('should maximize the window if AppleActionOnDoubleClick is maximize and the window is not maximized', () => {
-      const wrapper = mount(<ChromeMac appState={store} />);
-      const chrome = wrapper.instance() as ChromeMac;
-      const fakeWindow = new MockBrowserWindow();
-      (electron.remote.systemPreferences
-        .getUserDefault as jest.Mock).mockReturnValue('Maximize');
-      (electron.remote.getCurrentWindow as jest.Mock).mockReturnValue(
-        fakeWindow,
+      expect(ipcRendererManager.send).toHaveBeenCalledWith(
+        IpcEvents.CLICK_TITLEBAR_MAC,
       );
-      fakeWindow.isMaximized.mockReturnValue(false);
-
-      chrome.handleDoubleClick();
-
-      expect(
-        electron.remote.systemPreferences.getUserDefault,
-      ).toHaveBeenCalled();
-      expect(fakeWindow.minimize).not.toHaveBeenCalled();
-      expect(fakeWindow.maximize).toHaveBeenCalled();
-      expect(fakeWindow.unmaximize).not.toHaveBeenCalled();
-    });
-
-    it('should unmaximize the window if AppleActionOnDoubleClick is maximize and the window is maximized', () => {
-      const wrapper = mount(<ChromeMac appState={store} />);
-      const chrome = wrapper.instance() as ChromeMac;
-      const fakeWindow = new MockBrowserWindow();
-      (electron.remote.systemPreferences
-        .getUserDefault as jest.Mock).mockReturnValue('Maximize');
-      (electron.remote.getCurrentWindow as jest.Mock).mockReturnValue(
-        fakeWindow,
-      );
-      fakeWindow.isMaximized.mockReturnValue(true);
-
-      chrome.handleDoubleClick();
-
-      expect(
-        electron.remote.systemPreferences.getUserDefault,
-      ).toHaveBeenCalled();
-      expect(fakeWindow.minimize).not.toHaveBeenCalled();
-      expect(fakeWindow.maximize).not.toHaveBeenCalled();
-      expect(fakeWindow.unmaximize).toHaveBeenCalled();
-    });
-
-    it('should do nothingif AppleActionOnDoubleClick is an unknown value', () => {
-      const wrapper = mount(<ChromeMac appState={store} />);
-      const chrome = wrapper.instance() as ChromeMac;
-      const fakeWindow = new MockBrowserWindow();
-      (electron.remote.systemPreferences
-        .getUserDefault as jest.Mock).mockReturnValue('Nonsense');
-      (electron.remote.getCurrentWindow as jest.Mock).mockReturnValue(
-        fakeWindow,
-      );
-
-      chrome.handleDoubleClick();
-
-      expect(
-        electron.remote.systemPreferences.getUserDefault,
-      ).toHaveBeenCalled();
-      expect(fakeWindow.minimize).not.toHaveBeenCalled();
-      expect(fakeWindow.maximize).not.toHaveBeenCalled();
-      expect(fakeWindow.unmaximize).not.toHaveBeenCalled();
     });
   });
 });
diff --git a/tests/utils.ts b/tests/utils.ts
index e5c97463..faca7d57 100644
--- a/tests/utils.ts
+++ b/tests/utils.ts
@@ -1,6 +1,6 @@
 const platform = process.platform;
 
-export function overridePlatform(value: string) {
+export function overridePlatform(value: NodeJS.Platform) {
   Object.defineProperty(process, 'platform', {
     value,
     writable: true,
diff --git a/tests/utils/electron-name-spec.ts b/tests/utils/electron-name-spec.ts
index 8cab96f8..f1815257 100644
--- a/tests/utils/electron-name-spec.ts
+++ b/tests/utils/electron-name-spec.ts
@@ -7,11 +7,13 @@ describe('electron-name', () => {
   });
 
   it('returns the right name for each platform', () => {
-    [
+    const platforms: Array<{ platform: NodeJS.Platform; expected: string }> = [
       { platform: 'win32', expected: 'electron.exe' },
       { platform: 'darwin', expected: 'Electron.app' },
       { platform: 'linux', expected: 'electron' },
-    ].forEach(({ platform, expected }) => {
+    ];
+
+    platforms.forEach(({ platform, expected }) => {
       overridePlatform(platform);
       expect(getElectronNameForPlatform()).toBe(expected);
     });