feat: command palette for actions and tools - #1072
Conversation
pattern now follows default viewer_setup binding
also fixes the lifetime and binding locations to be more consistent with the default viewer setup and the input event bindings to help panel
Also removes doc level palette key listener, this was designed for when inside a number element for e.g. but not worth also explicitly labels the command type as opposed to infer from optional properties
There was a problem hiding this comment.
Firstly, I think this is an excellent idea for neuroglancer, so thank you for your work. Having a discoverable, searchable command surface is very useful, and exposing it through a catalog is neat.
At GScan, I am using neuroglancer as a 3D tomographic viewer for muon tomography scans, so some of my requirements might diverge from the neuroscience use cases. To that end, I maintain a fork where I expose an imperative control API over the viewer and wire it into our own UI chrome, rather than using neuroglancer's built-in panels and widgets.
The CommandCatalog primitive in this PR sits upstream of that control surface. As an introspective enumeration of everything the viewer can currently do, it is exactly the thing that I otherwise have to hand-maintain as a parallel list.
If accepted, I could enumerate capabilities from the catalog instead of this list, and bind according to our specific UI + UX requirements. The host-built control surface I maintain is a sibling consumer of the catalog, alongside the palette. This would make my fork less divergent from upstream as I could ditch the host-build control surface and just extend the catalog if I need to.
With this in mind, I have left a few suggestions which would let me consume the CommandCatalog without pulling in the palette's DOM/CSS. Nothing changes the behaviour; it just makes the enumeration primitive separable from the UI chrome.
If it's useful, I can open a discussion thread going into more detail about how my fork differs; in short: it keeps neuroglancer's rendering and control internals but strips out essentially all of the built-in UI chrome, with a host app driving everything via the control API interface. I haven't raised this topic upstream before because shipping a fully "headless" neuroglancer package carries a lot of overhead and may well not be useful to anyone else. This MR is the first surface where our interests overlap, so it seemed worth flagging.
Either way, thank you for the work on this PR!
edit: Here is the discussion thread going into more detail about embedding neuroglancer: #1073
| // context is the viewer instance; restoreTool walks its prototype chain | ||
| // to find the registered tool factory. | ||
| return restoreTool(context, toolJson); |
There was a problem hiding this comment.
The comments here notes that context must be the viewer instance as restoreTool can walk its prototype chain for the tool factory, but CommandCatalogContext (L39-44) only declares: globalToolBinder / layerManager / selectedLayer / inputEventBindings. So a caller that satisfies that interface with a plain object would pass type-check but fail to tool restore at runtime.
Could the requirement be made explicit? Either by typing the context as the Viewer or interface which includes factory lookup, or by threading an explicit restoreTool-capable handle through CommandCatalogContext ? Then, constructing the catalog off something other than the viewer is explicitly support or errors at compile time.
There was a problem hiding this comment.
Yes definitely a good idea, I was actually exploring something in https://github.com/MetaCell/neuroglancer/tree/feat/command-match around improving this part. So I'll take this idea on board to try iron something out, thanks!
Thank you very much for the comments and all the context! I'm glad to hear about a different kind of use case, and definitely your experience in managing that kind of system around neuroglancer is really useful here, cheers for bringing it up. I'll wait a bit to see if more feedback comes in and then can take a pass at changes. Please do let me know at that point if the catalog is generic enough to suit your needs, because in theory I think it should be able to be so. On the broader part about a discussion thread re the fork - I can't speak for @jbms or @chrisj but I imagine we'd all be interested to hear what you are doing if you want to write some more details whenever it suits. |
Extract the catalog — CommandCatalog, CommandCatalogContext, collectActionBindings, the CommandPaletteEntry types, and the tool/label helpers — from command_palette.ts into a new command_catalog.ts with no DOM or CSS dependencies. command_palette.ts keeps the Overlay-based CommandPalette UI and bindCommandPalette, importing the catalog and the stylesheet. Previously, importing CommandCatalog for its enumeration transitively pulled in command_palette.css and the Overlay class even when no palette was rendered. Splitting the modules lets the catalog be consumed (and unit-tested) without a DOM, and reused independently of the palette UI. - command_catalog.spec.ts (renamed from command_palette.spec.ts) now imports from command_catalog.js, so the catalog tests no longer depend on the palette module. - default_viewer_setup.ts imports CommandCatalog from command_catalog.js and bindCommandPalette from command_palette.js. No behavioural change.
|
this is a great feature, just a few thoughts for now
|
refactor: split CommandCatalog into a DOM-free command_catalog module
seankmartin
changed the title
|
Hi @Le0C thank you very much for your PR, I merged it in. Whenever you get a chance could you please sign the CLA for this repo? |
|
@seankmartin Sure thing, that is now signed |
Agreed thanks Chris, this all sounds great. On the keybinds do you happen to remember the commit(s) or PR(s)? Very happy to hear ideas, and I'm good to keep shift as a separate modifier. Overall I was just thinking that it might be best if the help panel, tool bindings section, and command palette all used ideally the same function to map the action binding to a user facing keybind/mousebind. I think the command palette and tools probably also want to map the action string to something a bit easier to read, though not sure if the help menu might want to keep the raw action string so at least one place in the UI has it. And keyn doesn't show in the help menu, just the tool bindings section. 
|
| * Persistent, signal-driven catalog of command palette entries. Subscribes to | ||
| * tool-binding and layer changes and rebuilds automatically via | ||
| * animationFrameDebounce so the palette always reflects current viewer state | ||
| * without rebuilding from scratch on every open. |
There was a problem hiding this comment.
Does it actually take any significant amount of time to collect the commands --- I think it may make more sense to always just build the list of commands when the palette opens. Still the refresh logic could be used to keep it up to date while the palette is open, although in most cases it is unlikely to change.
There was a problem hiding this comment.
Thanks, I added this in 05d10d4. I also kept the signals as @Le0C expressed interest in using the command catalog without the palette which seems reasonable to me, and in that case ideally the catalog is up to date. Rebuilding on palette open seems a good safe guard though, just in case a signal was missed.
Grouping is a presentation concern: the command palette and the help panel would reasonably group the same commands in different ways, so the section a command belongs to belongs to whoever is presenting it. A suggested binding on the command was only ever informational, and would drift from whatever binding is actually installed. The shortcut a consumer shows now always comes from the live input event bindings.
RenderedDataPanel registers its per-axis action listeners by iterating AXES_NAMES, so declaring the matching commands from a second local list of axis names left two places to keep in step. Import the same constant and fold the move and rotate generators into one pass over it.
Behaviour had nowhere to live on a plain data record, so each consumer re-derived it: the palette built the `action:<id>` CustomEvent itself, and the catalog translated the registry's `type` discriminant into its own `kind` discriminant to decide which branch to take. A Command now owns its id, label, optional description and how it runs. ActionCommand dispatches the DOM action, CallbackCommand runs a callback, and both take a CommandContext rather than a bare target so that more context (mouse position, originating layer) can be added later without touching every implementation. The registry stores instances and forwards each command's `changed` signal, which replaces the per-command WatchableValue subscription that backed the old `isAvailable`; that property is now a settable `enabled` on the command itself. The catalog's ActionCommandEntry and CommandEntry collapse into a single entry carrying the Command.
The registry lists the commands it was told about, and there is no way to make that list complete: a viewer embedded in another application, or driven from the Python integration, can bind an action without ever registering a command for it. Enumerating only the registry dropped those from the palette, which the previous catalog did show. The catalog now enumerates the registry first, so a registered command keeps its curated label and description, then adds an ActionCommand for each keyboard-bound action the registry does not know, labelled from its action id as before. Tool slots and layer-index actions stay excluded; the catalog contributes its own entries for those.
The command ids in default_commands.ts have to match the action ids the default input event bindings dispatch, and nothing checked that. A typo in either direction is silent: a command whose id no action listens for does nothing when invoked, and a bound action with no command loses its label and description. Assert both directions against the real binding maps, with the tool slots and layer-index actions excluded as dynamic, and the three actions that have no default binding listed explicitly.
Describe what each piece owns: a Command holds identity, presentation and behaviour; the registry holds which commands exist; the catalog turns that plus viewer state into an ordered list with shortcuts attached; the palette renders it. Records why the registry cannot be treated as the complete list of commands, and how a change flows from a registration through to a re-render.
feat: Add CommandRegistry and default command descriptions.
also binds to that command palette in the default setup
The command catalog is now constructed by the viewer, so default_viewer_setup no longer references CommandCatalog directly. The leftover import fails lint:check with no-unused-vars.
fix: remove unused CommandCatalog import
Instead they are directly CallbackCommands
instead store all command actions flat in a catalog, and leave the consumer (currently only palette) to group commands based on the old grouping information
since they can be used standalone
Existing signals for refreshing are kept, because there is interest in the catalog being used outside of the palette - and because although unlikely, in theory a command could change while the palette is open. Rebuilding when opening is another way to help ensure the catalog is up to date in case something was missed.
|
Hopefully this PR now addresses the initial feedback from @Le0C and @jbms, but open to any other thoughts or if anything different on those would be better. For @chrisj I added a keybind for deactivating a tool, currently it's escape which I agree works well though seem tools may use it for another reason. The tool bind should take precedence, in which case I think it's still ok. I didn't add fuzzy find or a recents section as those could come later. I know you also had some ideas for a better palette layout, especially when combined with filtering, happy to bring that in. Another piece (imo for later) is to update the help menu if the palette is merged. Both to make the distinction between the two clearer, and also to make better use of the registry. |
4 participants
2026-07-13.10-33-13.mp4
Adds an automatic command palette to neuroglancer which finds available actions and tools using a very similar system to what @jbms introduced for populating the tool palettes. Although not shown in the video, these actions also respect the panel they were triggered from - which matters for actions like "snap". The primary focus here has been on exploring what is possible to do this as automatically as possible. I don't want to introduce any extra overhead if possible, so all the logic and management can be as self contained to this new system as possible. So I'm very open to any code refactoring suggestions or changes in the architecture of how commands are found and activated.
A quick summary of what added
I'll raise some points here for consideration and some things not addressed or decided upon: