Support dynamic menu items in electron

[kittaakos]

Currently, the menu items are built at application startup and have their static nature. That means, enabled, visible properties are not updated dynamically.

Also, handler enablement is calculated when the application starts.

For now on, I enable everything (#210), just like VSCode and move the command > handler lookup into the click(() function.

See: electron/electron#528

[kittaakos]

electron/electron#846

[kittaakos]

VSCode: https://github.com/Microsoft/vscode/blob/9c27b11bde339d58a8da0e3f6d5f88f7a4fcafa7/src/vs/code/electron-main/menus.ts#L195

[svenefftinge]

I think electron/electron#528 (comment) could be a viable workaround until they support updates in the rendering process.

[kittaakos]

I have noticed that too, but when do you fire the ipcRenderer.sendSync('rebuild-menu');? Maybe, I am just missing something and this should be the workaround.

[svenefftinge]

you are right. Is there a callback on menu show?

[kittaakos]

I was looking for something like that but did not find anything useful.

[kittaakos]

electron/electron#8999

[vince-fugnitto]

@kittaakos I noticed that vscode has the option of having custom menus (much like our browser), would this be a viable option to update our menus to use our browser menus instead so we have more control and customizability over them ?

[kittaakos]

This should be possible to do with the current electron version we use.

node_modules/electron/electron.d.ts:

    /**
     * Emitted when a popup is closed either manually or with menu.closePopup().
     */
    on(event: 'menu-will-close', listener: (event: Event) => void): this;
    once(event: 'menu-will-close', listener: (event: Event) => void): this;
    addListener(event: 'menu-will-close', listener: (event: Event) => void): this;
    removeListener(event: 'menu-will-close', listener: (event: Event) => void): this;
    /**
     * Emitted when menu.popup() is called.
     */
    on(event: 'menu-will-show', listener: (event: Event) => void): this;
    once(event: 'menu-will-show', listener: (event: Event) => void): this;
    addListener(event: 'menu-will-show', listener: (event: Event) => void): this;
    removeListener(event: 'menu-will-show', listener: (event: Event) => void): this;

I propose adding this to our road-map, so finally, we can have the same application menu behavior in the browser and electron.

[renandecarlo]

My workaround was to get mouse cursor position when the user clicks on the menu, and then show it at the coordinates after recreating it with Menu.buildFromTemplate() or the Menu() object.
Basically like that:

let menuCoordinates = {};
contextMenu.on('menu-will-show', () => {
   menuCoordinates = require('electron').screen.getCursorScreenPoint();
});

... then rebuild it with Menu.buildFromTemplate() or new Menu()
... then reopen it at the coordinates we got:

contextMenu.popup({ x: menuCoordinates .x, y: menuCoordinates .y });

It should work with Tray too.

[tsmaeder]

There is an even for when menus in the menu bar are opened (menu-will-show), so in principle, we can update the menus on demand. There are two caveats, though:

1. There is no event for the menu bar itself (obviously, since it's always visible). Right now, the menu bar is modeled as a menu itself: we might want to change that in order to reflect the differences.
2. While you can add an item to a menu on menu-will-show, it will only show up the next time the menu is opened. So we can't use this capability for dynamic menus.

That said, the documentation says we can update the visible, checked and enabled fields dynamically. I'll investigate this.

[tsmaeder]

Unfortunately, changing the aforementioned flags in menu-will-show also does not affect the being shown until the next time it's popped up. So I think we'll have to push changes to enablement instead of pulling them on menu-will-show.

[tsmaeder]

We already have some change notification in place when updating toolbar items upon change of context keys in the case of the VS Code command enablement property. We could extend this support to other cases like selection-dependent commands or the "when" clauses of menu items. This would go a long ways, but there are cases (like enabling the "full-screen" command) where we don't get notification when the underlying condition changes ("fullscreenable" in an Electron Window. We could still fall back to the current behavior or "always enabled" in the cases where we don't have change notifications available.

[pchuong]

Hello, would you consider adding support for dynamic main menu and context menu by creating the menu items on the fly? We have use cases where the number of menu items can change, including visibility, enablement, and label based on the state of our debugger.

My current workaround is to define a large number of placeholder menu items, and then set the visibility depending on the debugger state. In older Theia, there is a way to change the label. But the menu item label property is made private in newer Theia.

[tsmaeder]

@pchuong that is already possible if you don't use the native electron menus. As for the electron case, I don't think it's possible for the reasons I explained in my comments above.

[tsmaeder]

@pchuong not really sure what you mean about the label being private? Seems public in MenuNode, for example. Btw: this issue is about the main application menu.

[tsmaeder]

@kittaakos I don't think we can make "on-demand" menus work for the main menu with what we have in Electron. My approach would be to track handler visibility & enablement and to push updates actively into the Electron main process. What do you think? It would probably be best to introduce change notification into the MenuNode interfaces to decouple from the concrete implementation (when-clauses, etc.)