menumanager: context menu for library/group

[Emiliano Heyns]

Does the menu manager have a target for a library/group? I see targets for collections and searches but not for the library/group.

[XY Wong]

You may want to use the `collectionTreeRow` from `getContext` [1] to decide if it's a library or group.


[1] https://github.com/zotero/zotero/blob/59056389077b3fc153d37263c422333bb3d04304/chrome/content/zotero/zoteroPane.js#L4208-L4214

[Emiliano Heyns]

How would I distinguish between an item being right-clicked and a library being right-clicked? In both cases collectionTreeRow would be populated right?

[XY Wong]

The item context menu and collection context menu are different targets ("main/library/item" and "main/library/collection").

[Emiliano Heyns]

Ah, and in the case of the library or group being selected, no collection is selected.

[XY Wong]

Please use `collectionTreeRow.type`.

Actually, you could register a menu with logging to console in its onShowing and inspect the properties of the context object passed to the callback.

[Emiliano Heyns]

Will do.

Is it possible to have a variable number of entries under a submenu?

[Xiangyu Wang]

The (sub)menus need to be registered first and then dynamically set to hidden/visible in the onShowing.

--
You received this message because you are subscribed to the Google Groups "zotero-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to zotero-dev+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/zotero-dev/e658678d-7508-4002-a02e-0e859a2b1294n%40googlegroups.com.

[Emiliano Heyns]

Alright, so I need to guess a sensible number. Not a major problem.

[Emiliano Heyns]

I thought I would use this to put up to 10 items in the submenu, but I can see the submenu onShowing is being called, but the submenu ones aren't. Have I overlooked something?

