Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a custom label for menu items, overriding the command name #570

Open
JasonWeill opened this issue Mar 31, 2023 · 2 comments
Open

Allow a custom label for menu items, overriding the command name #570

JasonWeill opened this issue Mar 31, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@JasonWeill
Copy link
Contributor

Problem

Found while working on a fix for jupyter/notebook#6806.

When I create a menu item based on a command, the command's label is the only possible label for the menu item:

return this._commands.label(this.command, this.args);

I would like to override this label to, for example, use a shorter one for a compact menu.

Proposed Solution

Add an optional property called label to Menu.IItemOptions. If present, the label in IItemOptions takes precedence over the label for the command.

@JasonWeill JasonWeill changed the title Allow devs to use a different label in menu items, overriding the command name Allow a custom label for menu items, overriding the command name Mar 31, 2023
@krassowski
Copy link
Member

Thank you for opening this. It looks like a reasonable additive change, I do not see any downsides.

I would like to override this label to, for example, use a shorter one for a compact menu.

This is currently accomplished by functional labels, e.g. for command palette we use longer label by checking for args['isPalette'] as in the terminal:create-new command. There are two problems with this solution:

  • command has to know about its uses
  • we do not receive isSomething from most emitters. Once we rework commands to allow them to receive the event that triggered them it would no longer be a problem as potentially it could enable something like:
    {
      label: (args) => (
        args.event & findMenuForEvent(args.event).?id == 'myCompactMenu'
          ? 'shortLabel'
          : 'longLabel'
      )
    }

The first problem is a design question - should menus be:

  • a) simple with commands being omniscient, or
  • b) allowed to take responsibility for labels, reducing the responsibilities of commands.

(b) would imply small changes to where localisation of label would need to happen (in package with command or package with menu, only of practical significance when reusing command in third-party package), but I think it is reasonable.

@fcollonval
Copy link
Member

Some thoughts, if we want to improve application performance, we should move towards having a no-code description of entry points such as commands in menus or toolbars. This means that we indeed have two paths for customization, allow the command consumer (i.e. menus or toolbars) to override the default command attribute (low code approach) or we go with a code approach like the one suggested by Mike above. But this definitely converge to design questions raised above (and not only for menus):

  • Omniscient commands
  • Customization by consumer

The second one is easier and more customizable (you may not have the power to customize the label of command from an external source). The first one ease code understanding by having a single source of truth.

From my experience with the introduction of customization through settings in core and extension, I lean towards the second approach (customization by consumer).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants