[Feature Request]: Add a close event to Menu instances

[nathanlesage]

Preflight Checklist

• I have read the Contributing Guidelines for this project.
• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for a feature request that matches the one I want to file, without success.

Problem Description

I have implemented a native context menu for my Electron application and want to listen for clicks on the menu items. The context menus are being generated in the renderer process, then handed over to the main process which displays them, and the main process shall return the ID of the clicked item (or undefined if no item was clicked). For these purposes, the renderer can define menu items with labels, accelerators, and an ID. The main process then attaches to it a click handler that triggers when the user clicks an item.

However, since the renderer process needs some feedback, as the user can keep open the context menu indefinitely, I need to use a promise, which needs to be resolved or rejected at some point. That means: All works fine if the user clicks a context menu item. But if the user clicks outside of the context menu, indicating an abort operation, I still need to resolve/reject the promise so that the renderer process is not sitting there, indefinitely for a promise that will never resolve.

Using the menu-will-close event of the menu does not allow me to cleanly do so, since that event is being fired before the click handler has any chance to resolve the promise so I end up always getting rejections (except my hacky solution, see below).

Proposed Solution

Add a close event (or after-close) that will be fired after any click-handler of the menu has been called (if applicable). This way I can "clean up" more cleanly.

Observe how the below code (in "Alternatives Considered") would change with a close-event:

private async _displayNativeContextMenu (menu: MenuItem[], x: number, y: number): Promise<string|undefined> {
  return await new Promise((resolve, reject) => {
    let resolvedID = undefined // If no item was clicked, resolve with undefined
    for (const item of menu) {
      // Hook onto a function that writes the item.id into the return value
      item.click = () => { resolvedID = item.id }
    }

    const popupMenu = Menu.buildFromTemplate(menu)
    popupMenu.on('close', (event) => {
      resolve(resolvedID) // At this point, resolvedID will be the correct ID if the user clicked an item
    })
   popupMenu.popup({ x: x, y: y })
  })
}

Alternatives Considered

A working solution is to create a return value, have the click handlers write the clicked ID into that, and, after a timeout of 100ms, resolve the promise with whatever ID has been clicked. However, that depends on factors I don't fully trust, and it would be more clean not having to use a timeout.

private async _displayNativeContextMenu (menu: MenuItem[], x: number, y: number): Promise<string|undefined> {
  return await new Promise((resolve, reject) => {
    let resolvedID = undefined // If no item was clicked, resolve with undefined
    for (const item of menu) {
      // Hook onto a function that writes the item.id into the return value
      item.click = () => { resolvedID = item.id }
    }

    const popupMenu = Menu.buildFromTemplate(menu)
    popupMenu.on('menu-will-close', (event) => {
      setTimeout(() => {
        resolve(resolvedID) // resolvedID will contain the correct ID only after a timeout
      }, 100)
    })
   popupMenu.popup({ x: x, y: y })
  })
}

Additional Information

Electron version: 12.0.2

[Prinzhorn]

I'm not entirely sure what your architecture looks like. But what I'm doing is each menu and item in it has an id, I pass the template to the main process and when a click happens I send the ID over to the renderer preload and do this:

let event = new CustomEvent(`context-menu:${menuId}`, { detail: { itemId } });
window.dispatchEvent(event);

and it's the job of each item (a Svelte component) to listen to the event or not (e.g. when the component is unmounted the listener is removed). It think an event-based approach makes more sense for menus than promises.

[nathanlesage]

That would be another alternative, but since several close-events are fired when closing other objects such as BrowserWindow or the app itself, this would be in line with the way Electron APIs work in general. Plus, in my setup I can't really use custom events.