Pass workspace id to contextmenu-show (#1429)

Sometimes, the context menu click handlers don't seem to get passed any window object. Here, I'm sending over the workspace id with the `contextmenu-show` event so that we can resolve our cached copy of the object in case the value from the click handler is empty.
2025-01-02 18:39:05 +01:00 · 2024-12-06 19:10:34 -08:00 · 2024-12-06 19:10:34 -08:00 · f858d3ba0f
commit f858d3ba0f
parent 7d21f55b84
7 changed files with 71 additions and 59 deletions
--- a/emain/emain-window.ts
+++ b/emain/emain-window.ts
@ -299,7 +299,10 @@ export class WaveBrowserWindow extends BaseWindow {
        const workspaceList = await WorkspaceService.ListWorkspaces();
        if (!workspaceList.find((wse) => wse.workspaceid === workspaceId)?.windowid) {
            const curWorkspace = await WorkspaceService.GetWorkspace(this.workspaceId);
-            if (curWorkspace.tabids.length > 1 && (!curWorkspace.name || !curWorkspace.icon)) {
+            if (
+                (curWorkspace.tabids?.length || curWorkspace.pinnedtabids?.length) &&
+                (!curWorkspace.name || !curWorkspace.icon)
+            ) {
                const choice = dialog.showMessageBoxSync(this, {
                    type: "question",
                    buttons: ["Cancel", "Open in New Window", "Yes"],
--- a/emain/emain.ts
+++ b/emain/emain.ts
@ -44,7 +44,7 @@ import {
 import { ElectronWshClient, initElectronWshClient } from "./emain-wsh";
 import { getLaunchSettings } from "./launchsettings";
 import { log } from "./log";
-import { instantiateAppMenu, makeAppMenu } from "./menu";
+import { makeAppMenu } from "./menu";
 import {
    getElectronAppBasePath,
    getElectronAppUnpackedBasePath,
@ -426,17 +426,6 @@ function saveImageFileWithNativeDialog(defaultFileName: string, mimeType: string

 electron.ipcMain.on("open-new-window", () => fireAndForget(createNewWaveWindow));

-electron.ipcMain.on("contextmenu-show", (event, menuDefArr?: ElectronContextMenuItem[]) => {
-    if (menuDefArr?.length === 0) {
-        return;
-    }
-    fireAndForget(async () => {
-        const menu = menuDefArr ? convertMenuDefArrToMenu(menuDefArr) : await instantiateAppMenu();
-        menu.popup();
-    });
-    event.returnValue = true;
-});
-
 // we try to set the primary display as index [0]
 function getActivityDisplays(): ActivityDisplayType[] {
    const displays = electron.screen.getAllDisplays();
@ -488,28 +477,6 @@ function runActiveTimer() {
    setTimeout(runActiveTimer, 60000);
 }

-function convertMenuDefArrToMenu(menuDefArr: ElectronContextMenuItem[]): electron.Menu {
-    const menuItems: electron.MenuItem[] = [];
-    for (const menuDef of menuDefArr) {
-        const menuItemTemplate: electron.MenuItemConstructorOptions = {
-            role: menuDef.role as any,
-            label: menuDef.label,
-            type: menuDef.type,
-            click: (_, window) => {
-                const ww = window as WaveBrowserWindow;
-                ww?.activeTabView?.webContents?.send("contextmenu-click", menuDef.id);
-            },
-            checked: menuDef.checked,
-        };
-        if (menuDef.submenu != null) {
-            menuItemTemplate.submenu = convertMenuDefArrToMenu(menuDef.submenu);
-        }
-        const menuItem = new electron.MenuItem(menuItemTemplate);
-        menuItems.push(menuItem);
-    }
-    return electron.Menu.buildFromTemplate(menuItems);
-}
-
 function hideWindowWithCatch(window: WaveBrowserWindow) {
    if (window == null) {
        return;
--- a/emain/menu.ts
+++ b/emain/menu.ts
@ -10,6 +10,7 @@ import {
    createNewWaveWindow,
    createWorkspace,
    focusedWaveWindow,
+    getWaveWindowByWorkspaceId,
    relaunchBrowserWindows,
    WaveBrowserWindow,
 } from "./emain-window";
@ -36,15 +37,14 @@ function getWindowWebContents(window: electron.BaseWindow): electron.WebContents
    return null;
 }

-async function getWorkspaceMenu(): Promise<Electron.MenuItemConstructorOptions[]> {
+async function getWorkspaceMenu(ww?: WaveBrowserWindow): Promise<Electron.MenuItemConstructorOptions[]> {
    const workspaceList = await RpcApi.WorkspaceListCommand(ElectronWshClient);
    console.log("workspaceList:", workspaceList);
    const workspaceMenu: Electron.MenuItemConstructorOptions[] = [
        {
            label: "Create New Workspace",
            click: (_, window) => {
-                const ww = window as WaveBrowserWindow;
-                fireAndForget(() => createWorkspace(ww));
+                fireAndForget(() => createWorkspace((window as WaveBrowserWindow) ?? ww));
            },
        },
    ];
@ -65,8 +65,7 @@ async function getWorkspaceMenu(): Promise<Electron.MenuItemConstructorOptions[]
                return {
                    label: `Switch to ${workspace.workspacedata.name} (${workspace.workspacedata.oid.slice(0, 5)})`,
                    click: (_, window) => {
-                        const ww = window as WaveBrowserWindow;
-                        ww.switchWorkspace(workspace.workspacedata.oid);
+                        ((window as WaveBrowserWindow) ?? ww)?.switchWorkspace(workspace.workspacedata.oid);
                    },
                    accelerator: getWorkspaceSwitchAccelerator(i),
                };
@ -75,7 +74,8 @@ async function getWorkspaceMenu(): Promise<Electron.MenuItemConstructorOptions[]
    return workspaceMenu;
 }

-async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
+async function getAppMenu(callbacks: AppMenuCallbacks, workspaceId?: string): Promise<Electron.Menu> {
+    const ww = workspaceId && getWaveWindowByWorkspaceId(workspaceId);
    const fileMenu: Electron.MenuItemConstructorOptions[] = [
        {
            label: "New Window",
@ -94,7 +94,7 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
        {
            label: "About Wave Terminal",
            click: (_, window) => {
-                getWindowWebContents(window)?.send("menu-item-about");
+                getWindowWebContents(window ?? ww)?.send("menu-item-about");
            },
        },
        {
@ -172,7 +172,7 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
            label: "Reload Tab",
            accelerator: "Shift+CommandOrControl+R",
            click: (_, window) => {
-                getWindowWebContents(window)?.reloadIgnoringCache();
+                getWindowWebContents(window ?? ww)?.reloadIgnoringCache();
            },
        },
        {
@ -191,7 +191,7 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
            label: "Toggle DevTools",
            accelerator: devToolsAccel,
            click: (_, window) => {
-                let wc = getWindowWebContents(window);
+                let wc = getWindowWebContents(window ?? ww);
                wc?.toggleDevTools();
            },
        },
@ -202,14 +202,14 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
            label: "Reset Zoom",
            accelerator: "CommandOrControl+0",
            click: (_, window) => {
-                getWindowWebContents(window)?.setZoomFactor(1);
+                getWindowWebContents(window ?? ww)?.setZoomFactor(1);
            },
        },
        {
            label: "Zoom In",
            accelerator: "CommandOrControl+=",
            click: (_, window) => {
-                const wc = getWindowWebContents(window);
+                const wc = getWindowWebContents(window ?? ww);
                if (wc == null) {
                    return;
                }
@ -223,7 +223,7 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
            label: "Zoom In (hidden)",
            accelerator: "CommandOrControl+Shift+=",
            click: (_, window) => {
-                const wc = getWindowWebContents(window);
+                const wc = getWindowWebContents(window ?? ww);
                if (wc == null) {
                    return;
                }
@ -239,7 +239,7 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
            label: "Zoom Out",
            accelerator: "CommandOrControl+-",
            click: (_, window) => {
-                const wc = getWindowWebContents(window);
+                const wc = getWindowWebContents(window ?? ww);
                if (wc == null) {
                    return;
                }
@ -253,7 +253,7 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
            label: "Zoom Out (hidden)",
            accelerator: "CommandOrControl+Shift+-",
            click: (_, window) => {
-                const wc = getWindowWebContents(window);
+                const wc = getWindowWebContents(window ?? ww);
                if (wc == null) {
                    return;
                }
@ -313,11 +313,14 @@ async function getAppMenu(callbacks: AppMenuCallbacks): Promise<Electron.Menu> {
    return electron.Menu.buildFromTemplate(menuTemplate);
 }

-export function instantiateAppMenu(): Promise<electron.Menu> {
-    return getAppMenu({
-        createNewWaveWindow,
-        relaunchBrowserWindows,
-    });
+export function instantiateAppMenu(workspaceId?: string): Promise<electron.Menu> {
+    return getAppMenu(
+        {
+            createNewWaveWindow,
+            relaunchBrowserWindows,
+        },
+        workspaceId
+    );
 }

 export function makeAppMenu() {
@ -332,4 +335,43 @@ waveEventSubscribe({
    handler: makeAppMenu,
 });

+function convertMenuDefArrToMenu(workspaceId: string, menuDefArr: ElectronContextMenuItem[]): electron.Menu {
+    const menuItems: electron.MenuItem[] = [];
+    for (const menuDef of menuDefArr) {
+        const menuItemTemplate: electron.MenuItemConstructorOptions = {
+            role: menuDef.role as any,
+            label: menuDef.label,
+            type: menuDef.type,
+            click: (_, window) => {
+                const ww = (window as WaveBrowserWindow) ?? getWaveWindowByWorkspaceId(workspaceId);
+                if (!ww) {
+                    console.error("invalid window for context menu click handler:", ww, window, workspaceId);
+                    return;
+                }
+                ww?.activeTabView?.webContents?.send("contextmenu-click", menuDef.id);
+            },
+            checked: menuDef.checked,
+        };
+        if (menuDef.submenu != null) {
+            menuItemTemplate.submenu = convertMenuDefArrToMenu(workspaceId, menuDef.submenu);
+        }
+        const menuItem = new electron.MenuItem(menuItemTemplate);
+        menuItems.push(menuItem);
+    }
+    return electron.Menu.buildFromTemplate(menuItems);
+}
+
+electron.ipcMain.on("contextmenu-show", (event, workspaceId: string, menuDefArr?: ElectronContextMenuItem[]) => {
+    if (menuDefArr?.length === 0) {
+        return;
+    }
+    fireAndForget(async () => {
+        const menu = menuDefArr
+            ? convertMenuDefArrToMenu(workspaceId, menuDefArr)
+            : await instantiateAppMenu(workspaceId);
+        menu.popup();
+    });
+    event.returnValue = true;
+});
+
 export { getAppMenu };
--- a/emain/preload.ts
+++ b/emain/preload.ts
@ -16,7 +16,7 @@ contextBridge.exposeInMainWorld("api", {
    getDocsiteUrl: () => ipcRenderer.sendSync("get-docsite-url"),
    getWebviewPreload: () => ipcRenderer.sendSync("get-webview-preload"),
    openNewWindow: () => ipcRenderer.send("open-new-window"),
-    showContextMenu: (menu, position) => ipcRenderer.send("contextmenu-show", menu, position),
+    showContextMenu: (workspaceId, menu) => ipcRenderer.send("contextmenu-show", workspaceId, menu),
    onContextMenuClick: (callback) => ipcRenderer.on("contextmenu-click", (_event, id) => callback(id)),
    downloadFile: (filePath) => ipcRenderer.send("download", { filePath }),
    openExternal: (url) => {
--- a/frontend/app/store/contextmenu.ts
+++ b/frontend/app/store/contextmenu.ts
@ -1,7 +1,7 @@
 // Copyright 2024, Command Line Inc.
 // SPDX-License-Identifier: Apache-2.0

-import { getApi } from "./global";
+import { atoms, getApi, globalStore } from "./global";

 class ContextMenuModelType {
    handlers: Map<string, () => void> = new Map(); // id -> handler
@ -48,7 +48,7 @@ class ContextMenuModelType {
    showContextMenu(menu: ContextMenuItem[], ev: React.MouseEvent<any>): void {
        this.handlers.clear();
        const electronMenuItems = this._convertAndRegisterMenu(menu);
-        getApi().showContextMenu(electronMenuItems);
+        getApi().showContextMenu(globalStore.get(atoms.workspace).oid, electronMenuItems);
    }
 }

--- a/frontend/app/tab/tabbar.tsx
+++ b/frontend/app/tab/tabbar.tsx
@ -599,7 +599,7 @@ const TabBar = memo(({ workspace }: TabBarProps) => {
    };

    function onEllipsisClick() {
-        getApi().showContextMenu();
+        getApi().showContextMenu(workspace.oid);
    }

    const tabsWrapperWidth = tabIds.length * tabWidthRef.current;
--- a/frontend/types/custom.d.ts
+++ b/frontend/types/custom.d.ts
@ -72,7 +72,7 @@ declare global {
        getWebviewPreload: () => string;
        getAboutModalDetails: () => AboutModalDetails;
        getDocsiteUrl: () => string;
-        showContextMenu: (menu?: ElectronContextMenuItem[]) => void;
+        showContextMenu: (workspaceId: string, menu?: ElectronContextMenuItem[]) => void;
        onContextMenuClick: (callback: (id: string) => void) => void;
        onNavigate: (callback: (url: string) => void) => void;
        onIframeNavigate: (callback: (url: string) => void) => void;