Blockly: Proposal: Pluggable common context menu items for BlockSvg

Created on 14 Oct 2019  Â·  8Comments  Â·  Source: google/blockly

Is your feature request related to a problem? Please describe.

To add or remove a common context menu item from the block context menu requires either overriding of the Blockly.BlockSvg.prototype.generateContextMenu method, or adding a custom contextmenu generator to every block.

Describe the solution you'd like

A registry for these common items, which would include all of the standard items too, so that's easier to override/remove these too.

Something like...

Blockly.ContextMenu.addCommonItem(fn, id?)

where fn is a function that generates a menu option (or null if it shouldn't appear) given the block. The id allows you to remove items later. Also a weight could be added the item data to determine the ordering.

So all standard items will be added in this way to, allowing them to be removed easily.

Blockly.BlockSvg.prototype.generateContextMenu will just be a simple fn that iterates the registry and calls the item fns.

Describe alternatives you've considered

The only way we've been able to do this so far is to replace the generateContextMenu, call the original fn and then manipulate the list of options - filtering on the option text.

Additional context

I'm happy to submit a PR for this if everyone is happy with the concept.

developer apis help wanted feature request

Most helpful comment

Agreed. A context object makes more sense. This would mean for a block context menu, the callback can access the workspace through contextObj.workspace.

To your second question, customContextMenu is for showing context menus on individual blocks, and would continue to coexist with this design. One thing that could be added to customContextMenu is the ability to include a weight in each context menu item returned, so we can sort everything by weight (rather than just appending it to the end as we do today).

All 8 comments

I think the idea behind generateContextMenu was that it could be overriden. I have no opinions on whether an addCommonItem api should be added. I'm just providing #2581 for context.

I've implemented this in an example of a more generalised plugin system... https://github.com/google/blockly/issues/3260

We discussed this today and came up with an API we're comfortable with.

Problem statement

There is no API to add a context menu option globally (to all blocks). Adding it to every block individually is a pain, and the alternative is a monkeypatch.

Proposed solution

A registry for context menu options with the following APIs. Note that anywhere I'm using strings (such as contextEnum) we will probably implement with an enum instead.

  • register(ContextMenuRegistryItem)

    • Adds the given item to the registry, using the item's existing ID. Errors if it's malformed or the ID is already in use.

  • unregister(id)

    • Remove the item with the given ID from the registry.

  • getItem(id)

    • Gets the item with the given ID from the registry. The item can then be modified in place.

  • getContextMenuOptions(contextEnum, workspace, block)

    • Gets a list of ContextMenuOptions, which can be passed into Blockly.ContextMenu.populate_.

    • Builds a list of options that are registered for the given context, then filters out any whose preconditionFns return not shown. Called from right-click on a block or workspace.

A ContextMenuRegistryItem contains all of the information about an entry:

  • callback

    • The callback to run when the context menu option is clicked by the user.

  • contextEnum

    • One of workspace or block. Says which context this function expects to be run in. Functions that should run on both the workspace and an individual block may be registered twice: once with each context.

  • displayTextFn(workspace, block)

    • A function to generate the display text for the generated context menu option. Takes in a block and workspace.

  • displayText

    • A string to use as the display text, for static cases. Used if displayTextFn is undefined.

  • preconditionFn(workspace, block):

    • Takes in a block and a workspace and returns one of three values: enabled, disabled, hidden.

    • Is evaluated by the context menu registry while building the list of workspace options or block options.

  • weight

    • A weight to use to order the context menu items. The higher the value, the lower the item is displayed visually on the list.

  • id

    • An ID to use when looking this up in the registry (e.g. to remove it).

A ContextMenuOption is equivalent to an existing "menu option" (generally annotated as an Object currently):

  • text
  • enabled
  • callback

Sample use cases:

Add a context menu option to all blocks

ContextMenuRegisteryItem myItem = new ContextMenuRegistryItem(
  cb, 
  'block', 
  function(block_, workspace_) { return 'some text';}, 
  function(block_, workspace_) { return 'enabled'; } 
  20, 
  'someID');

ContextMenuRegistry.register(myItem);

Add a context menu option that is only enabled on root blocks

ContextMenuRegisteryItem mySecondItem = new ContextMenuRegistryItem(
  cb, 
  'block', 
  function(block_, workspace_) { return 'some text';}, 
  function(block, workspace_) { 
    return block.getParentBlock() ? 'disabled' : 'enabled';
  } 
  19, 
  'someOtherID');

ContextMenuRegistry.register(mySecondItem);

Remove a context menu item

var id = 'some_id_we_published_for_builtin_items';
ContextMenuRegistry.unregister(id);

Why even bother with a class for the ContextMenuRegistryItem? ... it's not clear what the constructor args are meant to be, why not just allow registration using a literal object?...

ContextMenuRegistry.register({
   id: 'someId',
   callback: function() {...},
   weight: 20,
   ... etc ...
})

I'm not convinced it needs to be a class--I just needed a name to use to reference it. It can just be a typedef, which gets us type checking.

This new API seems really good! I love it when old systems get expanded :D

But one thing I'm wondering about is how you would handle adding context menus to new things. For example: What if core wanted to add support for context menus on flyouts, fields, workspace comments, etc? In the current design, all functions are tied strictly to blocks and workspaces.

If you think this is relevant to the discussion, one idea I had for solving this would be to pass a "context" object instead of individual objects. E.g:

@typedef {{
            workspace:Blockly.WorkspaceSvg
            block:Blockly.BlockSvg
         }}

Then as the contextEnum expanded the "context" object could expand as well.

One other question, does this mean that the customContextMenu function will be deprecated and replaced with specialized precondition functions? Or will the per-block level be left alone and this is just meant to handle global options?

Thanks again for the preliminary design! It looks really cool :D

Agreed. A context object makes more sense. This would mean for a block context menu, the callback can access the workspace through contextObj.workspace.

To your second question, customContextMenu is for showing context menus on individual blocks, and would continue to coexist with this design. One thing that could be added to customContextMenu is the ability to include a weight in each context menu item returned, so we can sort everything by weight (rather than just appending it to the end as we do today).

I filed a separate bug to add documentation for this feature, but it will be in the upcoming release and you can try it out now in develop. The default context menu items are now registered using this API so you can view those for an example.

Was this page helpful?
0 / 5 - 0 ratings