From 75ce3ac6a2cb998ee97c8396319c4f6f675790c3 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Thu, 22 Aug 2024 15:51:20 -0500 Subject: [PATCH] fix: menu should allow focused `BaseWindow` where possible (#43438) fix: menu should allow focused BaseWindow Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- docs/api/menu-item.md | 6 +++--- lib/browser/api/menu-item-roles.ts | 20 +++++++++++++------- lib/browser/api/menu-item.ts | 4 ++-- lib/browser/api/menu.ts | 4 ++-- spec/ts-smoke/electron/main.ts | 10 +++++----- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 3ad0c8e87cc7..0bc80b2bd37c 100644 --- 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 index 8fcc528271aa..1fa02345cb55 100644 --- 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 = { 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 = { 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 = { 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 index ae1c2ea5a2a2..ae74948064ae 100644 --- 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 index abdf50bf6d0c..48a7f475b1c1 100644 --- 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 index 2a8c13ac8750..278fef29a5b3 100644 --- a/spec/ts-smoke/electron/main.ts +++ b/spec/ts-smoke/electron/main.ts @@ -717,7 +717,7 @@ const template = [ label: 'Reload', accelerator: 'Command+R', click: (item, focusedWindow) => { - if (focusedWindow) { + if (focusedWindow instanceof BrowserWindow) { focusedWindow.webContents.reloadIgnoringCache(); } } @@ -726,7 +726,7 @@ const template = [ 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 = [ 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 = [ 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 = [ label: 'Zoom Out', accelerator: 'CmdOrCtrl+-', click: (item, focusedWindow) => { - if (focusedWindow) { + if (focusedWindow instanceof BrowserWindow) { const { webContents } = focusedWindow; webContents.zoomLevel -= 0.5; }