From d923d18f86ee4068011b9589350406e7f83c60da Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 20 Jul 2026 10:53:08 +0300 Subject: [PATCH 1/7] feat: Add CommandRegistry and default command descriptions. --- src/ui/command_catalog.spec.ts | 24 ++- src/ui/command_catalog.ts | 98 ++++++----- src/ui/command_palette.ts | 2 + src/ui/command_registry.spec.ts | 122 ++++++++++++++ src/ui/command_registry.ts | 160 ++++++++++++++++++ src/ui/default_commands.ts | 283 ++++++++++++++++++++++++++++++++ src/ui/default_viewer_setup.ts | 2 + src/viewer.ts | 6 + 8 files changed, 655 insertions(+), 42 deletions(-) create mode 100644 src/ui/command_registry.spec.ts create mode 100644 src/ui/command_registry.ts create mode 100644 src/ui/default_commands.ts diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index 19480e561a..6ca0ffbc7e 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -14,12 +14,13 @@ * limitations under the License. */ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; import { collectActionBindings, CommandCatalog, type CommandCatalogContext, } from "#src/ui/command_catalog.js"; +import { CommandRegistry } from "#src/ui/command_registry.js"; import { EventActionMap } from "#src/util/event_action_map.js"; import { Signal } from "#src/util/signal.js"; import type { InputEventBindings } from "#src/viewer.js"; @@ -42,9 +43,18 @@ function makeInputEventBindings( const noopSignal = { add: () => () => {} }; +// Registries created by makeContext, disposed after each test. +const activeRegistries: CommandRegistry[] = []; + +afterEach(() => { + while (activeRegistries.length > 0) activeRegistries.pop()!.dispose(); +}); + function makeContext( inputEventBindings = makeInputEventBindings(new EventActionMap()), + commandRegistry = new CommandRegistry(), ): CommandCatalogContext { + activeRegistries.push(commandRegistry); return { globalToolBinder: { changed: noopSignal, @@ -59,6 +69,7 @@ function makeContext( }, selectedLayer: {}, inputEventBindings, + commandRegistry, } as unknown as CommandCatalogContext; } @@ -111,10 +122,15 @@ describe("collectActionBindings", () => { }); describe("CommandCatalog.filter", () => { - // With empty bindings the catalog contains only the two supplemental commands: - // "Edit JSON State" and "Screenshot". + // Seed the registry with two commands so the catalog surfaces exactly + // "Edit JSON State" and "Screenshot" as its flat entries. function makeCatalog() { - return new CommandCatalog(makeContext()); + const registry = new CommandRegistry(); + registry.registerAction({ id: "edit-json-state", label: "Edit JSON State" }); + registry.registerAction({ id: "screenshot", label: "Screenshot" }); + return new CommandCatalog( + makeContext(makeInputEventBindings(new EventActionMap()), registry), + ); } it("returns all commands for an empty query", () => { diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index aae27d53a1..4c3b64fd6d 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -16,6 +16,7 @@ import type { LayerManager, SelectedLayerState } from "#src/layer/index.js"; import { UserLayer } from "#src/layer/index.js"; +import type { CommandRegistry } from "#src/ui/command_registry.js"; import { getMatchingTools, restoreTool, @@ -39,16 +40,14 @@ export interface CommandCatalogContext { layerManager: LayerManager; selectedLayer: SelectedLayerState; inputEventBindings: InputEventBindings; + /** + * Authoritative source of the flat command set. Its command-kind entries are + * enumerated directly; the input bindings are consulted only to annotate each + * command with its current shortcut, not to discover which commands exist. + */ + commandRegistry: CommandRegistry; } -const SUPPLEMENTAL_COMMANDS: readonly { - actionId: ActionIdentifier; - label: string; -}[] = [ - { actionId: "edit-json-state", label: "Edit JSON State" }, - { actionId: "screenshot", label: "Screenshot" }, -]; - export interface ActionBinding { readonly actionId: ActionIdentifier; readonly eventAction: EventAction; @@ -57,6 +56,8 @@ export interface ActionBinding { interface CommandPaletteEntryBase { readonly label: string; readonly shortcut: string; + /** Optional grouping section, carried through from a registered command. */ + readonly category?: string; } // Dispatched as an `action:` DOM event, exactly as the keyboard @@ -66,7 +67,16 @@ export interface ActionCommandEntry extends CommandPaletteEntryBase { readonly actionId: ActionIdentifier; } -// Runs a callback directly (no DOM action exists for it). +// A registered command that runs a callback. Unlike `execute`, it carries the +// registry's stable `id` so consumers can correlate it back to the registry. +export interface CommandEntry extends CommandPaletteEntryBase { + readonly kind: "command"; + readonly id: ActionIdentifier; + readonly invoke: () => void; +} + +// Runs an anonymous callback directly (no DOM action and no registry identity — +// e.g. a per-layer toggle or an unbound tool activation). export interface ExecuteCommandEntry extends CommandPaletteEntryBase { readonly kind: "execute"; readonly execute: () => void; @@ -80,6 +90,7 @@ export interface GroupCommandEntry extends CommandPaletteEntryBase { export type CommandPaletteEntry = | ActionCommandEntry + | CommandEntry | ExecuteCommandEntry | GroupCommandEntry; @@ -95,13 +106,6 @@ function formatKeyStroke(stroke: string): string { .join("+"); } -function actionIdToLabel(actionId: ActionIdentifier): string { - return actionId - .split("-") - .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) - .join(" "); -} - function isKeyboardEvent(normalizedId: NormalizedEventIdentifier): boolean { return ( !normalizedId.includes("mouse") && @@ -270,6 +274,9 @@ export class CommandCatalog extends RefCounted { this.registerDisposer( context.layerManager.layersChanged.add(debouncedRebuild), ); + this.registerDisposer( + context.commandRegistry.changed.add(debouncedRebuild), + ); this.rebuild(); } @@ -279,17 +286,10 @@ export class CommandCatalog extends RefCounted { layerManager, selectedLayer, inputEventBindings, + commandRegistry, } = this.context; const commands: CommandPaletteEntry[] = []; - // "Deactivate Active Tool" is always present — harmless no-op when nothing is active. - commands.push({ - kind: "action", - label: "Deactivate Active Tool", - shortcut: "", - actionId: "deactivate-active-tool", - }); - // Hierarchical layer actions — each group entry opens a sub-palette of layers. // The first 9 layers carry their digit-key shortcuts so users can see they // still work directly from the keyboard without opening the sub-palette. @@ -347,20 +347,42 @@ export class CommandCatalog extends RefCounted { ); } - for (const { actionId, eventAction } of bindings) { - if (/^tool-[A-Z]$/.test(actionId)) continue; - // Layer-index actions are replaced by hierarchical group entries above. - if (/^(toggle|select|toggle-pick)-layer-\d+$/.test(actionId)) continue; - - const label = actionIdToLabel(actionId); - const shortcut = formatKeyStroke( - friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), - ); - commands.push({ kind: "action", label, shortcut, actionId }); - } - - for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { - commands.push({ kind: "action", label, shortcut: "", actionId }); + // Flat commands come from the registry. A command's shortcut is whatever + // binding is currently installed for its id (or its suggested default), + // shown for reference only. + for (const command of commandRegistry.values()) { + if (command.isAvailable !== undefined && !command.isAvailable.value) { + continue; + } + const shortcut = + shortcutByAction.get(command.id) ?? + (command.defaultBinding !== undefined + ? formatKeyStroke(friendlyEventIdentifier(command.defaultBinding)) + : ""); + const { label, category } = command; + switch (command.type) { + case "action": + commands.push({ + kind: "action", + label, + shortcut, + category, + actionId: command.id, + }); + break; + case "callback": { + const invoke = command.invoke; + commands.push({ + kind: "command", + label, + shortcut, + category, + id: command.id, + invoke: () => invoke(), + }); + break; + } + } } const toolQueryResult = parseToolQuery("+"); diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index bee4a0743e..e84c6aa3fd 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -279,6 +279,8 @@ export class CommandPalette extends Overlay { if (command.kind === "execute") { command.execute(); + } else if (command.kind === "command") { + command.invoke(); } else if (command.kind === "action") { this.actionDispatchTarget.dispatchEvent( new CustomEvent(`action:${command.actionId}`, { diff --git a/src/ui/command_registry.spec.ts b/src/ui/command_registry.spec.ts new file mode 100644 index 0000000000..ee7c5a197b --- /dev/null +++ b/src/ui/command_registry.spec.ts @@ -0,0 +1,122 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, expect, it } from "vitest"; +import { CommandRegistry } from "#src/ui/command_registry.js"; +import { WatchableValue } from "#src/trackable_value.js"; + +describe("CommandRegistry", () => { + it("registers and retrieves a command by id", () => { + const registry = new CommandRegistry(); + registry.registerAction({ + id: "screenshot", + label: "Screenshot", + description: "Capture a screenshot.", + }); + expect(registry.has("screenshot")).toBe(true); + expect(registry.get("screenshot")?.label).toBe("Screenshot"); + registry.dispose(); + }); + + it("stamps the command type explicitly per registrar", () => { + const registry = new CommandRegistry(); + registry.registerAction({ id: "a", label: "A" }); + registry.registerCallback({ id: "c", label: "C", invoke: () => {} }); + expect(registry.get("a")?.type).toBe("action"); + expect(registry.get("c")?.type).toBe("callback"); + registry.dispose(); + }); + + it("invokes a callback command's callback", () => { + const registry = new CommandRegistry(); + let ran = false; + registry.registerCallback({ + id: "c", + label: "C", + invoke: () => { + ran = true; + }, + }); + const command = registry.get("c"); + if (command?.type === "callback") command.invoke(); + expect(ran).toBe(true); + registry.dispose(); + }); + + it("enumerates commands independent of any binding", () => { + const registry = new CommandRegistry(); + registry.registerAction({ id: "a", label: "A" }); + registry.registerAction({ id: "b", label: "B" }); + expect([...registry.values()].map((c) => c.id)).toStrictEqual(["a", "b"]); + registry.dispose(); + }); + + it("throws on duplicate id", () => { + const registry = new CommandRegistry(); + registry.registerAction({ id: "dup", label: "First" }); + expect(() => + registry.registerAction({ id: "dup", label: "Second" }), + ).toThrow(/already registered/); + registry.dispose(); + }); + + it("unregisters via the returned disposer", () => { + const registry = new CommandRegistry(); + const dispose = registry.registerAction({ id: "temp", label: "Temp" }); + expect(registry.has("temp")).toBe(true); + dispose(); + expect(registry.has("temp")).toBe(false); + registry.dispose(); + }); + + it("dispatches changed on register and unregister", () => { + const registry = new CommandRegistry(); + let count = 0; + registry.changed.add(() => ++count); + const dispose = registry.registerAction({ id: "x", label: "X" }); + expect(count).toBe(1); + dispose(); + expect(count).toBe(2); + registry.dispose(); + }); + + it("dispatches changed when a command's availability changes", () => { + const registry = new CommandRegistry(); + const isAvailable = new WatchableValue(true); + registry.registerAction({ id: "x", label: "X", isAvailable }); + let count = 0; + registry.changed.add(() => ++count); + isAvailable.value = false; + expect(count).toBe(1); + registry.dispose(); + }); + + it("stops observing availability after unregister", () => { + const registry = new CommandRegistry(); + const isAvailable = new WatchableValue(true); + const dispose = registry.registerAction({ + id: "x", + label: "X", + isAvailable, + }); + dispose(); + let count = 0; + registry.changed.add(() => ++count); + isAvailable.value = false; + expect(count).toBe(0); + registry.dispose(); + }); +}); diff --git a/src/ui/command_registry.ts b/src/ui/command_registry.ts new file mode 100644 index 0000000000..d8b59438d6 --- /dev/null +++ b/src/ui/command_registry.ts @@ -0,0 +1,160 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file Global, binding-independent registry of viewer commands. + * + * A command is declared once, with a stable id, a pretty label, and an optional + * help description. Any key binding is looked up separately and shown alongside; + * it is not the source of truth for which commands exist. + * + * The registry is owned by the viewer so commands can be registered before — or + * without — any UI chrome. The command palette and help panel are consumers of + * the same surface; see {@link CommandCatalog}. + */ + +import type { ActionIdentifier } from "#src/util/event_action_map.js"; +import { RefCounted } from "#src/util/disposable.js"; +import { Signal } from "#src/util/signal.js"; +import type { WatchableValueInterface } from "#src/trackable_value.js"; + +interface CommandInfoBase { + /** Stable, serialisable identifier, e.g. "toggle-scale-bar". */ + readonly id: ActionIdentifier; + /** Human-readable name shown in the palette / help panel. */ + readonly label: string; + /** + * Optional longer help text describing what the command does. Surfaced by + * hosts that can afford more than a label (help panel, tooltips). + */ + readonly description?: string; + /** Optional flat category / section for grouping in a host surface. */ + readonly category?: string; + /** + * Optional suggested key binding, e.g. "shift+keyc". Purely informational at + * the registry level — installing it into the input bindings is a consumer + * concern. A command with no `defaultBinding` and no live binding is still a + * first-class member of the registry. + */ + readonly defaultBinding?: string; + /** + * Optional observable of whether the command is currently usable. When it + * changes the registry dispatches `changed`, so consumers can re-enumerate + * "what's usable now" without polling. + */ + readonly isAvailable?: WatchableValueInterface; +} + +/** + * A command backed by a DOM action: invoking it dispatches `action:`, + * exactly as the equivalent keyboard shortcut would. `id` doubles as the action + * id, so existing actions need no extra wiring. + */ +export interface ActionCommandInfo extends CommandInfoBase { + readonly type: "action"; +} + +/** + * A command that runs a callback directly, for commands with no corresponding + * DOM action (e.g. host-registered commands). + */ +export interface CallbackCommandInfo extends CommandInfoBase { + readonly type: "callback"; + readonly invoke: (payload?: unknown) => unknown; +} + +/** + * A registered command. The `type` discriminant is stated explicitly by the + * registrant rather than inferred from which optional fields are present, so + * new command types can be added without changing how existing ones are read. + */ +export type CommandInfo = ActionCommandInfo | CallbackCommandInfo; + +export type CommandType = CommandInfo["type"]; + +/** + * Per-viewer registry of {@link CommandInfo}. Registration returns a disposer + * that unregisters the command, so feature code can add commands for the + * lifetime of a layer / control and clean up automatically. + */ +export class CommandRegistry extends RefCounted { + private readonly commands = new Map(); + private readonly availabilityDisposers = new Map< + ActionIdentifier, + () => void + >(); + + /** Dispatched when a command is added/removed, or its availability changes. */ + readonly changed = new Signal(); + + /** Registers an action-backed command. See {@link ActionCommandInfo}. */ + registerAction(options: Omit): () => void { + return this.register({ type: "action", ...options }); + } + + /** Registers a callback command. See {@link CallbackCommandInfo}. */ + registerCallback(options: Omit): () => void { + return this.register({ type: "callback", ...options }); + } + + /** Registers a command. Throws on duplicate `id`. Returns a disposer. */ + register(command: CommandInfo): () => void { + const { id } = command; + if (this.commands.has(id)) { + throw new Error(`Command already registered: ${JSON.stringify(id)}`); + } + this.commands.set(id, command); + const { isAvailable } = command; + if (isAvailable !== undefined) { + this.availabilityDisposers.set( + id, + isAvailable.changed.add(() => this.changed.dispatch()), + ); + } + this.changed.dispatch(); + return () => this.unregister(id); + } + + unregister(id: ActionIdentifier): void { + if (!this.commands.delete(id)) return; + const disposer = this.availabilityDisposers.get(id); + if (disposer !== undefined) { + disposer(); + this.availabilityDisposers.delete(id); + } + this.changed.dispatch(); + } + + get(id: ActionIdentifier): CommandInfo | undefined { + return this.commands.get(id); + } + + has(id: ActionIdentifier): boolean { + return this.commands.has(id); + } + + /** Iterates every registered command, regardless of current availability. */ + values(): IterableIterator { + return this.commands.values(); + } + + disposed() { + for (const disposer of this.availabilityDisposers.values()) disposer(); + this.availabilityDisposers.clear(); + this.commands.clear(); + super.disposed(); + } +} diff --git a/src/ui/default_commands.ts b/src/ui/default_commands.ts new file mode 100644 index 0000000000..3c3a92c1b2 --- /dev/null +++ b/src/ui/default_commands.ts @@ -0,0 +1,283 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file Declarations of the built-in viewer commands. + * + * Each entry names an existing DOM action (`id` === the `action:` id dispatched + * by the default input-event bindings) and gives it an explicit, human-readable + * label and help description. The registry — not the bindings — is now the + * authoritative list of commands; the shortcut shown for each command is looked + * up from whatever binding happens to be installed (see {@link CommandCatalog}). + * + * Commands whose behaviour is per-entity or otherwise dynamic (layer toggles, + * tool activation) are contributed by the catalog at enumeration time and are + * intentionally *not* declared here. + * + * This is the built-in *seed* set, not a required registry. It exists only + * because these commands correspond to DOM actions that predate the registry. + * Feature code should NOT add entries here; instead register commands + * colocated with the feature, for its own lifetime, e.g. + * + * this.registerDisposer( + * viewer.commandRegistry.registerCallback({ + * id: "clip.addPlane", + * label: "Add Clip Plane", + * invoke: () => this.addPlane(), + * }), + * ); + * + * `registerAction` / `registerCallback` each return a disposer, so commands may + * come and go with the feature (e.g. per-layer). `CommandRegistry` — not this + * file — is the authoritative, runtime-enumerable list. + */ + +import type { + ActionCommandInfo, + CommandRegistry, +} from "#src/ui/command_registry.js"; + +// Every built-in command is action-backed (dispatches `action:`); the type +// is stamped by `registerAction` at registration time. +type BuiltinCommand = Omit; + +const CATEGORY_VIEW = "View"; +const CATEGORY_NAVIGATION = "Navigation"; +const CATEGORY_ANNOTATION = "Annotation"; +const CATEGORY_LAYERS = "Layers"; +const CATEGORY_STATE = "State"; +const CATEGORY_TOOLS = "Tools"; + +const AXES = ["X", "Y", "Z"] as const; + +// Directional position nudges (arrow keys / , . / [ ] in the data panels). +function axisMoveCommands(): BuiltinCommand[] { + const commands: BuiltinCommand[] = []; + for (const axis of AXES) { + const lower = axis.toLowerCase(); + commands.push( + { + id: `${lower}-`, + label: `Move −${axis}`, + description: `Move the view one step in the −${axis} direction.`, + category: CATEGORY_NAVIGATION, + }, + { + id: `${lower}+`, + label: `Move +${axis}`, + description: `Move the view one step in the +${axis} direction.`, + category: CATEGORY_NAVIGATION, + }, + ); + } + return commands; +} + +// Relative rotations about each axis (r / e and shift+arrow keys). +function axisRotateCommands(): BuiltinCommand[] { + const commands: BuiltinCommand[] = []; + for (const axis of AXES) { + const lower = axis.toLowerCase(); + commands.push( + { + id: `rotate-relative-${lower}-`, + label: `Rotate −${axis}`, + description: `Rotate the view a small amount about the ${axis} axis (negative direction).`, + category: CATEGORY_NAVIGATION, + }, + { + id: `rotate-relative-${lower}+`, + label: `Rotate +${axis}`, + description: `Rotate the view a small amount about the ${axis} axis (positive direction).`, + category: CATEGORY_NAVIGATION, + }, + ); + } + return commands; +} + +const STATIC_COMMANDS: readonly BuiltinCommand[] = [ + // View toggles. + { + id: "toggle-show-slices", + label: "Toggle Slices in 3D", + description: "Show or hide the cross-section slices in the 3D view.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-scale-bar", + label: "Toggle Scale Bar", + description: "Show or hide the scale bar overlay.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-axis-lines", + label: "Toggle Axis Lines", + description: "Show or hide the axis line indicators.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-orthographic-projection", + label: "Toggle Orthographic Projection", + description: + "Switch the 3D view between perspective and orthographic projection.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-default-annotations", + label: "Toggle Bounding Box", + description: "Show or hide the default bounding-box annotations.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-show-statistics", + label: "Toggle Statistics", + description: "Show or hide the rendering statistics panel.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-layout", + label: "Toggle Layout", + description: "Cycle the data panel layout.", + category: CATEGORY_VIEW, + }, + { + id: "toggle-layout-alternative", + label: "Toggle Alternative Layout", + description: "Cycle the alternative data panel layout.", + category: CATEGORY_VIEW, + }, + { + id: "help", + label: "Show Help", + description: "Open the keyboard and mouse bindings help panel.", + category: CATEGORY_VIEW, + }, + // Navigation. + { + id: "snap", + label: "Snap to Axis", + description: + "Snap the view orientation to the nearest axis-aligned orientation.", + category: CATEGORY_NAVIGATION, + }, + { + id: "zoom-in", + label: "Zoom In", + description: "Zoom the view in.", + category: CATEGORY_NAVIGATION, + }, + { + id: "zoom-out", + label: "Zoom Out", + description: "Zoom the view out.", + category: CATEGORY_NAVIGATION, + }, + { + id: "depth-range-decrease", + label: "Decrease Depth Range", + description: "Decrease the visible depth range of the 3D projection.", + category: CATEGORY_NAVIGATION, + }, + { + id: "depth-range-increase", + label: "Increase Depth Range", + description: "Increase the visible depth range of the 3D projection.", + category: CATEGORY_NAVIGATION, + }, + { + id: "t-", + label: "Previous Timestep", + description: "Step backward one frame along the time axis.", + category: CATEGORY_NAVIGATION, + }, + { + id: "t+", + label: "Next Timestep", + description: "Step forward one frame along the time axis.", + category: CATEGORY_NAVIGATION, + }, + // Layers / segmentation. + { + id: "add-layer", + label: "Add Layer", + description: "Add a new layer to the viewer.", + category: CATEGORY_LAYERS, + }, + { + id: "recolor", + label: "Randomize Colors", + description: "Assign a new random color seed to segmentation layers.", + category: CATEGORY_LAYERS, + }, + { + id: "clear-segments", + label: "Clear Selected Segments", + description: "Deselect all currently selected segments.", + category: CATEGORY_LAYERS, + }, + // Annotation. + { + id: "finish-annotation", + label: "Finish Annotation", + description: "Complete the annotation currently being drawn.", + category: CATEGORY_ANNOTATION, + }, + { + id: "undo-annotation-step", + label: "Undo Annotation Step", + description: "Undo the last point added to the in-progress annotation.", + category: CATEGORY_ANNOTATION, + }, + // State — these have no default key binding; before the registry they were + // special-cased so the palette could surface them at all. + { + id: "edit-json-state", + label: "Edit JSON State", + description: "Open an editor for the raw viewer JSON state.", + category: CATEGORY_STATE, + }, + { + id: "screenshot", + label: "Screenshot", + description: "Capture a screenshot of the current view.", + category: CATEGORY_STATE, + }, + // Tools. + { + id: "deactivate-active-tool", + label: "Deactivate Active Tool", + description: "Deactivate whichever tool is currently active.", + category: CATEGORY_TOOLS, + }, +]; + +/** + * Registers the built-in commands into `registry`. Called once during default + * viewer setup. The registry is owned (and disposed) by the viewer, so no + * disposers are returned here — the commands live for the viewer's lifetime. + */ +export function registerDefaultCommands(registry: CommandRegistry): void { + for (const command of STATIC_COMMANDS) { + registry.registerAction(command); + } + for (const command of axisMoveCommands()) { + registry.registerAction(command); + } + for (const command of axisRotateCommands()) { + registry.registerAction(command); + } +} diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index 8adeb5e8b3..062b40c424 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -17,6 +17,7 @@ import { StatusMessage } from "#src/status.js"; import { CommandCatalog } from "#src/ui/command_catalog.js"; import { bindCommandPalette } from "#src/ui/command_palette.js"; +import { registerDefaultCommands } from "#src/ui/default_commands.js"; import { bindDefaultCopyHandler, bindDefaultPasteHandler, @@ -64,6 +65,7 @@ export function setupDefaultViewer(options?: Partial) { bindDefaultCopyHandler(viewer); bindDefaultPasteHandler(viewer); + registerDefaultCommands(viewer.commandRegistry); const catalog = viewer.registerDisposer(new CommandCatalog(viewer)); bindCommandPalette(viewer, catalog); diff --git a/src/viewer.ts b/src/viewer.ts index ce9b115946..14d8c5953d 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -94,6 +94,7 @@ import { SelectionDetailsPanel } from "#src/ui/selection_details.js"; import { SidePanelManager } from "#src/ui/side_panel.js"; import { StateEditorDialog } from "#src/ui/state_editor.js"; import { StatisticsDisplayState, StatisticsPanel } from "#src/ui/statistics.js"; +import { CommandRegistry } from "#src/ui/command_registry.js"; import { GlobalToolBinder, LocalToolBinder } from "#src/ui/tool.js"; import { MultiToolPaletteDropdownButton, @@ -1175,6 +1176,11 @@ export class Viewer extends RefCounted implements ViewerState { new GlobalToolBinder(this.toolInputEventMapBinder, this.toolPalettes), ); + // Global, binding-independent registry of viewer commands. Populated with the + // built-in commands during default viewer setup; feature code and hosts may + // register additional commands against it. + public commandRegistry = this.registerDisposer(new CommandRegistry()); + public toolBinder = this.registerDisposer( new LocalToolBinder(this, this.globalToolBinder), ); From 312ea9001b67d98584d19d21d01c4a81d313846d Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 14 Aug 2026 11:11:28 +0300 Subject: [PATCH 2/7] refactor: drop category and defaultBinding from CommandInfo 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. --- src/ui/command_catalog.ts | 12 ++---------- src/ui/command_registry.ts | 9 --------- src/ui/default_commands.ts | 35 ----------------------------------- 3 files changed, 2 insertions(+), 54 deletions(-) diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 4c3b64fd6d..f760b795f5 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -56,8 +56,6 @@ export interface ActionBinding { interface CommandPaletteEntryBase { readonly label: string; readonly shortcut: string; - /** Optional grouping section, carried through from a registered command. */ - readonly category?: string; } // Dispatched as an `action:` DOM event, exactly as the keyboard @@ -354,19 +352,14 @@ export class CommandCatalog extends RefCounted { if (command.isAvailable !== undefined && !command.isAvailable.value) { continue; } - const shortcut = - shortcutByAction.get(command.id) ?? - (command.defaultBinding !== undefined - ? formatKeyStroke(friendlyEventIdentifier(command.defaultBinding)) - : ""); - const { label, category } = command; + const shortcut = shortcutByAction.get(command.id) ?? ""; + const { label } = command; switch (command.type) { case "action": commands.push({ kind: "action", label, shortcut, - category, actionId: command.id, }); break; @@ -376,7 +369,6 @@ export class CommandCatalog extends RefCounted { kind: "command", label, shortcut, - category, id: command.id, invoke: () => invoke(), }); diff --git a/src/ui/command_registry.ts b/src/ui/command_registry.ts index d8b59438d6..1b43bd260f 100644 --- a/src/ui/command_registry.ts +++ b/src/ui/command_registry.ts @@ -41,15 +41,6 @@ interface CommandInfoBase { * hosts that can afford more than a label (help panel, tooltips). */ readonly description?: string; - /** Optional flat category / section for grouping in a host surface. */ - readonly category?: string; - /** - * Optional suggested key binding, e.g. "shift+keyc". Purely informational at - * the registry level — installing it into the input bindings is a consumer - * concern. A command with no `defaultBinding` and no live binding is still a - * first-class member of the registry. - */ - readonly defaultBinding?: string; /** * Optional observable of whether the command is currently usable. When it * changes the registry dispatches `changed`, so consumers can re-enumerate diff --git a/src/ui/default_commands.ts b/src/ui/default_commands.ts index 3c3a92c1b2..11e3b6725d 100644 --- a/src/ui/default_commands.ts +++ b/src/ui/default_commands.ts @@ -54,13 +54,6 @@ import type { // is stamped by `registerAction` at registration time. type BuiltinCommand = Omit; -const CATEGORY_VIEW = "View"; -const CATEGORY_NAVIGATION = "Navigation"; -const CATEGORY_ANNOTATION = "Annotation"; -const CATEGORY_LAYERS = "Layers"; -const CATEGORY_STATE = "State"; -const CATEGORY_TOOLS = "Tools"; - const AXES = ["X", "Y", "Z"] as const; // Directional position nudges (arrow keys / , . / [ ] in the data panels). @@ -73,13 +66,11 @@ function axisMoveCommands(): BuiltinCommand[] { id: `${lower}-`, label: `Move −${axis}`, description: `Move the view one step in the −${axis} direction.`, - category: CATEGORY_NAVIGATION, }, { id: `${lower}+`, label: `Move +${axis}`, description: `Move the view one step in the +${axis} direction.`, - category: CATEGORY_NAVIGATION, }, ); } @@ -96,13 +87,11 @@ function axisRotateCommands(): BuiltinCommand[] { id: `rotate-relative-${lower}-`, label: `Rotate −${axis}`, description: `Rotate the view a small amount about the ${axis} axis (negative direction).`, - category: CATEGORY_NAVIGATION, }, { id: `rotate-relative-${lower}+`, label: `Rotate +${axis}`, description: `Rotate the view a small amount about the ${axis} axis (positive direction).`, - category: CATEGORY_NAVIGATION, }, ); } @@ -115,56 +104,47 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ id: "toggle-show-slices", label: "Toggle Slices in 3D", description: "Show or hide the cross-section slices in the 3D view.", - category: CATEGORY_VIEW, }, { id: "toggle-scale-bar", label: "Toggle Scale Bar", description: "Show or hide the scale bar overlay.", - category: CATEGORY_VIEW, }, { id: "toggle-axis-lines", label: "Toggle Axis Lines", description: "Show or hide the axis line indicators.", - category: CATEGORY_VIEW, }, { id: "toggle-orthographic-projection", label: "Toggle Orthographic Projection", description: "Switch the 3D view between perspective and orthographic projection.", - category: CATEGORY_VIEW, }, { id: "toggle-default-annotations", label: "Toggle Bounding Box", description: "Show or hide the default bounding-box annotations.", - category: CATEGORY_VIEW, }, { id: "toggle-show-statistics", label: "Toggle Statistics", description: "Show or hide the rendering statistics panel.", - category: CATEGORY_VIEW, }, { id: "toggle-layout", label: "Toggle Layout", description: "Cycle the data panel layout.", - category: CATEGORY_VIEW, }, { id: "toggle-layout-alternative", label: "Toggle Alternative Layout", description: "Cycle the alternative data panel layout.", - category: CATEGORY_VIEW, }, { id: "help", label: "Show Help", description: "Open the keyboard and mouse bindings help panel.", - category: CATEGORY_VIEW, }, // Navigation. { @@ -172,75 +152,63 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ label: "Snap to Axis", description: "Snap the view orientation to the nearest axis-aligned orientation.", - category: CATEGORY_NAVIGATION, }, { id: "zoom-in", label: "Zoom In", description: "Zoom the view in.", - category: CATEGORY_NAVIGATION, }, { id: "zoom-out", label: "Zoom Out", description: "Zoom the view out.", - category: CATEGORY_NAVIGATION, }, { id: "depth-range-decrease", label: "Decrease Depth Range", description: "Decrease the visible depth range of the 3D projection.", - category: CATEGORY_NAVIGATION, }, { id: "depth-range-increase", label: "Increase Depth Range", description: "Increase the visible depth range of the 3D projection.", - category: CATEGORY_NAVIGATION, }, { id: "t-", label: "Previous Timestep", description: "Step backward one frame along the time axis.", - category: CATEGORY_NAVIGATION, }, { id: "t+", label: "Next Timestep", description: "Step forward one frame along the time axis.", - category: CATEGORY_NAVIGATION, }, // Layers / segmentation. { id: "add-layer", label: "Add Layer", description: "Add a new layer to the viewer.", - category: CATEGORY_LAYERS, }, { id: "recolor", label: "Randomize Colors", description: "Assign a new random color seed to segmentation layers.", - category: CATEGORY_LAYERS, }, { id: "clear-segments", label: "Clear Selected Segments", description: "Deselect all currently selected segments.", - category: CATEGORY_LAYERS, }, // Annotation. { id: "finish-annotation", label: "Finish Annotation", description: "Complete the annotation currently being drawn.", - category: CATEGORY_ANNOTATION, }, { id: "undo-annotation-step", label: "Undo Annotation Step", description: "Undo the last point added to the in-progress annotation.", - category: CATEGORY_ANNOTATION, }, // State — these have no default key binding; before the registry they were // special-cased so the palette could surface them at all. @@ -248,20 +216,17 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ id: "edit-json-state", label: "Edit JSON State", description: "Open an editor for the raw viewer JSON state.", - category: CATEGORY_STATE, }, { id: "screenshot", label: "Screenshot", description: "Capture a screenshot of the current view.", - category: CATEGORY_STATE, }, // Tools. { id: "deactivate-active-tool", label: "Deactivate Active Tool", description: "Deactivate whichever tool is currently active.", - category: CATEGORY_TOOLS, }, ]; From 9760c2e2a8e2ef40c10c2d4d97dc703c80ae716e Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 14 Aug 2026 11:12:13 +0300 Subject: [PATCH 3/7] refactor: derive the axis command ids from AXES_NAMES 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. --- src/ui/default_commands.ts | 54 +++++++++++++++----------------------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/src/ui/default_commands.ts b/src/ui/default_commands.ts index 11e3b6725d..0693bc8057 100644 --- a/src/ui/default_commands.ts +++ b/src/ui/default_commands.ts @@ -49,49 +49,40 @@ import type { ActionCommandInfo, CommandRegistry, } from "#src/ui/command_registry.js"; +import { AXES_NAMES } from "#src/util/geom.js"; // Every built-in command is action-backed (dispatches `action:`); the type // is stamped by `registerAction` at registration time. type BuiltinCommand = Omit; -const AXES = ["X", "Y", "Z"] as const; - -// Directional position nudges (arrow keys / , . / [ ] in the data panels). -function axisMoveCommands(): BuiltinCommand[] { +// Directional position nudges and relative rotations, one pair per axis (arrow +// keys / , . and r / e / shift+arrow keys in the data panels). The ids are +// derived from the same AXES_NAMES that RenderedDataPanel derives its action +// listeners from, so the two cannot drift apart. +function axisCommands(): BuiltinCommand[] { const commands: BuiltinCommand[] = []; - for (const axis of AXES) { - const lower = axis.toLowerCase(); + for (const axis of AXES_NAMES) { + const upper = axis.toUpperCase(); commands.push( { - id: `${lower}-`, - label: `Move −${axis}`, - description: `Move the view one step in the −${axis} direction.`, + id: `${axis}-`, + label: `Move −${upper}`, + description: `Move the view one step in the −${upper} direction.`, }, { - id: `${lower}+`, - label: `Move +${axis}`, - description: `Move the view one step in the +${axis} direction.`, + id: `${axis}+`, + label: `Move +${upper}`, + description: `Move the view one step in the +${upper} direction.`, }, - ); - } - return commands; -} - -// Relative rotations about each axis (r / e and shift+arrow keys). -function axisRotateCommands(): BuiltinCommand[] { - const commands: BuiltinCommand[] = []; - for (const axis of AXES) { - const lower = axis.toLowerCase(); - commands.push( { - id: `rotate-relative-${lower}-`, - label: `Rotate −${axis}`, - description: `Rotate the view a small amount about the ${axis} axis (negative direction).`, + id: `rotate-relative-${axis}-`, + label: `Rotate −${upper}`, + description: `Rotate the view a small amount about the ${upper} axis (negative direction).`, }, { - id: `rotate-relative-${lower}+`, - label: `Rotate +${axis}`, - description: `Rotate the view a small amount about the ${axis} axis (positive direction).`, + id: `rotate-relative-${axis}+`, + label: `Rotate +${upper}`, + description: `Rotate the view a small amount about the ${upper} axis (positive direction).`, }, ); } @@ -239,10 +230,7 @@ export function registerDefaultCommands(registry: CommandRegistry): void { for (const command of STATIC_COMMANDS) { registry.registerAction(command); } - for (const command of axisMoveCommands()) { - registry.registerAction(command); - } - for (const command of axisRotateCommands()) { + for (const command of axisCommands()) { registry.registerAction(command); } } From dcbffee192d8ee4d321f8d5374f01e49b3e36bdd Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 14 Aug 2026 11:13:02 +0300 Subject: [PATCH 4/7] refactor: replace CommandInfo with a Command class Behaviour had nowhere to live on a plain data record, so each consumer re-derived it: the palette built the `action:` 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. --- src/ui/command.ts | 125 +++++++++++++++++++++++++++++++ src/ui/command_catalog.spec.ts | 5 +- src/ui/command_catalog.ts | 68 +++++------------ src/ui/command_palette.ts | 10 +-- src/ui/command_registry.spec.ts | 116 +++++++++++++++------------- src/ui/command_registry.ts | 129 +++++++++----------------------- src/ui/default_commands.ts | 59 +++++++-------- src/ui/default_viewer_setup.ts | 2 +- src/viewer.ts | 2 +- 9 files changed, 278 insertions(+), 238 deletions(-) create mode 100644 src/ui/command.ts diff --git a/src/ui/command.ts b/src/ui/command.ts new file mode 100644 index 0000000000..4d72688651 --- /dev/null +++ b/src/ui/command.ts @@ -0,0 +1,125 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file A command: something the user can invoke, named independently of how it + * happens to be bound. + * + * A command owns its identity (`id`), how it presents (`label`, `description`) + * and how it runs (`invoke`). Bindings, ordering and grouping belong to the + * consumers that present it. See `docs/concepts/commands.rst` for how commands, + * {@link CommandRegistry} and {@link CommandCatalog} divide up the work. + */ + +import type { ActionIdentifier } from "#src/util/event_action_map.js"; +import { NullarySignal } from "#src/util/signal.js"; + +/** + * Stable, serialisable identifier of a command, e.g. "toggle-scale-bar". For an + * {@link ActionCommand} this is also the DOM action id, hence the shared type. + */ +export type CommandId = ActionIdentifier; + +/** + * What a command is told about the invocation. An interface rather than a bare + * target so that more context (mouse position, originating layer, …) can be + * added later without touching every implementation. + */ +export interface CommandContext { + /** + * Element the invocation is attributed to. For the command palette this is + * whichever element had focus when the palette was opened, so that a command + * acts on the panel the user was in. + */ + readonly dispatchTarget: EventTarget; +} + +/** + * Base class for commands. Subclass it to define how the command runs; the + * registry stores instances and consumers read `label`/`description` and call + * {@link invoke}. + */ +export abstract class Command { + /** + * Dispatched when anything a consumer renders or filters on changes. Bundling + * these into one signal means a consumer subscribes once per command rather + * than once per mutable property. Currently only {@link enabled} changes. + */ + readonly changed = new NullarySignal(); + + private enabled_ = true; + + constructor( + readonly id: CommandId, + readonly label: string, + readonly description?: string, + ) {} + + /** + * Whether the command can currently be invoked. Consumers are expected to + * omit disabled commands (the catalog does). Nothing built in flips this yet; + * it exists so a feature can register a command once and mark it unavailable + * while its preconditions are unmet, rather than register/unregister it. + */ + get enabled(): boolean { + return this.enabled_; + } + + set enabled(value: boolean) { + if (value === this.enabled_) return; + this.enabled_ = value; + this.changed.dispatch(); + } + + abstract invoke(context: CommandContext): void; +} + +/** + * A command backed by a DOM action: invoking it dispatches `action:` at the + * context's dispatch target, exactly as the equivalent key binding would, so + * existing `registerActionListener` handlers need no extra wiring. + */ +export class ActionCommand extends Command { + invoke({ dispatchTarget }: CommandContext) { + dispatchTarget.dispatchEvent( + new CustomEvent(`action:${this.id}`, { + bubbles: true, + cancelable: true, + detail: {}, + }), + ); + } +} + +/** + * A command that runs a callback directly, for behaviour with no corresponding + * DOM action, e.g. commands contributed by an application embedding the + * viewer. + */ +export class CallbackCommand extends Command { + constructor( + id: CommandId, + label: string, + private readonly callback: (context: CommandContext) => void, + description?: string, + ) { + super(id, label, description); + } + + invoke(context: CommandContext) { + this.callback(context); + } +} diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index 6ca0ffbc7e..7b14adbbf5 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -15,6 +15,7 @@ */ import { afterEach, describe, expect, it } from "vitest"; +import { ActionCommand } from "#src/ui/command.js"; import { collectActionBindings, CommandCatalog, @@ -126,8 +127,8 @@ describe("CommandCatalog.filter", () => { // "Edit JSON State" and "Screenshot" as its flat entries. function makeCatalog() { const registry = new CommandRegistry(); - registry.registerAction({ id: "edit-json-state", label: "Edit JSON State" }); - registry.registerAction({ id: "screenshot", label: "Screenshot" }); + registry.register(new ActionCommand("edit-json-state", "Edit JSON State")); + registry.register(new ActionCommand("screenshot", "Screenshot")); return new CommandCatalog( makeContext(makeInputEventBindings(new EventActionMap()), registry), ); diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index f760b795f5..94462c9a97 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -16,6 +16,8 @@ import type { LayerManager, SelectedLayerState } from "#src/layer/index.js"; import { UserLayer } from "#src/layer/index.js"; +import type { Command } from "#src/ui/command.js"; +import { ActionCommand } from "#src/ui/command.js"; import type { CommandRegistry } from "#src/ui/command_registry.js"; import { getMatchingTools, @@ -41,9 +43,9 @@ export interface CommandCatalogContext { selectedLayer: SelectedLayerState; inputEventBindings: InputEventBindings; /** - * Authoritative source of the flat command set. Its command-kind entries are - * enumerated directly; the input bindings are consulted only to annotate each - * command with its current shortcut, not to discover which commands exist. + * Authoritative source of the flat command set. Its commands are enumerated + * directly; the input bindings are consulted only to annotate each command + * with its current shortcut, not to discover which commands exist. */ commandRegistry: CommandRegistry; } @@ -58,23 +60,14 @@ interface CommandPaletteEntryBase { readonly shortcut: string; } -// Dispatched as an `action:` DOM event, exactly as the keyboard -// shortcut would be. -export interface ActionCommandEntry extends CommandPaletteEntryBase { - readonly kind: "action"; - readonly actionId: ActionIdentifier; -} - -// A registered command that runs a callback. Unlike `execute`, it carries the -// registry's stable `id` so consumers can correlate it back to the registry. +// A registered command. The consumer invokes it with its own context. export interface CommandEntry extends CommandPaletteEntryBase { readonly kind: "command"; - readonly id: ActionIdentifier; - readonly invoke: () => void; + readonly command: Command; } -// Runs an anonymous callback directly (no DOM action and no registry identity — -// e.g. a per-layer toggle or an unbound tool activation). +// Runs an anonymous callback directly (no command identity, e.g. a per-layer +// toggle or an unbound tool activation). export interface ExecuteCommandEntry extends CommandPaletteEntryBase { readonly kind: "execute"; readonly execute: () => void; @@ -87,7 +80,6 @@ export interface GroupCommandEntry extends CommandPaletteEntryBase { } export type CommandPaletteEntry = - | ActionCommandEntry | CommandEntry | ExecuteCommandEntry | GroupCommandEntry; @@ -345,36 +337,16 @@ export class CommandCatalog extends RefCounted { ); } - // Flat commands come from the registry. A command's shortcut is whatever - // binding is currently installed for its id (or its suggested default), - // shown for reference only. + // A command's shortcut is whatever binding is currently installed for its + // id, shown for reference only. for (const command of commandRegistry.values()) { - if (command.isAvailable !== undefined && !command.isAvailable.value) { - continue; - } - const shortcut = shortcutByAction.get(command.id) ?? ""; - const { label } = command; - switch (command.type) { - case "action": - commands.push({ - kind: "action", - label, - shortcut, - actionId: command.id, - }); - break; - case "callback": { - const invoke = command.invoke; - commands.push({ - kind: "command", - label, - shortcut, - id: command.id, - invoke: () => invoke(), - }); - break; - } - } + if (!command.enabled) continue; + commands.push({ + kind: "command", + label: command.label, + shortcut: shortcutByAction.get(command.id) ?? "", + command, + }); } const toolQueryResult = parseToolQuery("+"); @@ -416,10 +388,10 @@ export class CommandCatalog extends RefCounted { ? `${tool.description} — ${tool.context.managedLayer.name}` : tool.description; commands.push({ - kind: "action", + kind: "command", label, shortcut: shortcutByAction.get(actionId) ?? "", - actionId, + command: new ActionCommand(actionId, label), }); } else { const capturedToolJson = toolJson; diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index e84c6aa3fd..0fec7e3f20 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -280,15 +280,7 @@ export class CommandPalette extends Overlay { if (command.kind === "execute") { command.execute(); } else if (command.kind === "command") { - command.invoke(); - } else if (command.kind === "action") { - this.actionDispatchTarget.dispatchEvent( - new CustomEvent(`action:${command.actionId}`, { - bubbles: true, - cancelable: true, - detail: {}, - }), - ); + command.command.invoke({ dispatchTarget: this.actionDispatchTarget }); } } } diff --git a/src/ui/command_registry.spec.ts b/src/ui/command_registry.spec.ts index ee7c5a197b..a8fd962171 100644 --- a/src/ui/command_registry.spec.ts +++ b/src/ui/command_registry.spec.ts @@ -15,67 +15,81 @@ */ import { describe, expect, it } from "vitest"; +import type { CommandContext } from "#src/ui/command.js"; +import { ActionCommand, CallbackCommand } from "#src/ui/command.js"; import { CommandRegistry } from "#src/ui/command_registry.js"; -import { WatchableValue } from "#src/trackable_value.js"; -describe("CommandRegistry", () => { - it("registers and retrieves a command by id", () => { - const registry = new CommandRegistry(); - registry.registerAction({ - id: "screenshot", - label: "Screenshot", - description: "Capture a screenshot.", - }); - expect(registry.has("screenshot")).toBe(true); - expect(registry.get("screenshot")?.label).toBe("Screenshot"); - registry.dispose(); +function makeContext(dispatchTarget: EventTarget = new EventTarget()) { + return { dispatchTarget } satisfies CommandContext; +} + +describe("Command", () => { + it("dispatches an action event for an action command", () => { + const target = document.createElement("div"); + const seen: string[] = []; + target.addEventListener("action:screenshot", () => seen.push("screenshot")); + new ActionCommand("screenshot", "Screenshot").invoke(makeContext(target)); + expect(seen).toStrictEqual(["screenshot"]); }); - it("stamps the command type explicitly per registrar", () => { - const registry = new CommandRegistry(); - registry.registerAction({ id: "a", label: "A" }); - registry.registerCallback({ id: "c", label: "C", invoke: () => {} }); - expect(registry.get("a")?.type).toBe("action"); - expect(registry.get("c")?.type).toBe("callback"); - registry.dispose(); + it("passes the invocation context to a callback command", () => { + const target = new EventTarget(); + let seen: EventTarget | undefined; + new CallbackCommand("c", "C", (context) => { + seen = context.dispatchTarget; + }).invoke(makeContext(target)); + expect(seen).toBe(target); }); - it("invokes a callback command's callback", () => { + it("dispatches changed when enabled flips, and only then", () => { + const command = new ActionCommand("a", "A"); + let count = 0; + command.changed.add(() => ++count); + expect(command.enabled).toBe(true); + command.enabled = true; + expect(count).toBe(0); + command.enabled = false; + expect(count).toBe(1); + }); +}); + +describe("CommandRegistry", () => { + it("registers and retrieves a command by id", () => { const registry = new CommandRegistry(); - let ran = false; - registry.registerCallback({ - id: "c", - label: "C", - invoke: () => { - ran = true; - }, - }); - const command = registry.get("c"); - if (command?.type === "callback") command.invoke(); - expect(ran).toBe(true); + registry.register( + new ActionCommand("screenshot", "Screenshot", "Capture a screenshot."), + ); + expect(registry.has("screenshot")).toBe(true); + expect(registry.get("screenshot")?.label).toBe("Screenshot"); + expect(registry.get("screenshot")?.description).toBe( + "Capture a screenshot.", + ); registry.dispose(); }); - it("enumerates commands independent of any binding", () => { + it("enumerates commands in registration order, independent of any binding", () => { const registry = new CommandRegistry(); - registry.registerAction({ id: "a", label: "A" }); - registry.registerAction({ id: "b", label: "B" }); - expect([...registry.values()].map((c) => c.id)).toStrictEqual(["a", "b"]); + registry.register(new ActionCommand("a", "A")); + registry.register(new CallbackCommand("c", "C", () => {})); + expect([...registry.values()].map((command) => command.id)).toStrictEqual([ + "a", + "c", + ]); registry.dispose(); }); it("throws on duplicate id", () => { const registry = new CommandRegistry(); - registry.registerAction({ id: "dup", label: "First" }); - expect(() => - registry.registerAction({ id: "dup", label: "Second" }), - ).toThrow(/already registered/); + registry.register(new ActionCommand("dup", "First")); + expect(() => registry.register(new ActionCommand("dup", "Second"))).toThrow( + /already registered/, + ); registry.dispose(); }); it("unregisters via the returned disposer", () => { const registry = new CommandRegistry(); - const dispose = registry.registerAction({ id: "temp", label: "Temp" }); + const dispose = registry.register(new ActionCommand("temp", "Temp")); expect(registry.has("temp")).toBe(true); dispose(); expect(registry.has("temp")).toBe(false); @@ -86,36 +100,32 @@ describe("CommandRegistry", () => { const registry = new CommandRegistry(); let count = 0; registry.changed.add(() => ++count); - const dispose = registry.registerAction({ id: "x", label: "X" }); + const dispose = registry.register(new ActionCommand("x", "X")); expect(count).toBe(1); dispose(); expect(count).toBe(2); registry.dispose(); }); - it("dispatches changed when a command's availability changes", () => { + it("forwards a registered command's own changed signal", () => { const registry = new CommandRegistry(); - const isAvailable = new WatchableValue(true); - registry.registerAction({ id: "x", label: "X", isAvailable }); + const command = new ActionCommand("x", "X"); + registry.register(command); let count = 0; registry.changed.add(() => ++count); - isAvailable.value = false; + command.enabled = false; expect(count).toBe(1); registry.dispose(); }); - it("stops observing availability after unregister", () => { + it("stops forwarding a command's changed signal after unregister", () => { const registry = new CommandRegistry(); - const isAvailable = new WatchableValue(true); - const dispose = registry.registerAction({ - id: "x", - label: "X", - isAvailable, - }); + const command = new ActionCommand("x", "X"); + const dispose = registry.register(command); dispose(); let count = 0; registry.changed.add(() => ++count); - isAvailable.value = false; + command.enabled = false; expect(count).toBe(0); registry.dispose(); }); diff --git a/src/ui/command_registry.ts b/src/ui/command_registry.ts index 1b43bd260f..0fc1ee00e9 100644 --- a/src/ui/command_registry.ts +++ b/src/ui/command_registry.ts @@ -15,136 +15,79 @@ */ /** - * @file Global, binding-independent registry of viewer commands. + * @file Registry of the {@link Command}s a viewer knows about. * - * A command is declared once, with a stable id, a pretty label, and an optional - * help description. Any key binding is looked up separately and shown alongside; - * it is not the source of truth for which commands exist. + * The registry owns *what commands exist*: their id, how they present, and how + * they run. Bindings, ordering and grouping live with the consumers; a consumer + * that wants to show a shortcut looks the live binding up itself. * - * The registry is owned by the viewer so commands can be registered before — or - * without — any UI chrome. The command palette and help panel are consumers of - * the same surface; see {@link CommandCatalog}. + * The registry is owned by the viewer, so commands can be registered before any + * UI chrome exists, or without any at all. It 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 Python, can bind an action without ever + * registering a command for it. See `docs/concepts/commands.rst`. */ -import type { ActionIdentifier } from "#src/util/event_action_map.js"; +import type { Command, CommandId } from "#src/ui/command.js"; import { RefCounted } from "#src/util/disposable.js"; -import { Signal } from "#src/util/signal.js"; -import type { WatchableValueInterface } from "#src/trackable_value.js"; - -interface CommandInfoBase { - /** Stable, serialisable identifier, e.g. "toggle-scale-bar". */ - readonly id: ActionIdentifier; - /** Human-readable name shown in the palette / help panel. */ - readonly label: string; - /** - * Optional longer help text describing what the command does. Surfaced by - * hosts that can afford more than a label (help panel, tooltips). - */ - readonly description?: string; - /** - * Optional observable of whether the command is currently usable. When it - * changes the registry dispatches `changed`, so consumers can re-enumerate - * "what's usable now" without polling. - */ - readonly isAvailable?: WatchableValueInterface; -} +import { NullarySignal } from "#src/util/signal.js"; /** - * A command backed by a DOM action: invoking it dispatches `action:`, - * exactly as the equivalent keyboard shortcut would. `id` doubles as the action - * id, so existing actions need no extra wiring. - */ -export interface ActionCommandInfo extends CommandInfoBase { - readonly type: "action"; -} - -/** - * A command that runs a callback directly, for commands with no corresponding - * DOM action (e.g. host-registered commands). - */ -export interface CallbackCommandInfo extends CommandInfoBase { - readonly type: "callback"; - readonly invoke: (payload?: unknown) => unknown; -} - -/** - * A registered command. The `type` discriminant is stated explicitly by the - * registrant rather than inferred from which optional fields are present, so - * new command types can be added without changing how existing ones are read. - */ -export type CommandInfo = ActionCommandInfo | CallbackCommandInfo; - -export type CommandType = CommandInfo["type"]; - -/** - * Per-viewer registry of {@link CommandInfo}. Registration returns a disposer - * that unregisters the command, so feature code can add commands for the - * lifetime of a layer / control and clean up automatically. + * Per-viewer registry of {@link Command}s. Registration returns a disposer that + * unregisters the command, so feature code can add commands for the lifetime of + * a layer / control and clean up automatically. */ export class CommandRegistry extends RefCounted { - private readonly commands = new Map(); - private readonly availabilityDisposers = new Map< - ActionIdentifier, - () => void - >(); - - /** Dispatched when a command is added/removed, or its availability changes. */ - readonly changed = new Signal(); + private readonly commands = new Map(); + private readonly commandChangedDisposers = new Map void>(); - /** Registers an action-backed command. See {@link ActionCommandInfo}. */ - registerAction(options: Omit): () => void { - return this.register({ type: "action", ...options }); - } - - /** Registers a callback command. See {@link CallbackCommandInfo}. */ - registerCallback(options: Omit): () => void { - return this.register({ type: "callback", ...options }); - } + /** + * Dispatched when a command is registered or unregistered, or when a + * registered command reports a change of its own. + */ + readonly changed = new NullarySignal(); - /** Registers a command. Throws on duplicate `id`. Returns a disposer. */ - register(command: CommandInfo): () => void { + /** Registers `command`. Throws on duplicate id. Returns a disposer. */ + register(command: Command): () => void { const { id } = command; if (this.commands.has(id)) { throw new Error(`Command already registered: ${JSON.stringify(id)}`); } this.commands.set(id, command); - const { isAvailable } = command; - if (isAvailable !== undefined) { - this.availabilityDisposers.set( - id, - isAvailable.changed.add(() => this.changed.dispatch()), - ); - } + this.commandChangedDisposers.set( + id, + command.changed.add(() => this.changed.dispatch()), + ); this.changed.dispatch(); return () => this.unregister(id); } - unregister(id: ActionIdentifier): void { + unregister(id: CommandId): void { if (!this.commands.delete(id)) return; - const disposer = this.availabilityDisposers.get(id); + const disposer = this.commandChangedDisposers.get(id); if (disposer !== undefined) { disposer(); - this.availabilityDisposers.delete(id); + this.commandChangedDisposers.delete(id); } this.changed.dispatch(); } - get(id: ActionIdentifier): CommandInfo | undefined { + get(id: CommandId): Command | undefined { return this.commands.get(id); } - has(id: ActionIdentifier): boolean { + has(id: CommandId): boolean { return this.commands.has(id); } - /** Iterates every registered command, regardless of current availability. */ - values(): IterableIterator { + /** Iterates every registered command, enabled or not, in registration order. */ + values(): IterableIterator { return this.commands.values(); } disposed() { - for (const disposer of this.availabilityDisposers.values()) disposer(); - this.availabilityDisposers.clear(); + for (const disposer of this.commandChangedDisposers.values()) disposer(); + this.commandChangedDisposers.clear(); this.commands.clear(); super.disposed(); } diff --git a/src/ui/default_commands.ts b/src/ui/default_commands.ts index 0693bc8057..764951b85b 100644 --- a/src/ui/default_commands.ts +++ b/src/ui/default_commands.ts @@ -19,41 +19,39 @@ * * Each entry names an existing DOM action (`id` === the `action:` id dispatched * by the default input-event bindings) and gives it an explicit, human-readable - * label and help description. The registry — not the bindings — is now the - * authoritative list of commands; the shortcut shown for each command is looked - * up from whatever binding happens to be installed (see {@link CommandCatalog}). + * label and help description, so that consumers no longer have to prettify an + * action id and can show commands that happen to have no binding at all. * * Commands whose behaviour is per-entity or otherwise dynamic (layer toggles, * tool activation) are contributed by the catalog at enumeration time and are * intentionally *not* declared here. * - * This is the built-in *seed* set, not a required registry. It exists only - * because these commands correspond to DOM actions that predate the registry. - * Feature code should NOT add entries here; instead register commands - * colocated with the feature, for its own lifetime, e.g. + * This is the built-in *seed* set. It exists only because these commands + * correspond to DOM actions that predate the registry. Feature code should NOT + * add entries here; instead register commands colocated with the feature, for + * its own lifetime, e.g. * * this.registerDisposer( - * viewer.commandRegistry.registerCallback({ - * id: "clip.addPlane", - * label: "Add Clip Plane", - * invoke: () => this.addPlane(), - * }), + * viewer.commandRegistry.register( + * new CallbackCommand("add-clip-plane", "Add Clip Plane", () => + * this.addPlane(), + * ), + * ), * ); * - * `registerAction` / `registerCallback` each return a disposer, so commands may - * come and go with the feature (e.g. per-layer). `CommandRegistry` — not this - * file — is the authoritative, runtime-enumerable list. + * `register` returns a disposer, so commands may come and go with the feature + * (e.g. per-layer). */ -import type { - ActionCommandInfo, - CommandRegistry, -} from "#src/ui/command_registry.js"; +import { ActionCommand, type CommandId } from "#src/ui/command.js"; +import type { CommandRegistry } from "#src/ui/command_registry.js"; import { AXES_NAMES } from "#src/util/geom.js"; -// Every built-in command is action-backed (dispatches `action:`); the type -// is stamped by `registerAction` at registration time. -type BuiltinCommand = Omit; +interface BuiltinCommand { + readonly id: CommandId; + readonly label: string; + readonly description: string; +} // Directional position nudges and relative rotations, one pair per axis (arrow // keys / , . and r / e / shift+arrow keys in the data panels). The ids are @@ -201,8 +199,8 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ label: "Undo Annotation Step", description: "Undo the last point added to the in-progress annotation.", }, - // State — these have no default key binding; before the registry they were - // special-cased so the palette could surface them at all. + // Actions with no default key binding; before the registry the palette had to + // special-case these to surface them at all. { id: "edit-json-state", label: "Edit JSON State", @@ -213,7 +211,6 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ label: "Screenshot", description: "Capture a screenshot of the current view.", }, - // Tools. { id: "deactivate-active-tool", label: "Deactivate Active Tool", @@ -224,13 +221,13 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ /** * Registers the built-in commands into `registry`. Called once during default * viewer setup. The registry is owned (and disposed) by the viewer, so no - * disposers are returned here — the commands live for the viewer's lifetime. + * disposers are returned here, and the commands live for the viewer's lifetime. */ export function registerDefaultCommands(registry: CommandRegistry): void { - for (const command of STATIC_COMMANDS) { - registry.registerAction(command); - } - for (const command of axisCommands()) { - registry.registerAction(command); + for (const { id, label, description } of [ + ...STATIC_COMMANDS, + ...axisCommands(), + ]) { + registry.register(new ActionCommand(id, label, description)); } } diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index 062b40c424..b00de52e3a 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -17,11 +17,11 @@ import { StatusMessage } from "#src/status.js"; import { CommandCatalog } from "#src/ui/command_catalog.js"; import { bindCommandPalette } from "#src/ui/command_palette.js"; -import { registerDefaultCommands } from "#src/ui/default_commands.js"; import { bindDefaultCopyHandler, bindDefaultPasteHandler, } from "#src/ui/default_clipboard_handling.js"; +import { registerDefaultCommands } from "#src/ui/default_commands.js"; import { setDefaultInputEventBindings } from "#src/ui/default_input_event_bindings.js"; import { makeDefaultViewer } from "#src/ui/default_viewer.js"; import type { MinimalViewerOptions } from "#src/ui/minimal_viewer.js"; diff --git a/src/viewer.ts b/src/viewer.ts index 14d8c5953d..a218ade74d 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -82,6 +82,7 @@ import { observeWatchable, TrackableValue, } from "#src/trackable_value.js"; +import { CommandRegistry } from "#src/ui/command_registry.js"; import { LayerArchiveCountWidget, LayerListPanel, @@ -94,7 +95,6 @@ import { SelectionDetailsPanel } from "#src/ui/selection_details.js"; import { SidePanelManager } from "#src/ui/side_panel.js"; import { StateEditorDialog } from "#src/ui/state_editor.js"; import { StatisticsDisplayState, StatisticsPanel } from "#src/ui/statistics.js"; -import { CommandRegistry } from "#src/ui/command_registry.js"; import { GlobalToolBinder, LocalToolBinder } from "#src/ui/tool.js"; import { MultiToolPaletteDropdownButton, From 612106ae3854f984e26c13b1337b41ba51c187d5 Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 14 Aug 2026 11:13:18 +0300 Subject: [PATCH 5/7] feat: list keyboard-bound actions that have no registered 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. --- src/ui/command_catalog.spec.ts | 89 ++++++++++++++++++++++++++++++++++ src/ui/command_catalog.ts | 40 ++++++++++++--- 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index 7b14adbbf5..cb5f2b4fdd 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -160,6 +160,95 @@ describe("CommandCatalog.filter", () => { }); }); +describe("CommandCatalog command sources", () => { + function makeCatalog(map: EventActionMap, registry: CommandRegistry) { + return new CommandCatalog( + makeContext(makeInputEventBindings(map), registry), + ); + } + + function commandEntries(catalog: CommandCatalog) { + return catalog.commands.filter((entry) => entry.kind === "command"); + } + + it("annotates a registered command with its live binding", () => { + const map = new EventActionMap(); + map.set("keyb", "toggle-scale-bar"); + const registry = new CommandRegistry(); + registry.register( + new ActionCommand("toggle-scale-bar", "Toggle Scale Bar"), + ); + const catalog = makeCatalog(map, registry); + try { + const entries = commandEntries(catalog).filter( + (entry) => entry.command.id === "toggle-scale-bar", + ); + expect(entries).toHaveLength(1); + expect(entries[0].label).toBe("Toggle Scale Bar"); + expect(entries[0].shortcut).toBe("b"); + } finally { + catalog.dispose(); + } + }); + + it("still lists a bound action that nothing registered", () => { + // An embedder (or the Python integration) may bind an action without + // registering a command for it; it should not vanish from the palette. + const map = new EventActionMap(); + map.set("keyq", "embedder-action"); + const catalog = makeCatalog(map, new CommandRegistry()); + try { + const entries = commandEntries(catalog).filter( + (entry) => entry.command.id === "embedder-action", + ); + expect(entries).toHaveLength(1); + // Falls back to a label derived from the action id. + expect(entries[0].label).toBe("Embedder Action"); + expect(entries[0].shortcut).toBe("q"); + } finally { + catalog.dispose(); + } + }); + + it("does not duplicate an action that is both registered and bound", () => { + const map = new EventActionMap(); + map.set("keyb", "toggle-scale-bar"); + const registry = new CommandRegistry(); + registry.register( + new ActionCommand("toggle-scale-bar", "Toggle Scale Bar"), + ); + const catalog = makeCatalog(map, registry); + try { + expect( + commandEntries(catalog).filter( + (entry) => entry.command.id === "toggle-scale-bar", + ), + ).toHaveLength(1); + } finally { + catalog.dispose(); + } + }); + + it("omits a disabled command, binding or not", () => { + const map = new EventActionMap(); + map.set("keyb", "toggle-scale-bar"); + const registry = new CommandRegistry(); + const command = new ActionCommand("toggle-scale-bar", "Toggle Scale Bar"); + command.enabled = false; + registry.register(command); + const catalog = makeCatalog(map, registry); + try { + expect( + commandEntries(catalog).filter( + (entry) => entry.command.id === "toggle-scale-bar", + ), + ).toHaveLength(0); + } finally { + catalog.dispose(); + } + }); +}); + describe("CommandCatalog reactivity", () => { it("rebuilds (debounced) when a subscribed change signal fires", async () => { const layersChanged = new Signal(); diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 94462c9a97..72dd83fbd6 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -43,9 +43,10 @@ export interface CommandCatalogContext { selectedLayer: SelectedLayerState; inputEventBindings: InputEventBindings; /** - * Authoritative source of the flat command set. Its commands are enumerated - * directly; the input bindings are consulted only to annotate each command - * with its current shortcut, not to discover which commands exist. + * Primary source of the flat command set. Registered commands are enumerated + * directly and take precedence; any keyboard-bound action that is *not* + * registered is still listed afterwards, so actions an embedder or the Python + * integration only ever bound to a key do not disappear from the palette. */ commandRegistry: CommandRegistry; } @@ -60,7 +61,8 @@ interface CommandPaletteEntryBase { readonly shortcut: string; } -// A registered command. The consumer invokes it with its own context. +// A command, either taken from the registry or synthesised for a keyboard-bound +// action that nothing registered. The consumer invokes it with its own context. export interface CommandEntry extends CommandPaletteEntryBase { readonly kind: "command"; readonly command: Command; @@ -96,6 +98,14 @@ function formatKeyStroke(stroke: string): string { .join("+"); } +// Fallback label for a bound action that no command was registered for. +function actionIdToLabel(actionId: ActionIdentifier): string { + return actionId + .split("-") + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(" "); +} + function isKeyboardEvent(normalizedId: NormalizedEventIdentifier): boolean { return ( !normalizedId.includes("mouse") && @@ -337,8 +347,8 @@ export class CommandCatalog extends RefCounted { ); } - // A command's shortcut is whatever binding is currently installed for its - // id, shown for reference only. + // Registered commands come first. A command's shortcut is whatever binding + // is currently installed for its id, shown for reference only. for (const command of commandRegistry.values()) { if (!command.enabled) continue; commands.push({ @@ -349,6 +359,24 @@ export class CommandCatalog extends RefCounted { }); } + // The registry is not required to be exhaustive: an embedder (or the Python + // integration) may bind an action without registering a command for it. + // Those are listed too, labelled from their action id, so nothing that used + // to appear in the palette is lost. + for (const { actionId } of bindings) { + if (commandRegistry.has(actionId)) continue; + if (/^tool-[A-Z]$/.test(actionId)) continue; + // Layer-index actions are replaced by hierarchical group entries above. + if (/^(toggle|select|toggle-pick)-layer-\d+$/.test(actionId)) continue; + const label = actionIdToLabel(actionId); + commands.push({ + kind: "command", + label, + shortcut: shortcutByAction.get(actionId) ?? "", + command: new ActionCommand(actionId, label), + }); + } + const toolQueryResult = parseToolQuery("+"); if ("query" in toolQueryResult) { // Tool listers report changes to their available tool set (e.g. controls From 99ef88c3145eb616bc845ce91efae86d03db80d6 Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 14 Aug 2026 11:13:36 +0300 Subject: [PATCH 6/7] test: check the default commands against the default bindings 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. --- src/ui/default_commands.spec.ts | 119 ++++++++++++++++++++++++++++++++ src/ui/default_commands.ts | 10 +-- 2 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 src/ui/default_commands.spec.ts diff --git a/src/ui/default_commands.spec.ts b/src/ui/default_commands.spec.ts new file mode 100644 index 0000000000..f474ff422b --- /dev/null +++ b/src/ui/default_commands.spec.ts @@ -0,0 +1,119 @@ +/** + * @license + * Copyright 2026 Google Inc. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, expect, it } from "vitest"; +import { collectActionBindings } from "#src/ui/command_catalog.js"; +import { CommandRegistry } from "#src/ui/command_registry.js"; +import { + getDefaultCommands, + registerDefaultCommands, +} from "#src/ui/default_commands.js"; +import { + getDefaultGlobalBindings, + getDefaultPerspectivePanelBindings, + getDefaultSliceViewPanelBindings, +} from "#src/ui/default_input_event_bindings.js"; +import type { ActionIdentifier } from "#src/util/event_action_map.js"; +import { EventActionMap } from "#src/util/event_action_map.js"; +import type { InputEventBindings } from "#src/viewer.js"; + +// Actions the default commands deliberately do not declare: tool slots and +// per-layer-index actions, both of which the catalog contributes dynamically. +const DYNAMIC_ACTIONS = [ + /^tool-[A-Z]$/, + /^(toggle|select|toggle-pick)-layer-\d+$/, +]; + +// Commands that exist as viewer actions but have no default key binding. +const UNBOUND_BY_DEFAULT = new Set([ + "edit-json-state", + "screenshot", + "deactivate-active-tool", +]); + +function makeDefaultInputEventBindings(): InputEventBindings { + const make = (parent: EventActionMap) => { + const map = new EventActionMap(); + map.addParent(parent, Number.NEGATIVE_INFINITY); + return map; + }; + return { + global: make(getDefaultGlobalBindings()), + sliceView: make(getDefaultSliceViewPanelBindings()), + perspectiveView: make(getDefaultPerspectivePanelBindings()), + } as unknown as InputEventBindings; +} + +// Every keyboard-bound action reachable from the default global, slice-view and +// perspective-view bindings (the latter two pull in the shared rendered data +// panel bindings as a parent). +function defaultBoundActions(): ActionIdentifier[] { + return collectActionBindings(makeDefaultInputEventBindings()) + .map(({ actionId }) => actionId) + .filter((actionId) => !DYNAMIC_ACTIONS.some((re) => re.test(actionId))); +} + +function makeDefaultRegistry() { + const registry = new CommandRegistry(); + registerDefaultCommands(registry); + return registry; +} + +describe("registerDefaultCommands", () => { + it("declares a command for every default keyboard binding", () => { + const registry = makeDefaultRegistry(); + const missing = defaultBoundActions().filter( + (actionId) => !registry.has(actionId), + ); + expect(missing).toStrictEqual([]); + registry.dispose(); + }); + + it("declares no command id that is not a real action", () => { + // The reverse direction: a typo in a command id would otherwise register a + // command that dispatches an `action:` event nothing listens for. Every + // declared command must either be bound by default or be listed as + // knowingly unbound. + const bound = new Set(defaultBoundActions()); + const unaccounted = getDefaultCommands() + .map(({ id }) => id) + .filter((id) => !bound.has(id) && !UNBOUND_BY_DEFAULT.has(id)); + expect(unaccounted).toStrictEqual([]); + }); + + it("gives every command a label and a description", () => { + const registry = makeDefaultRegistry(); + for (const command of registry.values()) { + expect(command.label, command.id).not.toBe(""); + expect(command.description, command.id).toBeTruthy(); + } + registry.dispose(); + }); + + it("registers the axis commands for each of x, y and z", () => { + const registry = makeDefaultRegistry(); + for (const axis of ["x", "y", "z"]) { + for (const sign of ["-", "+"]) { + expect(registry.has(`${axis}${sign}`), `${axis}${sign}`).toBe(true); + expect( + registry.has(`rotate-relative-${axis}${sign}`), + `rotate-relative-${axis}${sign}`, + ).toBe(true); + } + } + registry.dispose(); + }); +}); diff --git a/src/ui/default_commands.ts b/src/ui/default_commands.ts index 764951b85b..4ab38c2ecc 100644 --- a/src/ui/default_commands.ts +++ b/src/ui/default_commands.ts @@ -218,16 +218,18 @@ const STATIC_COMMANDS: readonly BuiltinCommand[] = [ }, ]; +/** The built-in commands, in the order they are registered. */ +export function getDefaultCommands(): readonly BuiltinCommand[] { + return [...STATIC_COMMANDS, ...axisCommands()]; +} + /** * Registers the built-in commands into `registry`. Called once during default * viewer setup. The registry is owned (and disposed) by the viewer, so no * disposers are returned here, and the commands live for the viewer's lifetime. */ export function registerDefaultCommands(registry: CommandRegistry): void { - for (const { id, label, description } of [ - ...STATIC_COMMANDS, - ...axisCommands(), - ]) { + for (const { id, label, description } of getDefaultCommands()) { registry.register(new ActionCommand(id, label, description)); } } From 9497ae9c6c90f9cc6887100392cb7a4b8e173fff Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 14 Aug 2026 11:13:47 +0300 Subject: [PATCH 7/7] docs: add a commands concept page 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. --- docs/concepts/commands.rst | 121 +++++++++++++++++++++++++++++++++++++ docs/index.rst | 1 + src/viewer.ts | 8 ++- 3 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 docs/concepts/commands.rst diff --git a/docs/concepts/commands.rst b/docs/concepts/commands.rst new file mode 100644 index 0000000000..631f8eb6f9 --- /dev/null +++ b/docs/concepts/commands.rst @@ -0,0 +1,121 @@ +.. _command: + +Command +======= + +A command is something a user can invoke by name: "Toggle Scale Bar", +"Screenshot", "Add Layer". Commands are named separately from the keys they are +bound to, so a command with no keyboard shortcut still shows up wherever +commands are listed. + +.. _command-object: + +Command +------- + +Defined in ``src/ui/command.ts``. + +A ``Command`` holds a stable ``id``, the ``label`` and optional ``description`` +shown in the UI, and an ``invoke`` method that runs it. Bindings, ordering and +grouping live elsewhere. + +Two implementations ship with the viewer: + +- ``ActionCommand`` dispatches ``action:`` at the invocation context's + dispatch target, the same event the matching key binding sends, so existing + ``registerActionListener`` handlers keep working. +- ``CallbackCommand`` runs a callback, for behaviour with no DOM action behind + it. This is the usual choice for an application embedding the viewer. + +``invoke`` takes a ``CommandContext`` rather than a bare target, which leaves +room to pass more context later (mouse position, originating layer) without +changing every implementation. + +Each command has a ``changed`` signal covering everything a consumer might +redraw for, so a consumer subscribes once per command rather than once per +property. ``enabled`` is currently the only property that changes. + +.. _command-registry: + +Command registry +---------------- + +Defined in ``src/ui/command_registry.ts``. + +The registry holds the commands a viewer knows about, and each viewer owns one. +Default viewer setup seeds it with the built-in set from +``src/ui/default_commands.ts``, and feature code registers its own commands +next to the feature they belong to: + +.. code-block:: typescript + + this.registerDisposer( + viewer.commandRegistry.register( + new CallbackCommand("add-clip-plane", "Add Clip Plane", () => + this.addPlane(), + ), + ), + ); + +``register`` returns a disposer, so commands can come and go with the feature +that owns them, a per-layer control for example. The registry re-dispatches its +``changed`` signal when a command is registered, unregistered, or reports a +change of its own. + +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. Consumers read the registry first and fall back to the bindings for the +rest. + +.. _command-catalog: + +Command catalog +--------------- + +Defined in ``src/ui/command_catalog.ts``. + +The catalog turns the registry plus the current viewer state into a flat, +ordered list of entries to present. It: + +- enumerates the registry, skipping disabled commands; +- looks up the live key binding for each command's id and attaches it as a + display-only ``shortcut``. A suggested binding stored on the command instead + would drift from whatever is installed; +- adds an entry for each keyboard-bound action that has no registered command, + labelled from its action id. This is what keeps actions contributed by an + embedder or from Python visible; +- contributes the entries that cannot be declared ahead of time: the layer + pickers (``Toggle Layer`` and friends, as sub-palette groups) and the tools + currently available; +- orders, groups and filters. + +Grouping commands into sections belongs here or further out. The palette and the +help panel would reasonably group the same commands in different ways, which is +why the registry carries no category of its own. + +The catalog rebuilds itself, debounced to an animation frame, whenever the +registry, the layers or the tool bindings change. + +.. _command-palette: + +Command palette +--------------- + +Defined in ``src/ui/command_palette.ts``. + +The palette owns the UI. It renders a catalog, lets the user search and step +into sub-palettes, and invokes the selected entry with a ``CommandContext`` +whose dispatch target is whichever element had focus when the palette opened. + +Update flow +----------- + +.. code-block:: text + + register / unregister, command.changed + │ + ▼ + CommandRegistry.changed ──▶ CommandCatalog rebuild ──▶ CommandCatalog.changed ──▶ CommandPalette re-render + ▲ + layer / tool binding changes┘ diff --git a/docs/index.rst b/docs/index.rst index ed12341c45..5bfacdbe25 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -12,6 +12,7 @@ Neuroglancer :hidden: :caption: Concepts + concepts/commands concepts/coordinate_spaces concepts/data_views concepts/layers diff --git a/src/viewer.ts b/src/viewer.ts index a218ade74d..2f75fa1e2a 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -1176,9 +1176,11 @@ export class Viewer extends RefCounted implements ViewerState { new GlobalToolBinder(this.toolInputEventMapBinder, this.toolPalettes), ); - // Global, binding-independent registry of viewer commands. Populated with the - // built-in commands during default viewer setup; feature code and hosts may - // register additional commands against it. + // Binding-independent registry of the viewer's commands. Populated with the + // built-in commands during default viewer setup; feature code and embedding + // applications may register additional commands against it. It lists the + // commands it was told about, not every action that exists - see + // docs/concepts/commands.rst. public commandRegistry = this.registerDisposer(new CommandRegistry()); public toolBinder = this.registerDisposer(