fix(menu): remove accelerators for linux COMPASS-8494 (#6485)

mabaasit · web-flow · commit 72d1802cbdb0 · 2024-11-18T09:22:21.000+01:00
* remove accelerators for linux menu

* fix tests

* add todo ticket

* add test

* filter more accelerators for linux

* compass menu accelerator assertions

* skip tests on linux

* use actual menu for tests
diff --git a/packages/compass/src/main/menu.spec.ts b/packages/compass/src/main/menu.spec.ts
@@ -3,6 +3,7 @@ import type { MenuItemConstructorOptions } from 'electron';
 import { BrowserWindow, ipcMain, Menu, app, dialog } from 'electron';
 import { expect } from 'chai';
 import sinon from 'sinon';
+import os from 'os';
 import { createSandboxFromDefaultPreferences } from 'compass-preferences-model';
 
 import type { CompassApplication } from './application';
@@ -457,7 +458,8 @@ describe('CompassMenu', function () {
       ]);
     });
 
-    ['linux', 'win32'].forEach((platform) => {
+    // TODO(COMPASS-8505): Add `linux` back to this list
+    ['win32'].forEach((platform) => {
       // TODO(COMPASS-7906): remove
       it.skip(`[single-connection] should generate a menu template for ${platform}`, function () {
         sinon.stub(process, 'platform').value(platform);
@@ -588,6 +590,87 @@ describe('CompassMenu', function () {
       });
     });
 
+    // TODO(COMPASS-8505): Remove this test
+    it('should generate a menu template for linux', async function () {
+      await App.preferences.savePreferences({
+        enableMultipleConnectionSystem: true,
+      });
+      sinon.stub(process, 'platform').value('linux');
+
+      expect(serializable(CompassMenu.getTemplate(0))).to.deep.equal([
+        {
+          label: '&Connections',
+          submenu: [
+            { label: '&Import Saved Connections' },
+            { label: '&Export Saved Connections' },
+            { type: 'separator' },
+            { label: 'E&xit' },
+          ],
+        },
+        {
+          label: 'Edit',
+          submenu: [
+            { label: 'Undo', role: 'undo' },
+            { label: 'Redo', role: 'redo' },
+            { type: 'separator' },
+            { label: 'Cut', role: 'cut' },
+            { label: 'Copy', role: 'copy' },
+            { label: 'Paste', role: 'paste' },
+            {
+              label: 'Select All',
+              role: 'selectAll',
+            },
+            { type: 'separator' },
+            { label: 'Find' },
+            { type: 'separator' },
+            { label: '&Settings' },
+          ],
+        },
+        {
+          label: '&View',
+          submenu: [
+            { label: '&Reload' },
+            { label: '&Reload Data' },
+            { type: 'separator' },
+            { label: 'Actual Size' },
+            { label: 'Zoom In' },
+            { label: 'Zoom Out' },
+          ],
+        },
+        {
+          label: '&Help',
+          submenu: [
+            { label: `&Online ${app.getName()} Help` },
+            { label: '&License' },
+            { label: `&View Source Code on GitHub` },
+            { label: `&Suggest a Feature` },
+            { label: `&Report a Bug` },
+            { label: '&Open Log File' },
+            { type: 'separator' },
+            { label: `&About ${app.getName()}` },
+            { label: 'Check for updates…' },
+          ],
+        },
+      ]);
+    });
+
+    it('does not crash when rendering menu item with an accelerator', () => {
+      const window = new BrowserWindow({ show: false });
+      const template = CompassMenu.getTemplate(window.id);
+
+      // As the root menu items do not have accelerators, we test
+      // against each item's submenu.
+      for (const item of template) {
+        // for TS. compass menu has submenus
+        if (!Array.isArray(item.submenu)) {
+          continue;
+        }
+        const menu = Menu.buildFromTemplate(item.submenu);
+        menu.popup({ window });
+        menu.closePopup();
+      }
+    });
+
     it('should generate a menu template without collection submenu if `showCollection` is `false`', function () {
       expect(
         CompassMenu.getTemplate(0).find((item) => item.label === '&Collection')
@@ -612,7 +695,12 @@ describe('CompassMenu', function () {
         label: '&Collection',
         submenu: [
           {
-            accelerator: 'Alt+CmdOrCtrl+S',
+            // TODO(COMPASS-8505): Add `accelerator` back to this
+            ...(os.platform() === 'linux'
+              ? {}
+              : {
+                  accelerator: 'Alt+CmdOrCtrl+S',
+                }),
             label: '&Share Schema as JSON',
           },
           {
@@ -646,7 +734,12 @@ describe('CompassMenu', function () {
         label: '&Collection',
         submenu: [
           {
-            accelerator: 'Alt+CmdOrCtrl+S',
+            // TODO(COMPASS-8505): Add `accelerator` back to this
+            ...(os.platform() === 'linux'
+              ? {}
+              : {
+                  accelerator: 'Alt+CmdOrCtrl+S',
+                }),
             label: '&Share Schema as JSON',
           },
           {
@@ -670,7 +763,12 @@ describe('CompassMenu', function () {
       expect(
         menu.find((item: any) => item.label === '&Toggle DevTools')
       ).to.deep.eq({
-        accelerator: 'Alt+CmdOrCtrl+I',
+        // TODO(COMPASS-8505): Add `accelerator` back to this
+        ...(os.platform() === 'linux'
+          ? {}
+          : {
+              accelerator: 'Alt+CmdOrCtrl+I',
+            }),
         label: '&Toggle DevTools',
       });
     });
diff --git a/packages/compass/src/main/menu.ts b/packages/compass/src/main/menu.ts
@@ -1,7 +1,14 @@
 import type { MenuItemConstructorOptions } from 'electron';
-import { BrowserWindow, Menu, app, dialog, shell } from 'electron';
+import {
+  BrowserWindow,
+  Menu,
+  app as electronApp,
+  dialog,
+  shell,
+} from 'electron';
 import { ipcMain } from 'hadron-ipc';
 import fs from 'fs';
+import os from 'os';
 import path from 'path';
 import createDebug from 'debug';
 import type { THEMES } from 'compass-preferences-model';
@@ -34,11 +41,11 @@ function quitItem(
     accelerator: 'CmdOrCtrl+Q',
     click() {
       !compassApp.preferences.getPreferences().enableShowDialogOnQuit
-        ? app.quit()
+        ? electronApp.quit()
         : void dialog
             .showMessageBox({
               type: 'warning',
-              title: `Quit ${app.getName()}`,
+              title: `Quit ${electronApp.getName()}`,
               icon: COMPASS_ICON,
               message: 'Are you sure you want to quit?',
               buttons: ['Quit', 'Cancel'],
@@ -50,7 +57,7 @@ function quitItem(
                   void compassApp.preferences.savePreferences({
                     enableShowDialogOnQuit: false,
                   });
-                app.quit();
+                electronApp.quit();
               }
             });
     },
@@ -96,10 +103,10 @@ function darwinCompassSubMenu(
   compassApp: typeof CompassApplication
 ): MenuItemConstructorOptions {
   return {
-    label: app.getName(),
+    label: electronApp.getName(),
     submenu: [
       {
-        label: `About ${app.getName()}`,
+        label: `About ${electronApp.getName()}`,
         role: 'about',
       },
       updateSubmenu(windowState, compassApp),
@@ -239,14 +246,14 @@ function editSubMenu(): MenuItemConstructorOptions {
 
 function nonDarwinAboutItem(): MenuItemConstructorOptions {
   return {
-    label: `&About ${app.getName()}`,
+    label: `&About ${electronApp.getName()}`,
     click() {
       void dialog.showMessageBox({
         type: 'info',
-        title: 'About ' + app.getName(),
+        title: 'About ' + electronApp.getName(),
         icon: COMPASS_ICON,
-        message: app.getName(),
-        detail: 'Version ' + app.getVersion(),
+        message: electronApp.getName(),
+        detail: 'Version ' + electronApp.getVersion(),
         buttons: ['OK'],
       });
     },
@@ -255,7 +262,7 @@ function nonDarwinAboutItem(): MenuItemConstructorOptions {
 
 function helpWindowItem(): MenuItemConstructorOptions {
   return {
-    label: `&Online ${app.getName()} Help`,
+    label: `&Online ${electronApp.getName()} Help`,
     accelerator: 'F1',
     click() {
       void shell.openExternal(COMPASS_HELP);
@@ -299,7 +306,7 @@ function license(): MenuItemConstructorOptions {
     label: '&License',
     click() {
       void import('../../LICENSE').then(({ default: LICENSE }) => {
-        const licenseTemp = path.join(app.getPath('temp'), 'License');
+        const licenseTemp = path.join(electronApp.getPath('temp'), 'License');
         fs.writeFile(licenseTemp, LICENSE, (err) => {
           if (!err) {
             void shell.openPath(licenseTemp);
@@ -513,6 +520,24 @@ class WindowMenuState {
   updateManagerState: UpdateManagerState = 'idle';
 }
 
+function removeAcceleratorFromMenu(
+  menu?: MenuItemConstructorOptions[]
+): MenuItemConstructorOptions[] {
+  if (!Array.isArray(menu)) {
+    return [];
+  }
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  return menu.map(({ accelerator, ...item }) => {
+    if (!item.submenu || !Array.isArray(item.submenu)) {
+      return item;
+    }
+    return {
+      ...item,
+      submenu: removeAcceleratorFromMenu(item.submenu),
+    };
+  });
+}
+
 class CompassMenu {
   private constructor() {
     // marking constructor as private to disallow usage
@@ -618,9 +643,9 @@ class CompassMenu {
   }
 
   private static async setupDockMenu() {
-    await app.whenReady();
+    await electronApp.whenReady();
     if (process.platform === 'darwin') {
-      app.dock.setMenu(
+      electronApp.dock.setMenu(
         Menu.buildFromTemplate([
           {
             label: 'New Window',
@@ -686,7 +711,12 @@ class CompassMenu {
     if (process.platform === 'darwin') {
       return darwinMenu(menuState, this.app);
     }
-    return nonDarwinMenu(menuState, this.app);
+    const menu = nonDarwinMenu(menuState, this.app);
+    // TODO(COMPASS-8505): Remove this check once accelerator issue is resolve for linux.
+    if (os.platform() === 'linux') {
+      return removeAcceleratorFromMenu(menu);
+    }
+    return menu;
   }
 
   private static refreshMenu = () => {
