From 6d3d366d4545a721f2cac46a7bbfa2fc46448e23 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 25 Apr 2026 21:53:32 -0700 Subject: [PATCH] feat(menu): show File/Edit/View/Window menu bar on Windows & Linux (#1111) Co-authored-by: Orca --- src/main/menu/register-app-menu.test.ts | 96 +++++--- src/main/menu/register-app-menu.ts | 301 ++++++++++++++---------- src/main/window/createMainWindow.ts | 7 +- 3 files changed, 245 insertions(+), 159 deletions(-) diff --git a/src/main/menu/register-app-menu.test.ts b/src/main/menu/register-app-menu.test.ts index 239646ba9ca..6847f243622 100644 --- a/src/main/menu/register-app-menu.test.ts +++ b/src/main/menu/register-app-menu.test.ts @@ -21,6 +21,8 @@ vi.mock('electron', () => ({ import { registerAppMenu } from './register-app-menu' +const isMac = process.platform === 'darwin' + function buildMenuOptions() { return { onCheckForUpdates: vi.fn(), @@ -32,6 +34,18 @@ function buildMenuOptions() { } } +function getTemplate(): Electron.MenuItemConstructorOptions[] { + return buildFromTemplateMock.mock.calls[0][0] as Electron.MenuItemConstructorOptions[] +} + +function getSubmenu( + template: Electron.MenuItemConstructorOptions[], + label: string +): Electron.MenuItemConstructorOptions[] { + const item = template.find((entry) => entry.label === label) + return (item?.submenu ?? []) as Electron.MenuItemConstructorOptions[] +} + describe('registerAppMenu', () => { beforeEach(() => { buildFromTemplateMock.mockReset() @@ -44,23 +58,16 @@ describe('registerAppMenu', () => { registerAppMenu(buildMenuOptions()) expect(buildFromTemplateMock).toHaveBeenCalledTimes(1) - const template = buildFromTemplateMock.mock.calls[0][0] as Electron.MenuItemConstructorOptions[] - const viewMenu = template.find((item) => item.label === 'View') + const viewSubmenu = getSubmenu(getTemplate(), 'View') - expect(viewMenu?.submenu).toEqual( + expect(viewSubmenu).toEqual( expect.arrayContaining([ - expect.objectContaining({ - label: 'Reload' - }), - expect.objectContaining({ - label: 'Force Reload', - accelerator: 'Shift+CmdOrCtrl+R' - }) + expect.objectContaining({ label: 'Reload' }), + expect.objectContaining({ label: 'Force Reload', accelerator: 'Shift+CmdOrCtrl+R' }) ]) ) - const submenu = viewMenu?.submenu as Electron.MenuItemConstructorOptions[] - const reloadItem = submenu.find((item) => item.label === 'Reload') + const reloadItem = viewSubmenu.find((item) => item.label === 'Reload') expect(reloadItem?.accelerator).toBeUndefined() }) @@ -76,11 +83,7 @@ describe('registerAppMenu', () => { registerAppMenu(buildMenuOptions()) - const template = buildFromTemplateMock.mock.calls[0][0] as Electron.MenuItemConstructorOptions[] - const viewMenu = template.find((item) => item.label === 'View') - const submenu = viewMenu?.submenu as Electron.MenuItemConstructorOptions[] - const reloadItem = submenu.find((item) => item.label === 'Reload') - + const reloadItem = getSubmenu(getTemplate(), 'View').find((item) => item.label === 'Reload') reloadItem?.click?.({} as never, {} as never, {} as never) expect(reloadMock).toHaveBeenCalledTimes(1) @@ -99,11 +102,9 @@ describe('registerAppMenu', () => { registerAppMenu(buildMenuOptions()) - const template = buildFromTemplateMock.mock.calls[0][0] as Electron.MenuItemConstructorOptions[] - const viewMenu = template.find((item) => item.label === 'View') - const submenu = viewMenu?.submenu as Electron.MenuItemConstructorOptions[] - const forceReloadItem = submenu.find((item) => item.label === 'Force Reload') - + const forceReloadItem = getSubmenu(getTemplate(), 'View').find( + (item) => item.label === 'Force Reload' + ) forceReloadItem?.click?.({} as never, {} as never, {} as never) expect(reloadIgnoringCacheMock).toHaveBeenCalledTimes(1) @@ -114,10 +115,13 @@ describe('registerAppMenu', () => { const options = buildMenuOptions() registerAppMenu(options) - const template = buildFromTemplateMock.mock.calls[0][0] as Electron.MenuItemConstructorOptions[] - const appMenu = template.find((item) => item.label === 'Orca') - const submenu = appMenu?.submenu as Electron.MenuItemConstructorOptions[] - const item = submenu.find((entry) => entry.label === 'Check for Updates...') + // Why: Check for Updates lives under the app-name menu on macOS and + // under Help on Windows/Linux. The click behavior must be identical + // either way. + const parentLabel = isMac ? 'Orca' : 'Help' + const item = getSubmenu(getTemplate(), parentLabel).find( + (entry) => entry.label === 'Check for Updates...' + ) item?.click?.({} as never, undefined as never, { shiftKey: true } as Electron.KeyboardEvent) item?.click?.( @@ -139,13 +143,43 @@ describe('registerAppMenu', () => { it('shows the worktree palette shortcut as a display-only menu hint', () => { registerAppMenu(buildMenuOptions()) - const template = buildFromTemplateMock.mock.calls[0][0] as Electron.MenuItemConstructorOptions[] - const viewMenu = template.find((item) => item.label === 'View') - const submenu = viewMenu?.submenu as Electron.MenuItemConstructorOptions[] - const expectedLabel = `Open Worktree Palette\t${process.platform === 'darwin' ? 'Cmd+J' : 'Ctrl+Shift+J'}` - const paletteItem = submenu.find((item) => item.label === expectedLabel) + const viewSubmenu = getSubmenu(getTemplate(), 'View') + const expectedLabel = `Open Worktree Palette\t${isMac ? 'Cmd+J' : 'Ctrl+Shift+J'}` + const paletteItem = viewSubmenu.find((item) => item.label === expectedLabel) expect(paletteItem).toBeDefined() expect(paletteItem?.accelerator).toBeUndefined() }) + + it.runIf(!isMac)('puts Settings and Exit under File on Windows/Linux', () => { + registerAppMenu(buildMenuOptions()) + + const template = getTemplate() + // Why: no redundant app-named "Orca" menu should exist on non-mac — the + // app-menu contents (Settings, Exit, Check for Updates, About) have been + // redistributed so users see them in File / Help instead. + expect(template.find((item) => item.label === 'Orca')).toBeUndefined() + + const fileLabels = getSubmenu(template, 'File').map((item) => item.label) + expect(fileLabels).toEqual(expect.arrayContaining(['Export as PDF...', 'Settings', 'Exit'])) + + const helpLabels = getSubmenu(template, 'Help').map((item) => item.label) + expect(helpLabels).toEqual(expect.arrayContaining(['Check for Updates...'])) + }) + + it.runIf(isMac)('keeps the macOS app-named menu with Settings and quit roles', () => { + registerAppMenu(buildMenuOptions()) + + const template = getTemplate() + const appSubmenu = getSubmenu(template, 'Orca') + const appLabels = appSubmenu.map((item) => item.label) + expect(appLabels).toEqual(expect.arrayContaining(['Check for Updates...', 'Settings'])) + // Why: on macOS File should NOT duplicate Settings/Exit — those live in + // the system app menu, so only Export belongs under File. + const fileLabels = getSubmenu(template, 'File').map((item) => item.label) + expect(fileLabels).not.toContain('Settings') + expect(fileLabels).not.toContain('Exit') + // No Help menu on macOS — About/Check for Updates live in the app menu. + expect(template.find((item) => item.label === 'Help')).toBeUndefined() + }) }) diff --git a/src/main/menu/register-app-menu.ts b/src/main/menu/register-app-menu.ts index e9f2ac05c9c..776f48733b0 100644 --- a/src/main/menu/register-app-menu.ts +++ b/src/main/menu/register-app-menu.ts @@ -17,6 +17,8 @@ export function registerAppMenu({ onZoomReset, onToggleStatusBar }: RegisterAppMenuOptions): void { + const isMac = process.platform === 'darwin' + const reloadFocusedWindow = (ignoreCache: boolean): void => { const webContents = BrowserWindow.getFocusedWindow()?.webContents if (!webContents) { @@ -31,134 +33,179 @@ export function registerAppMenu({ webContents.reload() } - const template: Electron.MenuItemConstructorOptions[] = [ - { - label: app.name, - submenu: [ - { role: 'about' }, - { - label: 'Check for Updates...', - // Why: holding Shift while clicking opts this check into the - // release-candidate channel. The event carries the modifier keys - // down from the native menu — we only act on the mouse chord, not - // accelerator-triggered invocations (there is no accelerator on - // this item, so triggeredByAccelerator should always be false here, - // but guarding makes the intent explicit). - click: (_menuItem, _window, event) => { - const includePrerelease = !event.triggeredByAccelerator && event.shiftKey === true - onCheckForUpdates({ includePrerelease }) - } - }, - { - label: 'Settings', - accelerator: 'CmdOrCtrl+,', - click: () => onOpenSettings() - }, - { type: 'separator' }, - { role: 'services' }, - { type: 'separator' }, - { role: 'hide' }, - { role: 'hideOthers' }, - { role: 'unhide' }, - { type: 'separator' }, - { role: 'quit' } - ] - }, - { - label: 'File', - submenu: [ - { - label: 'Export as PDF...', - accelerator: 'CmdOrCtrl+Shift+E', - click: () => { - // Why: fire a one-way event into the focused renderer. The renderer - // owns the knowledge of whether a markdown surface is active and - // what DOM to extract — when no markdown surface is active this is - // a silent no-op on that side (see design doc §4 "Renderer UI - // trigger"). Keeping this as a send (not an invoke) avoids main - // needing to reason about surface state. Using - // BrowserWindow.getFocusedWindow() rather than the menu's - // focusedWindow param avoids the BaseWindow typing gap. - BrowserWindow.getFocusedWindow()?.webContents.send('export:requestPdf') - } - } - ] - }, - { - label: 'Edit', - submenu: [ - { role: 'undo' }, - { role: 'redo' }, - { type: 'separator' }, - { role: 'cut' }, - { role: 'copy' }, - { role: 'paste' }, - { role: 'selectAll' } - ] - }, - { - label: 'View', - submenu: [ - { - label: 'Reload', - click: () => reloadFocusedWindow(false) - }, - { - label: 'Force Reload', - accelerator: 'Shift+CmdOrCtrl+R', - click: () => reloadFocusedWindow(true) - }, - { role: 'toggleDevTools' }, - { type: 'separator' }, - { - label: 'Reset Size', - accelerator: 'CmdOrCtrl+0', - // Why: Some keyboard layouts/platforms intercept Cmd/Ctrl+zoom chords - // before before-input-event fires. Binding the menu accelerator gives - // us a reliable cross-platform fallback path. - click: () => onZoomReset() - }, - { - label: 'Zoom In', - accelerator: 'CmdOrCtrl+=', - click: () => onZoomIn() - }, - { - label: 'Zoom Out', - accelerator: 'CmdOrCtrl+-', - click: () => onZoomOut() - }, - { - label: 'Zoom Out (Shift Alias)', - // Why: Some Linux keyboard layouts report the top-row minus chord as - // an underscore accelerator. Keep this hidden alias so Ctrl+- and - // Ctrl+_ can both route to terminal zoom out. - accelerator: 'CmdOrCtrl+_', - visible: false, - click: () => onZoomOut() - }, - { type: 'separator' }, - { - // Why: display-only shortcut hint — do NOT set `accelerator` here. - // Menu accelerators intercept key events at the main-process level - // before the renderer's keydown handler fires. The overlay - // mutual-exclusion logic (which runs in the renderer) would be - // bypassed if this were a real accelerator binding. - label: `Open Worktree Palette\t${process.platform === 'darwin' ? 'Cmd+J' : 'Ctrl+Shift+J'}` - }, - { type: 'separator' }, - { role: 'togglefullscreen' }, - { type: 'separator' }, - { - label: 'Toggle Status Bar', - click: () => onToggleStatusBar() - } - ] - }, - { - label: 'Window', - submenu: [{ role: 'minimize' }, { role: 'zoom' }] + // Why: holding Shift while clicking Check for Updates opts this check into + // the release-candidate channel. Extracted so both the macOS app-menu entry + // and the Windows/Linux Help-menu entry share the exact same behavior. + const checkForUpdatesClick: Electron.MenuItemConstructorOptions['click'] = ( + _menuItem, + _window, + event + ) => { + const includePrerelease = !event.triggeredByAccelerator && event.shiftKey === true + onCheckForUpdates({ includePrerelease }) + } + + const checkForUpdatesItem: Electron.MenuItemConstructorOptions = { + label: 'Check for Updates...', + click: checkForUpdatesClick + } + + const settingsItem: Electron.MenuItemConstructorOptions = { + label: 'Settings', + accelerator: 'CmdOrCtrl+,', + click: () => onOpenSettings() + } + + const exportPdfItem: Electron.MenuItemConstructorOptions = { + label: 'Export as PDF...', + accelerator: 'CmdOrCtrl+Shift+E', + click: () => { + // Why: fire a one-way event into the focused renderer. The renderer + // owns the knowledge of whether a markdown surface is active and + // what DOM to extract — when no markdown surface is active this is + // a silent no-op on that side (see design doc §4 "Renderer UI + // trigger"). Keeping this as a send (not an invoke) avoids main + // needing to reason about surface state. Using + // BrowserWindow.getFocusedWindow() rather than the menu's + // focusedWindow param avoids the BaseWindow typing gap. + BrowserWindow.getFocusedWindow()?.webContents.send('export:requestPdf') } + } + + // Why: the macOS app-menu (named after the app) is mandatory on darwin and + // owns hide/hideOthers/unhide/services/quit roles that only make sense in + // the system menu bar. On Windows/Linux that menu would render as a + // redundant "Orca" entry with roles that don't apply, so we omit it there + // and distribute its items across File / Help instead. + const macAppMenu: Electron.MenuItemConstructorOptions = { + label: app.name, + submenu: [ + { role: 'about' }, + checkForUpdatesItem, + settingsItem, + { type: 'separator' }, + { role: 'services' }, + { type: 'separator' }, + { role: 'hide' }, + { role: 'hideOthers' }, + { role: 'unhide' }, + { type: 'separator' }, + { role: 'quit' } + ] + } + + const fileMenu: Electron.MenuItemConstructorOptions = { + label: 'File', + submenu: [ + exportPdfItem, + // Why: on Windows/Linux there is no app-named menu, so Settings and + // Quit live under File — matching the common platform convention and + // keeping all user-facing actions reachable from the in-window menu bar. + ...(isMac + ? [] + : ([ + { type: 'separator' }, + settingsItem, + { type: 'separator' }, + { role: 'quit', label: 'Exit' } + ] satisfies Electron.MenuItemConstructorOptions[])) + ] + } + + const editMenu: Electron.MenuItemConstructorOptions = { + label: 'Edit', + submenu: [ + { role: 'undo' }, + { role: 'redo' }, + { type: 'separator' }, + { role: 'cut' }, + { role: 'copy' }, + { role: 'paste' }, + { role: 'selectAll' } + ] + } + + const viewMenu: Electron.MenuItemConstructorOptions = { + label: 'View', + submenu: [ + { + label: 'Reload', + click: () => reloadFocusedWindow(false) + }, + { + label: 'Force Reload', + accelerator: 'Shift+CmdOrCtrl+R', + click: () => reloadFocusedWindow(true) + }, + { role: 'toggleDevTools' }, + { type: 'separator' }, + { + label: 'Reset Size', + accelerator: 'CmdOrCtrl+0', + // Why: Some keyboard layouts/platforms intercept Cmd/Ctrl+zoom chords + // before before-input-event fires. Binding the menu accelerator gives + // us a reliable cross-platform fallback path. + click: () => onZoomReset() + }, + { + label: 'Zoom In', + accelerator: 'CmdOrCtrl+=', + click: () => onZoomIn() + }, + { + label: 'Zoom Out', + accelerator: 'CmdOrCtrl+-', + click: () => onZoomOut() + }, + { + label: 'Zoom Out (Shift Alias)', + // Why: Some Linux keyboard layouts report the top-row minus chord as + // an underscore accelerator. Keep this hidden alias so Ctrl+- and + // Ctrl+_ can both route to terminal zoom out. + accelerator: 'CmdOrCtrl+_', + visible: false, + click: () => onZoomOut() + }, + { type: 'separator' }, + { + // Why: display-only shortcut hint — do NOT set `accelerator` here. + // Menu accelerators intercept key events at the main-process level + // before the renderer's keydown handler fires. The overlay + // mutual-exclusion logic (which runs in the renderer) would be + // bypassed if this were a real accelerator binding. + label: `Open Worktree Palette\t${isMac ? 'Cmd+J' : 'Ctrl+Shift+J'}` + }, + { type: 'separator' }, + { role: 'togglefullscreen' }, + { type: 'separator' }, + { + label: 'Toggle Status Bar', + click: () => onToggleStatusBar() + } + ] + } + + const windowMenu: Electron.MenuItemConstructorOptions = { + label: 'Window', + submenu: [{ role: 'minimize' }, { role: 'zoom' }] + } + + // Why: Windows/Linux have no app-named menu, so About + Check for Updates + // go into a Help menu — the standard place for those entries on those + // platforms. On macOS the system "About Orca" and "Check for Updates" + // already sit under the app menu, so we don't duplicate them here. + const helpMenu: Electron.MenuItemConstructorOptions = { + label: 'Help', + submenu: [{ role: 'about' }, checkForUpdatesItem] + } + + const template: Electron.MenuItemConstructorOptions[] = [ + ...(isMac ? [macAppMenu] : []), + fileMenu, + editMenu, + viewMenu, + windowMenu, + ...(isMac ? [] : [helpMenu]) ] Menu.setApplicationMenu(Menu.buildFromTemplate(template)) diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index 7be28e67ec0..4d628c981d6 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -84,7 +84,12 @@ export function createMainWindow( minWidth: 600, minHeight: 400, show: false, - autoHideMenuBar: true, + // Why: on macOS the menu lives in the system menu bar, so the in-window + // menu bar is irrelevant and stays hidden. On Windows/Linux the menu bar + // *is* the only surface for File/Edit/View/Window, so keep it always + // visible — otherwise users never see the menu items at all (they'd have + // to press Alt to toggle it). + autoHideMenuBar: process.platform === 'darwin', backgroundColor: nativeTheme.shouldUseDarkColors ? '#0a0a0a' : '#ffffff', titleBarStyle: process.platform === 'darwin' ? 'hiddenInset' : undefined, // Why: initial position for 1x zoom; syncTrafficLightPosition() adjusts