Enabled HTML-based app menu for Linux. (#2020)

microsoft · Dec 9, 2019 · 7e922ad · 7e922ad
1 parent a106274
commit 7e922ad
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 78 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - [2017](https://github.com/microsoft/BotFramework-Emulator/pull/2017)
   - [2018](https://github.com/microsoft/BotFramework-Emulator/pull/2018)
   - [2019](https://github.com/microsoft/BotFramework-Emulator/pull/2019)
+  - [2020](https://github.com/microsoft/BotFramework-Emulator/pull/2020)
   - [2021](https://github.com/microsoft/BotFramework-Emulator/pull/2021)
 
 - [main] Increased ngrok spawn timeout to 15 seconds to be more forgiving to slower networks in PR [1998](https://github.com/microsoft/BotFramework-Emulator/pull/1998)

diff --git a/packages/app/client/src/ui/shell/main.tsx b/packages/app/client/src/ui/shell/main.tsx
@@ -33,7 +33,7 @@
 
 import { Splitter } from '@bfemulator/ui-react';
 import * as React from 'react';
-import { remote } from 'electron';
+import { isLinux, isWindows } from '@bfemulator/app-shared';
 
 import * as Constants from '../../constants';
 import { Editor } from '../../state/reducers/editor';
@@ -121,7 +121,7 @@ export class Main extends React.Component<MainProps, MainState> {
 
     return (
       <>
-        {this.platform === 'win32' && <AppMenuContainer />}
+        {(isWindows() || isLinux()) && <AppMenuContainer />}
         <div className={styles.main}>
           <div className={styles.nav}>
             {!this.props.presentationModeEnabled && (

diff --git a/packages/app/main/src/appMenuBuilder.spec.ts b/packages/app/main/src/appMenuBuilder.spec.ts
@@ -333,82 +333,12 @@ describe('AppMenuBuilder', () => {
   });
 
   it('should initialize the app menu for Linux', async () => {
-    let appMenuTemplate;
-    mockBuildFromTemplate = jest.fn(template => {
-      // when trying to build from the template, pull the template out
-      // so that we can examine it and return some menu placeholder
-      appMenuTemplate = template;
-      return 'I am a menu';
-    });
-    const mockState = {
-      chat: {
-        chats: {
-          someDocId: {
-            conversationId: 'someConversationId',
-          },
-        },
-      },
-      editor: {
-        activeEditor: 'primary',
-        editors: {
-          primary: {
-            activeDocumentId: 'someDocId',
-            documents: {
-              someDocId: {
-                contentType: SharedConstants.ContentTypes.CONTENT_TYPE_LIVE_CHAT,
-              },
-            },
-          },
-        },
-      },
-    };
-    const mockRemoteCall = jest.fn(commandName => {
-      if (commandName === SharedConstants.Commands.Misc.GetStoreState) {
-        return Promise.resolve(mockState);
-      } else {
-        return Promise.resolve({});
-      }
-    });
-    jest.spyOn(commandService, 'remoteCall').mockImplementation(mockRemoteCall);
     Object.defineProperty(process, 'platform', { value: 'linux' });
-    await AppMenuBuilder.initAppMenu();
 
-    // verify that each section of the menu is the expected length
-    expect(appMenuTemplate).toHaveLength(6); // file, debug, edit, view, convo, help
-
-    const fileMenuTemplate = appMenuTemplate[0].submenu;
-    expect(fileMenuTemplate).toHaveLength(17);
-
-    // should show the currently signed in user
-    const azureSignInItem = fileMenuTemplate[9];
-    expect(azureSignInItem.label).toBe('Sign out (TheAmazingAuthLad@hotmail.com)');
-
-    // should list all available themes and selected theme (midnight) as checked
-    const themeMenu = fileMenuTemplate[12];
-    expect(themeMenu.label).toBe('Themes');
-    expect(themeMenu.submenu).toHaveLength(3); // light, dark, midnight
-    expect(themeMenu.submenu[2].type).toBe('checkbox');
-    expect(themeMenu.submenu[2].label).toBe('midnight');
-    expect(themeMenu.submenu[2].checked).toBe(true);
-
-    const debugMenuTemplate = appMenuTemplate[1].submenu;
-    expect(debugMenuTemplate).toHaveLength(1);
-
-    const editMenuTemplate = appMenuTemplate[2].submenu;
-    expect(editMenuTemplate).toHaveLength(7);
-
-    const viewMenuTemplate = appMenuTemplate[3].submenu;
-    expect(viewMenuTemplate).toHaveLength(6);
-
-    const convoMenuTemplate = appMenuTemplate[4].submenu;
-    expect(convoMenuTemplate).toHaveLength(1);
-    const sendActivityMenu = convoMenuTemplate[0].submenu;
-    expect(sendActivityMenu).toHaveLength(6);
-
-    const helpMenuTemplate = appMenuTemplate[5].submenu;
-    expect(helpMenuTemplate).toHaveLength(14);
+    const result = await AppMenuBuilder.initAppMenu();
 
-    expect(mockSetApplicationMenu).toHaveBeenCalledWith('I am a menu');
+    expect(result).toBe(undefined);
+    expect(mockSetApplicationMenu).toHaveBeenCalledWith(mockBuildFromTemplate([]));
   });
 
   it('should initialize the app menu for Mac', async () => {

diff --git a/packages/app/main/src/appMenuBuilder.ts b/packages/app/main/src/appMenuBuilder.ts
@@ -31,7 +31,7 @@
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
 
-import { BotInfo, isMac, SharedConstants, UpdateStatus } from '@bfemulator/app-shared';
+import { BotInfo, isLinux, isMac, isWindows, SharedConstants, UpdateStatus } from '@bfemulator/app-shared';
 import { CommandServiceImpl, CommandServiceInstance, ConversationService } from '@bfemulator/sdk-shared';
 import { app, clipboard, Menu, MenuItem, MenuItemConstructorOptions, shell } from 'electron';
 
@@ -72,12 +72,19 @@ export class AppMenuBuilder {
 
   /** Called on app startup */
   public static async initAppMenu(): Promise<void> {
-    // show an HTML app menu on Windows (a11y)
-    if (process.platform === 'win32') {
+    // show an HTML app menu on Windows & Linux (a11y)
+    if (isWindows()) {
       Menu.setApplicationMenu(null);
       return;
     }
 
+    // Work-around for Linux. Menu.setApplicationMenu(null) does not work on v4.x (https://github.com/electron/electron/issues/16521)
+    // Comes with the side effect of having an empty app menu bar at the top of the window.
+    if (isLinux()) {
+      Menu.setApplicationMenu(Menu.buildFromTemplate([]));
+      return;
+    }
+
     const template: MenuOpts[] = [
       await this.initFileMenu(),
       this.initDebugMenu(),

diff --git a/packages/app/shared/src/utils.spec.ts b/packages/app/shared/src/utils.spec.ts
@@ -39,6 +39,7 @@ import {
   deepCopySlow,
   isLinux,
   isMac,
+  isWindows,
   newBot,
   newEndpoint,
   getFirstBotEndpoint,
@@ -167,4 +168,14 @@ describe('isMac() and isLinux()', () => {
     Object.defineProperty(process, 'platform', { value: 'something-else' });
     expect(isLinux()).toBe(false);
   });
+
+  it('returns true when platform is windows', () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' });
+    expect(isWindows()).toBe(true);
+  });
+
+  it('returns false when platform is not windows', () => {
+    Object.defineProperty(process, 'platform', { value: 'something-else' });
+    expect(isWindows()).toBe(false);
+  });
 });
diff --git a/packages/app/shared/src/utils.ts b/packages/app/shared/src/utils.ts
@@ -139,3 +139,7 @@ export function isMac() {
 export function isLinux() {
   return process.platform === 'linux';
 }
+
+export function isWindows() {
+  return process.platform === 'win32';
+}