Menu manager: auto-hide submenus

[Emiliano Heyns]

Would it be possible to auto-hide submenu's when they have no visible entries? Right now I am checking twice (once for the entry, once for the menu holding it) and it gets pretty repetitive.

[XY Wong]

It would be helpful if you could describe the use case with more details.

In general, the submenu item visibility check hook is only triggered when the submenu popup shows. If you already set the menu item that holds the submenus to be invisible, this hook will not be triggered, so there is no real duplication in code execution.

For a coherent UI experience, in some cases it would be better to show the submenu with a row like "no available options" instead of hiding them. This depends on your use case, though.

[Emiliano Heyns]

The use case is that I have dynamic menus, and that I end up having the check "should menu entry X be shown" in both the submenu and in the menuitem onShowing. I'm not so much worried that it will run twice (these are fast checks) but that I have to have the check at two places. There is not a simple check that pre-decides whether "based on this independent criterium, I know at least one of my children will be visible" holds or not.

If you want a "no available options" menu item you still have to do these checks to see if it should show, so the "no available options" menu item should decide for itself that it should be shown, and then the containing submenu would just follow. "No available options" is a menu item like any other, which some logic to determine it should be shown.

I can't think of a scenario where you would ever want to see the submenu even where there are 0 shown items under it.

I was thinking of something like this

function

autoHi

de

(context) {

let anyChildVisible = false

for

(const child of context.menus || []) {

if

(typeof child.onShowing !== "function") {

hasVisibleChild = true

break

}

let thisChildVisible = true

child.

onShowing

(event, {

...context,

setVisible: (visible) => {

thisChildVisible = !!visible

}

})

if

(thisChildVisible) {

anyVisibleChild = true

break

}

}

context.

setVisible

(anyVisibleChild)

}

but I am not sure the context can be shared this way

[Emiliano Heyns]

or rather

function

autoH

ide

(context) {

let anyChildVisible = false

for

(const child of context.menus || []) {

if

(typeof child.onShowing !== 'function') {

anyChildVisible = true

break

}

let thisChildVisible = true

child.

onShowing

(event, {

...context,

setVisible: visible => {

thisChildVisible = !!visible

},

})

if

(thisChildVisible) {

anyChildVisible = true

break

}

}

context.

setVisible

(anyChildVisible)

}

[XY Wong]

Thanks for the reply.

Technically, I assume you can have a hasSubMenu() method and call it in the menu that holds the submenus and set its visibility, so that you don't have to manually set visibility for each submenu item if the submenu items do not have their own separate conditions for visibility.

The above suggested code could work, but that can also break the promise that onShowing is only called when a menu is actually about to show. To update if a menu holds submenus is visibie, it must be at the moment the menu is about to show, and by then the submenus are not yet about to show. Calling the onShowing hook of the submenus basically breaks this promise.

Back to the topic, if we are not worried about the redundant execution, having the visibility check as one function and use it for multiple / different menu items doesn't feel so stressful to me.

[Emiliano Heyns]

(doesn't look like it works)

[Emiliano Heyns]

We obviously differ on how stressful this is, but if my request is unlikely (or not possible) to be accepted, I'll deal with it.

[Emiliano Heyns]

> The above suggested code could work, but that can also break the promise that onShowing is only called when a menu is actually about to show

The only way this sentence makes sense to me is of the parent submenu can structurally decide beforehand whether at least on of it's children is going to be visible without repeating the logic that happens in the child onShowing. That does not hold for me. 

[XY Wong]

Had the agent quickly draft an example to illustrate my idea:

<code>

(() => {
// Plain userland helpers over the existing API -- nothing new needed
let showIf = predicate => (ev, context) => context.setVisible(!!predicate(context));

// One predicate per entry; adding a 10th submenu is one more key here
let visibility = {
newItemNote: ({ items }) => items?.length && items.every(it => it.isRegularItem()),
showFile: ({ items }) => items?.length && items.every(it => it.isAttachment()),
};
let anyVisible = context => Object.values(visibility).some(p => p(context));

let menuID = Zotero.MenuManager.registerMenu({
menuID: "example-conditional-menu",
pluginID: "exa...@example.example",
target: "main/library/item",
menus: [{
menuType: "submenu",
// "More Options…" (zotero.ftl)
l10nID: "menu-custom-group-submenu",
// Hides itself when no entry would be visible, regardless of entry count,
// without ever invoking the children's onShowing hooks
onShowing: showIf(anyVisible),
menus: [
{
menuType: "menuitem",
// "New Item Note" (zotero.ftl)
l10nID: "menu-new-item-note",
onShowing: showIf(visibility.newItemNote),
onCommand: (ev, context) => {
Zotero.debug("Regular-item action: " + context.items.map(it => it.id).join(", "));
},
},
{
menuType: "menuitem",
// "Show File" (zotero.ftl)
l10nID: "menu-file-show-file",
onShowing: showIf(visibility.showFile),
onCommand: (ev, context) => {
Zotero.debug("Attachment action: " + context.items.map(it => it.id).join(", "));
},
},
],
}],
});
Zotero.debug("Registered: " + menuID);
return menuID;
})();
</code>

[Emiliano Heyns]

Ah. Yes. That would work, but this is where our aesthetic preferences differ. I like the logic to sit together with the context in which it applies (that is, in the menu item config. To me this just doesn't call the actual onShowing method but does call the onShowing logic, and if `visibility` has a function for a menuitem I comment out, the menu could show when it should not. But I will take this under consideration. It may be the least undesirable option.

[XY Wong]

Understand. Probably then define the submenus as your own abstraction (e.g. class / interface) and inline the visibility logic there, then construct the actual object passing to API registration call, where you can still use the anyVisible-like onShowing for the parent menu.