diff --git a/src/main/index.ts b/src/main/index.ts index b61cde0ea..91a45ea1f 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -51,7 +51,7 @@ app.whenReady().then(async () => { initializeAppLifecycle(mb, contextMenu, protocol); // Configure window event handlers (Escape key, DevTools resize) - configureWindowEvents(mb); + configureWindowEvents(mb, menuBuilder); // Register IPC handlers for various channels registerTrayHandlers(mb); diff --git a/src/main/lifecycle/startup.test.ts b/src/main/lifecycle/startup.test.ts index fbf7eff69..211e3bcb0 100644 --- a/src/main/lifecycle/startup.test.ts +++ b/src/main/lifecycle/startup.test.ts @@ -28,6 +28,11 @@ vi.mock('../../shared/logger', () => ({ logWarn: (...a: unknown[]) => logWarnMock(...a), })); +const isLinuxMock = vi.fn(() => false); +vi.mock('../../shared/platform', () => ({ + isLinux: () => isLinuxMock(), +})); + function createMb() { return { on: vi.fn(), @@ -38,6 +43,7 @@ function createMb() { setIgnoreDoubleClickEvents: vi.fn(), on: vi.fn(), popUpContextMenu: vi.fn(), + setContextMenu: vi.fn(), }, }; } @@ -63,6 +69,44 @@ describe('main/lifecycle/startup.ts', () => { expect(appQuitMock).toHaveBeenCalled(); expect(logWarnMock).toHaveBeenCalled(); }); + + it('uses setContextMenu on Linux', () => { + isLinuxMock.mockReturnValueOnce(true); + const mb = createMb(); + const contextMenu = {} as Electron.Menu; + + initializeAppLifecycle(mb as unknown as Menubar, contextMenu, 'gitify'); + + const readyHandler = (mb.on as unknown as ReturnType).mock + .calls[0]?.[1]; + expect(readyHandler).toBeDefined(); + readyHandler?.(); + + expect(mb.tray.setContextMenu).toHaveBeenCalledWith(contextMenu); + expect(mb.tray.on).not.toHaveBeenCalledWith( + 'right-click', + expect.any(Function), + ); + }); + + it('uses popUpContextMenu on non-Linux platforms', () => { + isLinuxMock.mockReturnValueOnce(false); + const mb = createMb(); + const contextMenu = {} as Electron.Menu; + + initializeAppLifecycle(mb as unknown as Menubar, contextMenu, 'gitify'); + + const readyHandler = (mb.on as unknown as ReturnType).mock + .calls[0]?.[1]; + expect(readyHandler).toBeDefined(); + readyHandler?.(); + + expect(mb.tray.setContextMenu).not.toHaveBeenCalled(); + expect(mb.tray.on).toHaveBeenCalledWith( + 'right-click', + expect.any(Function), + ); + }); }); describe('handleProtocolURL', () => { diff --git a/src/main/lifecycle/startup.ts b/src/main/lifecycle/startup.ts index 87b65d6cd..6296be019 100644 --- a/src/main/lifecycle/startup.ts +++ b/src/main/lifecycle/startup.ts @@ -4,6 +4,7 @@ import type { Menubar } from 'menubar'; import { APPLICATION } from '../../shared/constants'; import { EVENTS } from '../../shared/events'; import { logInfo, logWarn } from '../../shared/logger'; +import { isLinux } from '../../shared/platform'; import { sendRendererEvent } from '../events'; @@ -27,9 +28,19 @@ export function initializeAppLifecycle( mb.tray.setIgnoreDoubleClickEvents(true); - mb.tray.on('right-click', (_event, bounds) => { - mb.tray.popUpContextMenu(contextMenu, { x: bounds.x, y: bounds.y }); - }); + if (isLinux()) { + // Linux trays go through libappindicator / StatusNotifierItem (D-Bus), + // where Electron only emits 'right-click' if no context menu is set. + // setContextMenu hands the menu to the host indicator so it renders + // natively on right-click. Don't use this on macOS — there + // setContextMenu intercepts left-click too and would break the + // menubar window toggle. + mb.tray.setContextMenu(contextMenu); + } else { + mb.tray.on('right-click', (_event, bounds) => { + mb.tray.popUpContextMenu(contextMenu, { x: bounds.x, y: bounds.y }); + }); + } }); preventSecondInstance(mb, protocol); diff --git a/src/main/lifecycle/window.test.ts b/src/main/lifecycle/window.test.ts index 6bf021ce8..870254de0 100644 --- a/src/main/lifecycle/window.test.ts +++ b/src/main/lifecycle/window.test.ts @@ -1,5 +1,6 @@ import type { Menubar } from 'menubar'; +import type MenuBuilder from '../menu'; import { __resetWindowLifecycleForTests, configureWindowEvents, @@ -51,6 +52,7 @@ const flushDeferred = () => new Promise((resolve) => setImmediate(resolve)); describe('main/lifecycle/window.ts', () => { let menubar: Menubar; + let menuBuilder: MenuBuilder; beforeEach(() => { appOnMock.mockClear(); @@ -80,6 +82,9 @@ describe('main/lifecycle/window.ts', () => { move: vi.fn(), }, } as unknown as Menubar; + menuBuilder = { + setWindowVisibility: vi.fn(), + } as unknown as MenuBuilder; }); afterEach(() => { @@ -90,12 +95,12 @@ describe('main/lifecycle/window.ts', () => { const mbNoWindow = { ...menubar, window: null }; expect(() => - configureWindowEvents(mbNoWindow as unknown as Menubar), + configureWindowEvents(mbNoWindow as unknown as Menubar, menuBuilder), ).not.toThrow(); }); it('configureWindowEvents registers webContents event listeners', () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); expect(menubar.window?.webContents.on).toHaveBeenCalledWith( 'before-input-event', @@ -112,7 +117,7 @@ describe('main/lifecycle/window.ts', () => { }); it('configureWindowEvents registers window close, before-quit and window-all-closed listeners', () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); expect(menubar.window?.on).toHaveBeenCalledWith( 'close', @@ -125,9 +130,29 @@ describe('main/lifecycle/window.ts', () => { ); }); + describe('window visibility forwarding', () => { + it('forwards window show events to menu builder', () => { + configureWindowEvents(menubar, menuBuilder); + + const showHandler = findWindowHandler(menubar, 'show'); + showHandler?.({ preventDefault: vi.fn() }); + + expect(menuBuilder.setWindowVisibility).toHaveBeenCalledWith(true); + }); + + it('forwards window hide events to menu builder', () => { + configureWindowEvents(menubar, menuBuilder); + + const hideHandler = findWindowHandler(menubar, 'hide'); + hideHandler?.({ preventDefault: vi.fn() }); + + expect(menuBuilder.setWindowVisibility).toHaveBeenCalledWith(false); + }); + }); + describe('window close handler', () => { it('hides the window and restores menubar reference on a WM close', async () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); const closeHandler = findWindowHandler(menubar, 'close'); const event = { preventDefault: vi.fn() }; @@ -149,7 +174,7 @@ describe('main/lifecycle/window.ts', () => { }); it('skips the deferred hide when the captured window is destroyed', async () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); const captured = menubar.window; const closeHandler = findWindowHandler(menubar, 'close'); @@ -163,7 +188,7 @@ describe('main/lifecycle/window.ts', () => { }); it('lets the window close during quit', async () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); findAppHandler('before-quit')?.(); @@ -180,7 +205,7 @@ describe('main/lifecycle/window.ts', () => { describe('window-all-closed handler', () => { it('keeps the app alive when not quitting', () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); findAppHandler('window-all-closed')?.(); @@ -188,7 +213,7 @@ describe('main/lifecycle/window.ts', () => { }); it('quits on linux when the user is quitting', () => { - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); findAppHandler('before-quit')?.(); findAppHandler('window-all-closed')?.(); @@ -198,7 +223,7 @@ describe('main/lifecycle/window.ts', () => { it('does not quit on macOS', () => { setPlatform('darwin'); - configureWindowEvents(menubar); + configureWindowEvents(menubar, menuBuilder); findAppHandler('before-quit')?.(); findAppHandler('window-all-closed')?.(); diff --git a/src/main/lifecycle/window.ts b/src/main/lifecycle/window.ts index 90cf679f1..12cfa58fd 100644 --- a/src/main/lifecycle/window.ts +++ b/src/main/lifecycle/window.ts @@ -5,6 +5,7 @@ import { logWarn, toError } from '../../shared/logger'; import { isMacOS } from '../../shared/platform'; import { WindowConfig } from '../config'; +import type MenuBuilder from '../menu'; let isQuitting = false; @@ -44,13 +45,26 @@ function restoreMenubarWindowReference(mb: Menubar, win: BrowserWindow): void { * Attach window-level event listeners for keyboard input and DevTools. * * @param mb - The menubar instance whose window events are configured. + * @param menuBuilder - The menu builder used to keep the Show / Hide tray + * menu items in sync with window visibility. */ -export function configureWindowEvents(mb: Menubar): void { +export function configureWindowEvents( + mb: Menubar, + menuBuilder: MenuBuilder, +): void { const win = mb.window; if (!win) { return; } + win.on('show', () => { + menuBuilder.setWindowVisibility(true); + }); + + win.on('hide', () => { + menuBuilder.setWindowVisibility(false); + }); + /** * Track explicit quit requests so the close handlers can distinguish * between an app quit and a WM-initiated window close. diff --git a/src/main/menu.test.ts b/src/main/menu.test.ts index 95a54e7b4..3e751825f 100644 --- a/src/main/menu.test.ts +++ b/src/main/menu.test.ts @@ -5,7 +5,7 @@ import type { Menubar } from 'menubar'; import type { Mock } from 'vitest'; import { APPLICATION } from '../shared/constants'; -import { isMacOS } from '../shared/platform'; +import { isLinux, isMacOS } from '../shared/platform'; import { resetApp } from './lifecycle/reset'; import MenuBuilder from './menu'; @@ -54,6 +54,7 @@ vi.mock('./lifecycle/reset', () => ({ })); vi.mock('../shared/platform', () => ({ + isLinux: vi.fn(() => false), isMacOS: vi.fn(), })); @@ -90,9 +91,18 @@ describe('main/menu.ts', () => { }; beforeEach(() => { + vi.mocked(isLinux).mockReturnValue(false); vi.mocked(isMacOS).mockReturnValue(false); menuItemInstances.length = 0; // Clear tracked instances - menubar = { app: { quit: vi.fn() } } as unknown as Menubar; + menubar = { + app: { quit: vi.fn() }, + showWindow: vi.fn(), + hideWindow: vi.fn(), + tray: { + isDestroyed: vi.fn(() => false), + setContextMenu: vi.fn(), + }, + } as unknown as Menubar; menuBuilder = new MenuBuilder(menubar); }); @@ -197,6 +207,71 @@ describe('main/menu.ts', () => { }); }); + describe('windowVisibilityMenuItems', () => { + it('show item is visible by default; hide item is not', () => { + const showCfg = getMenuItemConfigByLabel(`Show ${APPLICATION.NAME}`); + const hideCfg = getMenuItemConfigByLabel(`Hide ${APPLICATION.NAME}`); + + expect(showCfg?.visible).toBe(true); + expect(hideCfg?.visible).toBe(false); + }); + + it('setWindowVisibility(true) shows hide item, hides show item', () => { + menuBuilder.setWindowVisibility(true); + + // biome-ignore lint/complexity/useLiteralKeys: This is a test + expect(menuBuilder['showWindowMenuItem'].visible).toBe(false); + // biome-ignore lint/complexity/useLiteralKeys: This is a test + expect(menuBuilder['hideWindowMenuItem'].visible).toBe(true); + }); + + it('setWindowVisibility(false) shows show item, hides hide item', () => { + menuBuilder.setWindowVisibility(true); + menuBuilder.setWindowVisibility(false); + + // biome-ignore lint/complexity/useLiteralKeys: This is a test + expect(menuBuilder['showWindowMenuItem'].visible).toBe(true); + // biome-ignore lint/complexity/useLiteralKeys: This is a test + expect(menuBuilder['hideWindowMenuItem'].visible).toBe(false); + }); + + it('does not re-publish the menu on non-Linux platforms', () => { + menuBuilder.buildMenu(); + menuBuilder.setWindowVisibility(true); + + expect(menubar.tray.setContextMenu).not.toHaveBeenCalled(); + }); + + it('re-publishes the menu over D-Bus on Linux', () => { + vi.mocked(isLinux).mockReturnValue(true); + const menu = {} as Electron.Menu; + (Menu.buildFromTemplate as Mock).mockReturnValueOnce(menu); + menuBuilder.buildMenu(); + + menuBuilder.setWindowVisibility(true); + + expect(menubar.tray.setContextMenu).toHaveBeenCalledWith(menu); + }); + + it('skips re-publishing if buildMenu has not run yet', () => { + vi.mocked(isLinux).mockReturnValue(true); + + menuBuilder.setWindowVisibility(true); + + expect(menubar.tray.setContextMenu).not.toHaveBeenCalled(); + }); + + it('skips re-publishing when the tray is destroyed', () => { + vi.mocked(isLinux).mockReturnValue(true); + menuBuilder.buildMenu(); + (menubar.tray.isDestroyed as Mock).mockReturnValue(true); + + menuBuilder.setWindowVisibility(true); + + expect(menubar.tray.setContextMenu).not.toHaveBeenCalled(); + }); + }); + describe('click handlers', () => { it('invokes autoUpdater.checkForUpdatesAndNotify when clicking "Check for updates"', () => { const cfg = getMenuItemConfigByLabel('Check for updates'); @@ -257,6 +332,18 @@ describe('main/menu.ts', () => { expect(menubar.app.quit).toHaveBeenCalled(); }); + it('show window menu item calls showWindow', () => { + const cfg = getMenuItemConfigByLabel(`Show ${APPLICATION.NAME}`); + cfg?.click?.(); + expect(menubar.showWindow).toHaveBeenCalled(); + }); + + it('hide window menu item calls hideWindow', () => { + const cfg = getMenuItemConfigByLabel(`Hide ${APPLICATION.NAME}`); + cfg?.click?.(); + expect(menubar.hideWindow).toHaveBeenCalled(); + }); + it('developer submenu includes expected static accelerators', () => { const template = buildAndGetTemplate(); const devEntry = template.find( diff --git a/src/main/menu.ts b/src/main/menu.ts index 4aeca1cea..7d9824117 100644 --- a/src/main/menu.ts +++ b/src/main/menu.ts @@ -3,7 +3,7 @@ import { autoUpdater } from 'electron-updater'; import type { Menubar } from 'menubar'; import { APPLICATION } from '../shared/constants'; -import { isMacOS } from '../shared/platform'; +import { isLinux, isMacOS } from '../shared/platform'; import { resetApp } from './lifecycle/reset'; import { openLogsDirectory, takeScreenshot } from './utils'; @@ -16,8 +16,11 @@ export default class MenuBuilder { private readonly noUpdateAvailableMenuItem: MenuItem; private readonly updateAvailableMenuItem: MenuItem; private readonly updateReadyForInstallMenuItem: MenuItem; + private readonly showWindowMenuItem: MenuItem; + private readonly hideWindowMenuItem: MenuItem; private readonly menubar: Menubar; + private menu?: Menu; /** * @param menubar - The menubar instance used for window and app interactions within menu actions. @@ -53,13 +56,32 @@ export default class MenuBuilder { autoUpdater.quitAndInstall(); }, }); + + this.showWindowMenuItem = new MenuItem({ + label: `Show ${APPLICATION.NAME}`, + visible: true, + click: () => { + this.menubar.showWindow(); + }, + }); + + this.hideWindowMenuItem = new MenuItem({ + label: `Hide ${APPLICATION.NAME}`, + visible: false, + click: () => { + this.menubar.hideWindow(); + }, + }); } /** * Build and return the tray right-click context menu. */ buildMenu(): Menu { - const contextMenu = Menu.buildFromTemplate([ + this.menu = Menu.buildFromTemplate([ + this.showWindowMenuItem, + this.hideWindowMenuItem, + { type: 'separator' }, this.checkForUpdatesMenuItem, this.noUpdateAvailableMenuItem, this.updateAvailableMenuItem, @@ -117,7 +139,32 @@ export default class MenuBuilder { }, ]); - return contextMenu; + return this.menu; + } + + /** + * Reflect the current window visibility in the Show / Hide menu items. + * On Linux, the indicator caches the menu over D-Bus, so we re-publish + * via setContextMenu to push the visibility flip through immediately + * instead of waiting for the next mutation event. + * + * @param isVisible - Whether the popup window is currently visible. + */ + setWindowVisibility(isVisible: boolean) { + this.showWindowMenuItem.visible = !isVisible; + this.hideWindowMenuItem.visible = isVisible; + + if (!isLinux() || !this.menu) { + return; + } + try { + if (!this.menubar.tray.isDestroyed()) { + this.menubar.tray.setContextMenu(this.menu); + } + } catch { + // Tray not yet ready; the initial setContextMenu in + // initializeAppLifecycle will pick up the current state. + } } /**