diff --git a/emain/emain-menu.ts b/emain/emain-menu.ts index 41752147d1..6685d79087 100644 --- a/emain/emain-menu.ts +++ b/emain/emain-menu.ts @@ -408,7 +408,8 @@ function getWebContentsByWorkspaceOrBuilderId(workspaceOrBuilderId: string): ele function convertMenuDefArrToMenu( webContents: electron.WebContents, - menuDefArr: ElectronContextMenuItem[] + menuDefArr: ElectronContextMenuItem[], + menuState: { hasClick: boolean } ): electron.Menu { const menuItems: electron.MenuItem[] = []; for (const menuDef of menuDefArr) { @@ -416,19 +417,15 @@ function convertMenuDefArrToMenu( role: menuDef.role as any, label: menuDef.label, type: menuDef.type, - click: (_, window) => { - const wc = getWindowWebContents(window) ?? webContents; - if (!wc) { - console.error("invalid window for context menu click handler:", window); - return; - } - wc.send("contextmenu-click", menuDef.id); + click: () => { + menuState.hasClick = true; + webContents.send("contextmenu-click", menuDef.id); }, checked: menuDef.checked, enabled: menuDef.enabled, }; if (menuDef.submenu != null) { - menuItemTemplate.submenu = convertMenuDefArrToMenu(webContents, menuDef.submenu); + menuItemTemplate.submenu = convertMenuDefArrToMenu(webContents, menuDef.submenu, menuState); } const menuItem = new electron.MenuItem(menuItemTemplate); menuItems.push(menuItem); @@ -439,19 +436,27 @@ function convertMenuDefArrToMenu( electron.ipcMain.on( "contextmenu-show", (event, workspaceOrBuilderId: string, menuDefArr: ElectronContextMenuItem[]) => { + const webContents = getWebContentsByWorkspaceOrBuilderId(workspaceOrBuilderId); + if (!webContents) { + console.error("invalid window for context menu:", workspaceOrBuilderId); + event.returnValue = true; + return; + } if (menuDefArr.length === 0) { + webContents.send("contextmenu-click", null); event.returnValue = true; return; } fireAndForget(async () => { - const webContents = getWebContentsByWorkspaceOrBuilderId(workspaceOrBuilderId); - if (!webContents) { - console.error("invalid window for context menu:", workspaceOrBuilderId); - return; - } - - const menu = convertMenuDefArrToMenu(webContents, menuDefArr); - menu.popup(); + const menuState = { hasClick: false }; + const menu = convertMenuDefArrToMenu(webContents, menuDefArr, menuState); + menu.popup({ + callback: () => { + if (!menuState.hasClick) { + webContents.send("contextmenu-click", null); + } + }, + }); }); event.returnValue = true; } diff --git a/emain/preload.ts b/emain/preload.ts index 107f7d5c5d..7acdf2e73a 100644 --- a/emain/preload.ts +++ b/emain/preload.ts @@ -21,7 +21,8 @@ contextBridge.exposeInMainWorld("api", { showWorkspaceAppMenu: (workspaceId) => ipcRenderer.send("workspace-appmenu-show", workspaceId), showBuilderAppMenu: (builderId) => ipcRenderer.send("builder-appmenu-show", builderId), showContextMenu: (workspaceId, menu) => ipcRenderer.send("contextmenu-show", workspaceId, menu), - onContextMenuClick: (callback) => ipcRenderer.on("contextmenu-click", (_event, id) => callback(id)), + onContextMenuClick: (callback: (id: string | null) => void) => + ipcRenderer.on("contextmenu-click", (_event, id: string | null) => callback(id)), downloadFile: (filePath) => ipcRenderer.send("download", { filePath }), openExternal: (url) => { if (url && typeof url === "string") { diff --git a/frontend/app/store/contextmenu.test.ts b/frontend/app/store/contextmenu.test.ts index c74eb7359c..975529c780 100644 --- a/frontend/app/store/contextmenu.test.ts +++ b/frontend/app/store/contextmenu.test.ts @@ -2,7 +2,11 @@ import { describe, expect, it, vi } from "vitest"; describe("ContextMenuModel", () => { it("initializes only when getInstance is called", async () => { + let contextMenuCallback: (id: string | null) => void; const onContextMenuClick = vi.fn(); + onContextMenuClick.mockImplementation((callback) => { + contextMenuCallback = callback; + }); const getApi = vi.fn(() => ({ onContextMenuClick, showContextMenu: vi.fn(), @@ -24,5 +28,107 @@ describe("ContextMenuModel", () => { expect(firstInstance).toBe(secondInstance); expect(getApi).toHaveBeenCalledTimes(1); expect(onContextMenuClick).toHaveBeenCalledTimes(1); + expect(contextMenuCallback).toBeTypeOf("function"); + }); + + it("runs select and close callbacks after item handler", async () => { + let contextMenuCallback: (id: string | null) => void; + const showContextMenu = vi.fn(); + const onContextMenuClick = vi.fn((callback) => { + contextMenuCallback = callback; + }); + const getApi = vi.fn(() => ({ + onContextMenuClick, + showContextMenu, + })); + const workspace = { oid: "workspace-1" }; + + vi.resetModules(); + vi.doMock("./global", () => ({ + atoms: { workspace: "workspace", builderId: "builderId" }, + getApi, + globalStore: { + get: vi.fn((atom) => { + if (atom === "workspace") { + return workspace; + } + return "builder-1"; + }), + }, + })); + + const { ContextMenuModel } = await import("./contextmenu"); + const model = ContextMenuModel.getInstance(); + const order: string[] = []; + const itemClick = vi.fn(() => { + order.push("item"); + }); + const onSelect = vi.fn((item) => { + order.push(`select:${item.label}`); + }); + const onClose = vi.fn((item) => { + order.push(`close:${item?.label ?? "null"}`); + }); + + model.showContextMenu( + [{ label: "Open", click: itemClick }], + { stopPropagation: vi.fn() } as any, + { onSelect, onClose } + ); + const menuId = showContextMenu.mock.calls[0][1][0].id; + contextMenuCallback(menuId); + + expect(order).toEqual(["item", "select:Open", "close:Open"]); + expect(itemClick).toHaveBeenCalledTimes(1); + expect(onSelect).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("runs cancel and close callbacks when no item is selected", async () => { + let contextMenuCallback: (id: string | null) => void; + const showContextMenu = vi.fn(); + const onContextMenuClick = vi.fn((callback) => { + contextMenuCallback = callback; + }); + const getApi = vi.fn(() => ({ + onContextMenuClick, + showContextMenu, + })); + const workspace = { oid: "workspace-1" }; + + vi.resetModules(); + vi.doMock("./global", () => ({ + atoms: { workspace: "workspace", builderId: "builderId" }, + getApi, + globalStore: { + get: vi.fn((atom) => { + if (atom === "workspace") { + return workspace; + } + return "builder-1"; + }), + }, + })); + + const { ContextMenuModel } = await import("./contextmenu"); + const model = ContextMenuModel.getInstance(); + const order: string[] = []; + const onCancel = vi.fn(() => { + order.push("cancel"); + }); + const onClose = vi.fn((item) => { + order.push(`close:${item == null ? "null" : item.label}`); + }); + + model.showContextMenu( + [{ label: "Open", click: vi.fn() }], + { stopPropagation: vi.fn() } as any, + { onCancel, onClose } + ); + contextMenuCallback(null); + + expect(order).toEqual(["cancel", "close:null"]); + expect(onCancel).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledTimes(1); }); }); diff --git a/frontend/app/store/contextmenu.ts b/frontend/app/store/contextmenu.ts index 2c3446a295..89e72c5613 100644 --- a/frontend/app/store/contextmenu.ts +++ b/frontend/app/store/contextmenu.ts @@ -3,9 +3,16 @@ import { atoms, getApi, globalStore } from "./global"; +type ShowContextMenuOpts = { + onSelect?: (item: ContextMenuItem) => void; + onCancel?: () => void; + onClose?: (item: ContextMenuItem | null) => void; +}; + class ContextMenuModel { private static instance: ContextMenuModel; - handlers: Map void> = new Map(); // id -> handler + handlers: Map = new Map(); // id -> item + activeOpts: ShowContextMenuOpts | null = null; private constructor() { getApi().onContextMenuClick(this.handleContextMenuClick.bind(this)); @@ -18,11 +25,19 @@ class ContextMenuModel { return ContextMenuModel.instance; } - handleContextMenuClick(id: string): void { - const handler = this.handlers.get(id); - if (handler) { - handler(); + handleContextMenuClick(id: string | null): void { + const opts = this.activeOpts; + this.activeOpts = null; + const item = id != null ? this.handlers.get(id) : null; + this.handlers.clear(); + if (item == null) { + opts?.onCancel?.(); + opts?.onClose?.(null); + return; } + item.click?.(); + opts?.onSelect?.(item); + opts?.onClose?.(item); } _convertAndRegisterMenu(menu: ContextMenuItem[]): ElectronContextMenuItem[] { @@ -43,7 +58,7 @@ class ContextMenuModel { electronItem.enabled = false; } if (item.click) { - this.handlers.set(electronItem.id, item.click); + this.handlers.set(electronItem.id, item); } if (item.submenu) { electronItem.submenu = this._convertAndRegisterMenu(item.submenu); @@ -53,9 +68,10 @@ class ContextMenuModel { return electronMenuItems; } - showContextMenu(menu: ContextMenuItem[], ev: React.MouseEvent): void { + showContextMenu(menu: ContextMenuItem[], ev: React.MouseEvent, opts?: ShowContextMenuOpts): void { ev.stopPropagation(); this.handlers.clear(); + this.activeOpts = opts; const electronMenuItems = this._convertAndRegisterMenu(menu); const workspace = globalStore.get(atoms.workspace); diff --git a/frontend/types/custom.d.ts b/frontend/types/custom.d.ts index 12e2827119..f75cab3b63 100644 --- a/frontend/types/custom.d.ts +++ b/frontend/types/custom.d.ts @@ -93,7 +93,7 @@ declare global { showWorkspaceAppMenu: (workspaceId: string) => void; // workspace-appmenu-show showBuilderAppMenu: (builderId: string) => void; // builder-appmenu-show showContextMenu: (workspaceId: string, menu: ElectronContextMenuItem[]) => void; // contextmenu-show - onContextMenuClick: (callback: (id: string) => void) => void; // contextmenu-click + onContextMenuClick: (callback: (id: string | null) => void) => void; // contextmenu-click onNavigate: (callback: (url: string) => void) => void; onIframeNavigate: (callback: (url: string) => void) => void; downloadFile: (path: string) => void; // download