-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Description
Check for duplicates
- I have searched for similar issues before opening a new one.
Problem
While working on keyboard navigation experimentation, it was noted that there are many situations where context menu items overlap with keyboard shortcuts, and that it would be desirable to have a unified "action" API, where an action represents something which could be a keyboard shortcut or a context menu item—or both, which might be the most common case.
It is difficult to write actions of this form at the moment because the shortcut and context menu item APIs are quite different—for example:
- The precondition function for shortcuts returns a boolean but for menu items an enum.
- The callback for shortcuts takes a
Workspaceand keyboard event, but for menu options a context object and pointer event.
Request
Create a new Action API, intended to eventually supercede the existing shortcut and context menu item APIs, that provides a single unified way of specifying an action.
Specification
These are features of the API that seem to be essential ("must") or highly desirable ("should"), at least in the opinion of @rachel-fenichel and/or @cpcallen
Actions can be context menu items and/or keyboard shortcuts
- An action must be able to be displayed and invoked as a context menu item (if it so chooses):
- The action must be able to dynamically specify the menu text, and this must be localisable.
- The action must be able to decide in which contexts the menu item is shown in the context menu, and whether it is shown as enabled or disabled. (See below for more on context)
- When the action can also be invoked as a keyboard shortcut, the menu must display (at least one of) the shortcut's current key bindings.
- An action must be able to be invoked via a keyboard shortcut (if it so chooses):
- It should be possible for the action to specify which key or keys it is bound to by default.
- Ideally it would be possible to specify multiple keyboard bindings for a single action, both when specifying the default bindings and when configuring bindings at runtime.
- e.g., macOS supports Emacs-style cursor keys
^Fand^Bas synonyms for→and←respectively; it should be possible for a Blockly action to do the same.
- e.g., macOS supports Emacs-style cursor keys
- Ideally it would be possible to specify multiple keyboard bindings for a single action, both when specifying the default bindings and when configuring bindings at runtime.
- It should be possible to modify what key(s) an action is bound to at runtime, to allow customisation by the developer or (if the developer provides a suitable configuration system) by the user.
- The action must be able to specify how the shortcut is described (e.g. in shortcuts help).
- This description should be dynamic,
- and must be localisable.
- It must be possible to bind to different keys on different platforms (Windows / macOS / linux / etc.)
- It should be easy and convenient to specify platform-specific shortcuts.
- @rachel-fenichel specified "where a shortcut is intended to be on the meta key, [the API] should take in that information and map to the appropriate key for the platform, rather than requiring the creator of the shortcut to enumerate all options."
- That seems convenient, but beware that "meta' is itself an actual modifier—one of the many modifers supported by the web platform and that there will be shortcuts that do not map on to the same non-modifier keys on different platforms (e.g. ⌘Q vs Alt-F4).
- It should be easy and convenient to specify platform-specific shortcuts.
- It should be possible to localise shortcut bindings, especially for non-modified alphabetic keys (e.g.
Ifor insert). - Keys must be bound appropriately to ensure they do not interfere with other parts of the page.
- Key bindings should not be made at document level by default, it may be possible to do so.
- It must be possible to invoke different actions using the same key depending on where focus is.
- E.g. to have enter bound to the edit-field action on fields but to the insert-block action on connections.
- This might be achevied by having very fine-grained key binding, or by having a method on each action decide if it is applicable to the current focus node, with bindings tried in priority order until an applicable one is found.
- This requirement is to obviate the need to create actions that do completely unrelated things in different contexts.
- It should be possible for the action to specify which key or keys it is bound to by default.
- It must be posible to invoke different actions using the same key based on a mode.
- E.g. to have the
enterkey finish a block move if one was in progress, but otherwise perform some other (context-dependent) action.
- E.g. to have the
- Where there are multiple actions bound to the same key, it must be possible to determine which action would be invoked without actually invoking any of them.
- This is needed so we can show context-senitive keyboard shortcut help.
Note that, though perhaps unlikely, an action could be neither a context menu item nor bound to a keyboard shortcut, but such an action could still be invoked programmatically—perhaps by another action.
- A single API interaction (e.g. adding to an action registry) should be sufficient to make a new action appear in the context menu and be invocable by keyboard shortcut in any applicable context by default.
- It must be possible for developers to be able to disable some or all actions in particular workspaces; alternatively, it should be possible to to provide actions that are only applicable to particular workspaces.
- E.g., none of the default actions should be enabled in the the minimap workspace, but it might have some special actions of its own.
Action methods and context information
- Actions must be able to specify a set of callback methods to be invoked as needed (e.g. display text, precondition, do-action, etc.)
- Where reasonable, the same method on the action should be called regardless of whether the action is being invoked via keyboard shortcut or context menu item; this must be the case for the ultimate do-action callback.
- Methods on the action object must be passed context information, akin to (but not necessarily implemented the same way as) the existing
ContextMenuRegistry.Scopeinterface.- The context info must distinguish whether the action is being displayed and/or invoked as a context menu item or via a keyboard shortcut.
- The context info must include the workspace
- The context info must include the particular object to which the context menu is attached or (when invoked by keybinding) which had focus.
- The context info should allow the method to easily distinguish whether the context is a workspace, comment, block or connection—or none of those.
- Being able to do
instanceofchecks might suffice to satisfy this requirement, though we probably want to be able to distinguish any kind of IFocusNode, even for ones which are defined by interface type rather than superclass.
- Being able to do
- When an action is used as a context menu item, its methods methods must be passed information about (see note 1):
- whether the mouse or keyboard was used to open the menu, and if the action is selected
- whether the mouse or keyboard was used to select the menu item.
- When the context is a workspace, the context info must include relevant coordinates:
- of the location of the mouse or workspace cursor (as appropriate) at the time the context menu was opened, or
- of the location of the workspace cursor at the time the shortcut was invoked.
- Methods on action objects must be called with
thisset to the action object. - The set of methods on an action, and the context info passed to those methods, should be chosen to meet two goals—in the following order of priority:
- The API design should be as clean and simple as possible.
- The API design should facilitate porting existing shortcuts and context menu items to the new API—in the ideal case making it possible to replace the existing APIs with simple wrappers around the new one.
Note 1: This will enable different actions to be shown in the context menu, and for those actions to behave in different, appropriate ways depending on whether the user is using keyboard navigation or not. For example, [the "Edit (→)" option](RaspberryPiFoundation/blockly-keyboard-experimentation#132 would probalby only appear when the menu was opened via the keyboard.
Additional thoughts
This section is non-normative, and contians my thoughts about what the actual API design might look like.
High-level organisation:
- The existing context menu registry seems like a reasonably good model.
- It's probably best that actions be defined via an
Actioninterface, akin to the existingContextMenuRegistry.RegistryIteminterface, since that allows maximum flexibility, incuding defining actions via a simple object literal.- If we find that many of our actions have common code, we could create a common base class and define actions using (singleton) instances of subclasses of that class—but that would just be how we create things to pass to the API, rather something about the API itself.
On the details of an Action interface:
- It's probably best that action have an "ID" rather than a "name", and that we make clear that the ID is used only internally (e.g. for registering/unregistering) and should never be displayed to the user.
- In writing context menu items for the keyboard navigation action menu, @cpcallen has noted that many menu items seem to end up with somewhat similar code in their
displayTextandpreconditionFncallbacks. It seems like it might be advantageous to combine these in to a single method (perhapsgetMenuItem, that would return something like… where a bare{ displayText: string | HTMLElement, disabled?: boolean, } string | HTMLElement | null
stringorHTMLElementbeing thedisplayTextof an implicitly-enabled menu option, andnullindicating that the action should not be included in the context menu at all (i.e. that it is hidden). - Similarly, it's not clear that the separate
preconditionFnin the existingKeyboardShortcutinterface is especially useful. Why not just put any precondition checks at the top of the main callback, returningfalseif they fail?- [edit:] Actually, there is one possible good reason: in order to be able to determine which of several shortcuts bound to a single key will be invoked if that key is pressed in the current context (without actually invoking any of them), in order to provide context-sensitive shortcut help.
- We should try to choose better method names this time:
callbackis not very meaningful, andpreconditionFnhas unnecessary hungarian notation.
On context parameters:
- Wherever appropriate we should probably pass the same context info in the same way to each API method that needs context.
- The main bit of context info should probably be a single
IFocusNode, being the currently focused item (when the action is being used as a keyboard shortcut) or object to which the context menu is attached (when being used as a context menu item).- We should think carefully about whether it's possible that there would be items with context menus that are not focusable—but that seems unlikely.
- It's necessary to supply coordinates for actions invoked via workspace context menu / workspace cursor action menu.
- This is used e.g. to place new workspace comments.
- In
ContextMenuRegistry.RegistryItemthis is done by passing aPointerEvent, but this was observed to complicate the code to use the context menu using the keyboard, because we had to create syntheticPointerEvents in order to programmatically invoke context menu items. - For backward compatibility, we may still want to pass
PointerEvents(when we have them) in the context. This would help make it possible to replace the existingContextMenuRegistryAPI with a wrapper around the Action API. - But consumption of the
PointerEventshould otherwise be discouraged, perhaps by making it undocumented (not unlike it being previously undocumented inLegacyContextMenuOption).
- Some thought needs to be given to how to handle modifier keys. Passing the
KeyboardEventis probably not a good approach, especially as context menu items should also be able to support modifier keys, ideally with the display text changing dynamically as the user presses and releases the modifier keys, as it does in macOS. - One possibility would be to refresh the menu each time the set of depressed modifiers changes, calling each action's
getMenuItemmethod again at that point.
Alternatives considered
- Having all actions be subclasses of a (probably
abstract)Actionbase class.- This would seem to be unnecessarily constraining, and possibly make defining new actions more verbose (each would require at least a class, plus an instantiation of that class, instead of just an object literal.
- It's not obvious that there would be any code to put on the base class.
Additional context
No response