function selectedAutoExports(mode) {
  const selected = mode === 'collection'
    ? Zotero.getActiveZoteroPane().getSelectedCollection(true)
    : Zotero.getActiveZoteroPane().getSelectedLibraryID()
    return AutoExport.db.all(_ => _.type === mode && _.id === selected)
}
Zotero.MenuManager.registerMenu({
  menuID: `${pluginID}-menu-collection`,
  pluginID,
  target: 'main/library/collection',
  menus: [
    {
      menuType: 'submenu',
      l10nID: 'better-bibtex',
      icon: 'chrome://zotero-better-bibtex/content/skin/bibtex-menu.svg',
      menus: [
        {
          menuType: 'submenu',
          l10nID: 'better-bibtex_preferences_auto-export',
          onShowing: (_event, context) => {
            const type = context.collectionTreeRow.type
            context.setVisible(selectedAutoExports(type).length !== 0)
            Zotero.debug(`submenu ${type}: ${selectedAutoExports(type).length}`)
          },
          menus: Array.from({ length: 10 }).map((_, i) => ({
            menuType: 'menuitem',
            onShowing: (event, context) => {
              const type = context.collectionTreeRow.type
              const aes = selectedAutoExports(type)
              Zotero.debug(`menuitem ${i} ${type}: ${typeof aes[i] !== 'undefined'}`)
              context.setVisible(typeof aes[i] !== 'undefined')
              context.menuElem.setAttribute('label', aes[i]?.path || '[path not set]')
            },
            onCommand: (_event, _context) => {
              const ae = selectedAutoExports('collection')[i]
              if (ae) Zotero.BetterBibTeX.AutoExport.run(ae.path)
            },
          })),
        ],
      },
    ],
  }
)

[XY Wong]

The submenus' onShowing event is only get triggered when its popup is showing, not when the top level popup is showing.

The following code works correctly for me:

```js
Zotero.MenuManager.registerMenu({
    menuID: "1",
    pluginID: "yo...@plugin.id",

    target: "main/library/collection",
    menus: [
      {
        menuType: "submenu",

        l10nID: "menu-print",
        menus: [
          {
            menuType: "submenu",
            onShowing: (_event, context) => {
              Zotero.warn(new Error(`Showing submenu parent`));
            },
            l10nID: "menu-print",

            menus: Array.from({ length: 10 }).map((_, i) => ({
              menuType: "menuitem",
              onShowing: (event, context) => {

                Zotero.warn(new Error(`Showing menu item ${i}`));

              },
            })),
          },
        ],
      },

    ],
  });

```

Btw, it would be nice if you would provide the minimal code for reproduce that can run fully on its own, instead of ones with function calls/vars that are not available in the code snippets.

[Emiliano Heyns]

Totally fair -- it should have been

        const aes = [ { path: 'path 1' }, { path: 'path 2' } ]

        Zotero.MenuManager.registerMenu({
          menuID: '1',
          pluginID: 'yo...@plugin.id',
          target: 'main/library/collection',
          menus: [{
            menuType: 'submenu',

            onShowing: (_event, context) => {
              context.menuElem.setAttribute('label', 'level 1')
              Zotero.warn(new Error('Showing submenu level 1'))

            },
            menus: [{
              menuType: 'submenu',
              onShowing: (_event, context) => {

                context.menuElem.setAttribute('label', 'level 2')
                context.setVisible(aes.length > 0)
                Zotero.warn(new Error('Showing submenu level 2'))

              },
              menus: Array.from({ length: 10 }).map((_, i) => ({
                menuType: 'menuitem',

                onShowing: (_event, context) => {
                  // @ts-expect-error TS2339
                  context.menuElem.setAttribute('label', aes[i]?.path || `${i}`)
                  // @ts-expect-error TS2339
                  context.setVisible(aes.length > i)

                  Zotero.warn(new Error(`Showing menu item ${i}`))
                },
              })),
            }],
          }],

        })

and that does work, so I have some debugging to do.

[Emiliano Heyns]

I am really sorry, but I am still not seeing it in the actual code. I think I have stripped this down to have no dependencies outside the snippet, to me this looks equivalent to the standalone snippet I posted above this:

        function selectedAutoExports(_mode) {
          return [ { path: '/x/y' }, { path: '/foo/bar' } ]

        }
        Zotero.MenuManager.registerMenu({
          menuID: `${pluginID}-menu-collection`,
          pluginID,
          target: 'main/library/collection',
          menus: [
            {
              menuType: 'submenu',
              l10nID: 'better-bibtex',
              icon: 'chrome://zotero-better-bibtex/content/skin/bibtex-menu.svg',
              menus: [
                {
                  menuType: 'submenu',
                  l10nID: 'better-bibtex_preferences_auto-export',
                  onShowing: (_event, context) => {

                    Zotero.warn(new Error(`submenu showing: ${selectedAutoExports('any').length !== 0}`))

                    const type = context.collectionTreeRow.type
                    context.setVisible(selectedAutoExports(type).length !== 0)

                  },
                  menus: Array.from({ length: 10 }).map((_, i) => ({
                    menuType: 'menuitem',

                    onShowing: (event: Event, context) => {
                      Zotero.warn(new Error(`menuitem ${i} showing?`))

                      const type = context.collectionTreeRow.type
                      const aes = selectedAutoExports(type)

                      context.setVisible(aes.length > i)
                      Zotero.warn(new Error(`menuitem ${i} showing: ${aes.length > i}`))

                      context.menuElem.setAttribute('label', aes[i]?.path || '[path not set]')
                    },

                    /*onCommand: (_event: Event, _context) => {

                      const ae = selectedAutoExports('collection')[i]
                      if (ae) Zotero.BetterBibTeX.AutoExport.run(ae.path)

                    }*/,
                  }))/* as MenuItem[] */,
                },
                // { menuType: 'menuitem', l10nID: 'better-bibtex_zotero-pane_show_collection-key', onCommand: (_event, _context) => showPullExportURLs('collection') },
                // { menuType: 'menuitem', l10nID: 'better-bibtex_aux-scanner', onCommand: (_event, _context) => void Zotero.BetterBibTeX.scanAUX('collection') },
                // { menuType: 'menuitem', l10nID: 'better-bibtex_report-errors', onCommand: (_event, _context) => void Zotero.BetterBibTeX.ErrorReport.open('collection') },
              ],
            },
          ],
        })

and I don't see 

menuitem ${i} showing?

where I would have expected it to. I do see 

`submenu showing: true`

[Emiliano Heyns]

I saw a mention on the forums that maybe the missling l10nid plays a role. How do I set l10nargs in the onShowing? Using  context.menuElem.setAttribute?

[XY Wong]

Use [1] `context.setL10nArgs`. It is always recommended to use l10n instead of directly setting attributes like lable.

https://github.com/zotero/zotero/blob/81f80a62e37e1f3b2bda9a576dbd8e6844ba8202/chrome/content/zotero/xpcom/pluginAPI/menuManager.js#L576

[Emiliano Heyns]

I now have this

function selectedAutoExports(type) {
          const selected = type === 'collection'
            ? Zotero.getActiveZoteroPane().getSelectedCollection(true)
            : Zotero.getActiveZoteroPane().getSelectedLibraryID()
          // return AutoExport.db.all(_ => _.type === type && _.id === selected)
          return Array.from({ length: 3 }).map((_, i) => ({ path: `/path/${i}`, type, id: selected }))

        }
        Zotero.MenuManager.registerMenu({
          menuID: '1',
          pluginID: 'yo...@plugin.id',
          target: 'main/library/collection',
          menus: [{
            menuType: 'submenu',

            l10nID: 'better-bibtex',
            onShowing: (_event, _context) => {
              Zotero.debug('Showing submenu level 1')

            },
            menus: [{
              menuType: 'submenu',
              l10nID: 'better-bibtex_preferences_auto-export',
              onShowing: (_event, context) => {
                const type = context.collectionTreeRow.type

                const aes = selectedAutoExports(type)
                context.setVisible(aes.length > 0)
                Zotero.debug('Showing submenu level 2')

              },
              menus: Array.from({ length: 10 }).map((_, i) => ({
                menuType: 'menuitem',

                l10nID: 'better-bibtex_auto-export_path',
                onShowing: (_event, context) => {
                  Zotero.debug(`Showing menu item ${i}`)

                  const type = context.collectionTreeRow.type
                  const aes = selectedAutoExports(type)

                  // @ts-expect-error TS2339
                  context.setL10nArgs({ path: aes[i]?.path || '' })

                  // @ts-expect-error TS2339
                  context.setVisible(aes.length > i)

                },
              })),
            }],
          }],
        })

with

better-bibtex =
  .label = Better BibTeX
better-bibtex_preferences_auto-export = Automatic export

  .label = { better-bibtex_preferences_auto-export }

better-bibtex_auto-export_path = { $path }
  .label = { better-bibtex_auto-export_path }

I think this is fully self-contained? But the menu items are not showing, and I don't see `Showing menu item ${i}` in my logs

[XY Wong]

```

better-bibtex_preferences_auto-export = Automatic export
  .label = { better-bibtex_preferences_auto-export }
better-bibtex_auto-export_path = { $path }
  .label = { better-bibtex_auto-export_path }

```

Same as the situation we discussed in another thread, that your l10n string is replacing the whole innerHTML of the popup and thus there is no sub menus at all.

Remove the `Automatic export` and `{ $path }`.

[Northword]

```ts

type ItemMenu = _ZoteroTypes.MenuManager.MenuData<_ZoteroTypes.MenuManager.LibraryMenuContext>;

      Zotero.MenuManager.registerMenu({

          menuID: '00000000000002',

          pluginID: 'y...@plugin.id',

          target: 'main/library/collection',

          menus: [{

            menuType: 'submenu',

            l10nID: 'better-bibtex',

            onShowing: (_event, _context) => {

              Zotero.debug('Showing submenu level 1')

            },

            menus: [{

              menuType: 'submenu',

              l10nID: 'better-bibtex_preferences_auto-export',

              onShowing: (_event, context) => {

                Zotero.debug('Showing submenu level 2')

              },

              menus: Array.from({ length: 10 }).map((_, i): ItemMenu => ({

                menuType: 'menuitem',

                l10nID: 'better-bibtex_auto-export_path',

                l10nArgs: { path: i },

                onShowing: (_event, context) => {

                  Zotero.debug(`Showing menu item ${i}`)

                },

              })),

            }],

          }],

        })

```

```ftl

better-bibtex =
  .label = Better BibTeX
better-bibtex_preferences_auto-export =

  .label = Automatic export
better-bibtex_auto-export_path =
  .label = { $path }

```

[Emiliano Heyns]

Oh. I am using that construct to prevent duplicate strings piling up in the FTL. So I should probably never have both the toplevel string and attributes in one entry? I thought that it would just prioritize the .label if it was present.

[XY Wong]

You can use both, just keep in mind what they actually do. There are cases like setting the innerHTML of a div element with a `title` attribute that would need both.

And better not to reuse one key for multiple elements unless they are meant to be exactly the same. To prevent duplication, you can use FTL variables for the same string; but not the same key for multiple elements.

[Emiliano Heyns]

They are meant to be exactly the same

I am closer now; with this code

function selectedAutoExports(type: 'collection' | 'library') {

          const selected = type === 'collection'
            ? Zotero.getActiveZoteroPane().getSelectedCollection(true)
            : Zotero.getActiveZoteroPane().getSelectedLibraryID()
          // return AutoExport.db.all(_ => _.type === type && _.id === selected)
          return Array.from({ length: 3 }).map((_, i) => ({ path: `/path/${i}`, type, id: selected }))
        }
        Zotero.MenuManager.registerMenu({

          menuID: `${pluginID}-menu-collection`,
          pluginID,
          target: 'main/library/collection',
          menus: [
            {
              menuType: 'submenu',
              l10nID: 'better-bibtex',
              icon: 'chrome://zotero-better-bibtex/content/skin/bibtex-menu.svg',
              menus: [
                {
                  menuType: 'submenu',

                  l10nID: 'better-bibtex_collection-menu_auto-export',

                  onShowing: (_event, context) => {
                    const type = context.collectionTreeRow.type
                    const aes = selectedAutoExports(type)
                    context.setVisible(aes.length > 0)

                  },
                  menus: Array.from({ length: 10 }).map((_, i) => ({
                    menuType: 'menuitem',

                    l10nID: 'better-bibtex_collection-menu_auto-export_path',
                    onShowing: (event: Event, context: any) => {

                      const type = context.collectionTreeRow.type
                      const aes = selectedAutoExports(type)

                      context.setVisible(aes.length > i)
                      Zotero.debug(`menuitem ${i} = ${JSON.stringify(aes[i] || {})}`)
                      context.setL10nArgs(aes[i] || {})
                    },
                    onCommand: (_event: Event, context) => {
                      const type = context.collectionTreeRow.type
                      const ae = selectedAutoExports(type)[i]
                      if (ae) Zotero.BetterBibTeX.AutoExport.run(ae.path)
                    },
                  })) as MenuItem[],

                },
                // { menuType: 'menuitem', l10nID: 'better-bibtex_zotero-pane_show_collection-key', onCommand: (_event, _context) => showPullExportURLs('collection') },
                // { menuType: 'menuitem', l10nID: 'better-bibtex_aux-scanner', onCommand: (_event, _context) => void Zotero.BetterBibTeX.scanAUX('collection') },
                // { menuType: 'menuitem', l10nID: 'better-bibtex_report-errors', onCommand: (_event, _context) => void Zotero.BetterBibTeX.ErrorReport.open('collection') },
              ],
            },
          ],
        })

and this ftl

better-bibtex =
  .label = Better BibTeX

better-bibtex_collection-menu_auto-export =
  .label = Automatic export
better-bibtex_collection-menu_auto-export_path =
  .label = { $path }

the menu options do all show, but the menu items all show "{ $path }"

with the log showing

zotero(3)(+0000000): menuitem 0 = {"path":"/path/0","type":"library","id":1}

zotero(3)(+0000000): menuitem 1 = {"path":"/path/1","type":"library","id":1}

zotero(3)(+0000001): menuitem 2 = {"path":"/path/2","type":"library","id":1}

zotero(3)(+0000000): menuitem 3 = {}

zotero(3)(+0000000): menuitem 4 = {}

zotero(3)(+0000001): menuitem 5 = {}

zotero(3)(+0000000): menuitem 6 = {}

zotero(3)(+0000000): menuitem 7 = {}

zotero(3)(+0000000): menuitem 8 = {}

zotero(3)(+0000001): menuitem 9 = {}

[Emiliano Heyns]

On the other hand, setting the 'label' attribute works, so I could just do that instead

[Emiliano Heyns]

BTW I specify an icon on the first submenu, but in my tests it appears in all things under it, yet *not* on the submenu itself.

[XY Wong]

`context.setL10nArgs(JSON.stringify(aes[i] || {}));`

It only accepts a json format string. The type definition lib might need to be corrected.

[XY Wong]

The icon (should) actually be visible on the top level submenu, though because of platform-specific rules on MacOS it is hidden. In the design of the app we don't allow any icons on the context menu.

It is a bug that the icon appears in its child elements. Before the fix, you can use an invalid icon path for the child elements as a temporary workaround. Thanks for the report.

[Emiliano Heyns]

https://github.com/zotero/zotero/blob/81f80a62e37e1f3b2bda9a576dbd8e6844ba8202/chrome/content/zotero/xpcom/pluginAPI/menuManager.js#L84

and
https://github.com/zotero/zotero/blob/81f80a62e37e1f3b2bda9a576dbd8e6844ba8202/chrome/content/zotero/xpcom/pluginAPI/menuManager.js#L443
seem to suggest it takes an object though

[XY Wong]

That's the initial value of `l10nArgs`, which takes an object because it's already part of an object passed for configuration, while the function `setL10nArgs` accepts only a string. It was designed like this because for function calls we prioritize passing primitives instead of objects whenever possible. We probably should allow initial `l10nArgs` as string, though. That's a good point.