fix: menu should allow focused BaseWindow where possible (#43437)

trop[bot] · codebytere · web-flow · commit 204011c3f2bf · 2024-08-22T18:33:23.000-05:00
fix: menu should allow focused BaseWindow

Co-authored-by: trop[bot] &lt;37223003+trop[bot]@users.noreply.github.com&gt;
Co-authored-by: Shelley Vohr &lt;shelley.vohr@gmail.com&gt;
diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md
@@ -10,9 +10,9 @@ See [`Menu`](menu.md) for examples.
 
 * `options` Object
   * `click` Function (optional) - Will be called with
-    `click(menuItem, browserWindow, event)` when the menu item is clicked.
+    `click(menuItem, window, event)` when the menu item is clicked.
     * `menuItem` MenuItem
-    * `browserWindow` [BrowserWindow](browser-window.md) | undefined - This will not be defined if no window is open.
+    * `window` [BaseWindow](base-window.md) | undefined - This will not be defined if no window is open.
     * `event` [KeyboardEvent](structures/keyboard-event.md)
   * `role` string (optional) - Can be `undo`, `redo`, `cut`, `copy`, `paste`, `pasteAndMatchStyle`, `delete`, `selectAll`, `reload`, `forceReload`, `toggleDevTools`, `resetZoom`, `zoomIn`, `zoomOut`, `toggleSpellChecker`, `togglefullscreen`, `window`, `minimize`, `close`, `help`, `about`, `services`, `hide`, `hideOthers`, `unhide`, `quit`, `showSubstitutions`, `toggleSmartQuotes`, `toggleSmartDashes`, `toggleTextReplacement`, `startSpeaking`, `stopSpeaking`, `zoom`, `front`, `appMenu`, `fileMenu`, `editMenu`, `viewMenu`, `shareMenu`, `recentDocuments`, `toggleTabBar`, `selectNextTab`, `selectPreviousTab`, `showAllTabs`, `mergeAllWindows`, `clearRecentDocuments`, `moveTabToNewWindow` or `windowMenu` - Define the action of the menu item, when specified the
     `click` property will be ignored. See [roles](#roles).
@@ -146,7 +146,7 @@ A `Function` that is fired when the MenuItem receives a click event.
 It can be called with `menuItem.click(event, focusedWindow, focusedWebContents)`.
 
 * `event` [KeyboardEvent](structures/keyboard-event.md)
-* `focusedWindow` [BrowserWindow](browser-window.md)
+* `focusedWindow` [BaseWindow](browser-window.md)
 * `focusedWebContents` [WebContents](web-contents.md)
 
 #### `menuItem.submenu`
diff --git a/lib/browser/api/menu-item-roles.ts b/lib/browser/api/menu-item-roles.ts
@@ -1,4 +1,4 @@
-import { app, BrowserWindow, session, webContents, WebContents, MenuItemConstructorOptions } from 'electron/main';
+import { app, BaseWindow, BrowserWindow, session, webContents, WebContents, MenuItemConstructorOptions } from 'electron/main';
 
 const isMac = process.platform === 'darwin';
 const isWindows = process.platform === 'win32';
@@ -13,7 +13,7 @@ interface Role {
   label: string;
   accelerator?: string;
   checked?: boolean;
-  windowMethod?: ((window: BrowserWindow) => void);
+  windowMethod?: ((window: BaseWindow) => void);
   webContentsMethod?: ((webContents: WebContents) => void);
   appMethod?: () => void;
   registerAccelerator?: boolean;
@@ -53,8 +53,10 @@ export const roleList: Record<RoleId, Role> = {
     label: 'Force Reload',
     accelerator: 'Shift+CmdOrCtrl+R',
     nonNativeMacOSRole: true,
-    windowMethod: (window: BrowserWindow) => {
-      window.webContents.reloadIgnoringCache();
+    windowMethod: (window: BaseWindow) => {
+      if (window instanceof BrowserWindow) {
+        window.webContents.reloadIgnoringCache();
+      }
     }
   },
   front: {
@@ -110,7 +112,11 @@ export const roleList: Record<RoleId, Role> = {
     label: 'Reload',
     accelerator: 'CmdOrCtrl+R',
     nonNativeMacOSRole: true,
-    windowMethod: w => w.reload()
+    windowMethod: (w: BaseWindow) => {
+      if (w instanceof BrowserWindow) {
+        w.reload();
+      }
+    }
   },
   resetzoom: {
     label: 'Actual Size',
@@ -164,7 +170,7 @@ export const roleList: Record<RoleId, Role> = {
   togglefullscreen: {
     label: 'Toggle Full Screen',
     accelerator: isMac ? 'Control+Command+F' : 'F11',
-    windowMethod: (window: BrowserWindow) => {
+    windowMethod: (window: BaseWindow) => {
       window.setFullScreen(!window.isFullScreen());
     }
   },
@@ -361,7 +367,7 @@ export function getDefaultSubmenu (role: RoleId) {
   return submenu;
 }
 
-export function execute (role: RoleId, focusedWindow: BrowserWindow, focusedWebContents: WebContents) {
+export function execute (role: RoleId, focusedWindow: BaseWindow, focusedWebContents: WebContents) {
   if (!canExecuteRole(role)) return false;
 
   const { appMethod, webContentsMethod, windowMethod } = roleList[role];
diff --git a/lib/browser/api/menu-item.ts b/lib/browser/api/menu-item.ts
@@ -1,5 +1,5 @@
 import * as roles from '@electron/internal/browser/api/menu-item-roles';
-import { Menu, BrowserWindow, WebContents, KeyboardEvent } from 'electron/main';
+import { Menu, BaseWindow, WebContents, KeyboardEvent } from 'electron/main';
 
 let nextCommandId = 0;
 
@@ -53,7 +53,7 @@ const MenuItem = function (this: any, options: any) {
   });
 
   const click = options.click;
-  this.click = (event: KeyboardEvent, focusedWindow: BrowserWindow, focusedWebContents: WebContents) => {
+  this.click = (event: KeyboardEvent, focusedWindow: BaseWindow, focusedWebContents: WebContents) => {
     // Manually flip the checked flags when clicked.
     if (!roles.shouldOverrideCheckStatus(this.role) &&
         (this.type === 'checkbox' || this.type === 'radio')) {
diff --git a/lib/browser/api/menu.ts b/lib/browser/api/menu.ts
@@ -1,4 +1,4 @@
-import { BaseWindow, MenuItem, webContents, Menu as MenuType, BrowserWindow, MenuItemConstructorOptions } from 'electron/main';
+import { BaseWindow, MenuItem, webContents, Menu as MenuType, MenuItemConstructorOptions } from 'electron/main';
 import { sortMenuItems } from '@electron/internal/browser/api/menu-utils';
 import { setApplicationMenuWasSet } from '@electron/internal/browser/default-menu';
 
@@ -54,7 +54,7 @@ Menu.prototype._executeCommand = function (event, id) {
   const command = this.commandsMap[id];
   if (!command) return;
   const focusedWindow = BaseWindow.getFocusedWindow();
-  command.click(event, focusedWindow instanceof BrowserWindow ? focusedWindow : undefined, webContents.getFocusedWebContents());
+  command.click(event, focusedWindow, webContents.getFocusedWebContents());
 };
 
 Menu.prototype._menuWillShow = function () {
diff --git a/spec/ts-smoke/electron/main.ts b/spec/ts-smoke/electron/main.ts
@@ -717,7 +717,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         label: 'Reload',
         accelerator: 'Command+R',
         click: (item, focusedWindow) => {
-          if (focusedWindow) {
+          if (focusedWindow instanceof BrowserWindow) {
             focusedWindow.webContents.reloadIgnoringCache();
           }
         }
@@ -726,7 +726,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         label: 'Toggle DevTools',
         accelerator: 'Alt+Command+I',
         click: (item, focusedWindow) => {
-          if (focusedWindow) {
+          if (focusedWindow instanceof BrowserWindow) {
             focusedWindow.webContents.toggleDevTools();
           }
         }
@@ -738,7 +738,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         label: 'Actual Size',
         accelerator: 'CmdOrCtrl+0',
         click: (item, focusedWindow) => {
-          if (focusedWindow) {
+          if (focusedWindow instanceof BrowserWindow) {
             focusedWindow.webContents.zoomLevel = 0;
           }
         }
@@ -747,7 +747,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         label: 'Zoom In',
         accelerator: 'CmdOrCtrl+Plus',
         click: (item, focusedWindow) => {
-          if (focusedWindow) {
+          if (focusedWindow instanceof BrowserWindow) {
             const { webContents } = focusedWindow;
             webContents.zoomLevel += 0.5;
           }
@@ -757,7 +757,7 @@ const template = <Electron.MenuItemConstructorOptions[]> [
         label: 'Zoom Out',
         accelerator: 'CmdOrCtrl+-',
         click: (item, focusedWindow) => {
-          if (focusedWindow) {
+          if (focusedWindow instanceof BrowserWindow) {
             const { webContents } = focusedWindow;
             webContents.zoomLevel -= 0.5;
           }
