From 758970aa38e4c6708618b58e10efd6bdae1862de Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Sat, 6 Jun 2026 23:55:16 +0200 Subject: [PATCH 01/30] feat: add command palette --- src/ui/command_palette.css | 68 +++++ src/ui/command_palette.spec.ts | 102 +++++++ src/ui/command_palette.ts | 355 +++++++++++++++++++++++++ src/ui/default_input_event_bindings.ts | 1 + src/util/event_action_map.ts | 2 +- src/viewer.ts | 12 + 6 files changed, 539 insertions(+), 1 deletion(-) create mode 100644 src/ui/command_palette.css create mode 100644 src/ui/command_palette.spec.ts create mode 100644 src/ui/command_palette.ts diff --git a/src/ui/command_palette.css b/src/ui/command_palette.css new file mode 100644 index 0000000000..57e2edaba0 --- /dev/null +++ b/src/ui/command_palette.css @@ -0,0 +1,68 @@ +/** + * @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. + */ + +.neuroglancer-command-palette.overlay-content { + background: #222; + color: #fff; + padding: 0; + width: 500px; + max-width: 90vw; + display: flex; + flex-direction: column; + max-height: 60vh; +} + +.neuroglancer-command-palette-input-row { + border-bottom: 1px solid #444; +} + +.neuroglancer-command-palette-input { + width: 100%; + background: transparent; + border: none; + outline: none; + color: inherit; + padding: 8px; + box-sizing: border-box; +} + +.neuroglancer-command-palette-results { + overflow-y: auto; +} + +.neuroglancer-command-palette-row { + display: flex; + justify-content: space-between; + align-items: center; + padding: 4px 8px; + cursor: pointer; +} + +.neuroglancer-command-palette-row:hover, +.neuroglancer-command-palette-row[data-active] { + background: #444; +} + +.neuroglancer-command-palette-shortcut { + color: #aaa; + margin-left: 12px; + flex-shrink: 0; +} + +.neuroglancer-command-palette-empty { + padding: 8px; + color: #888; +} diff --git a/src/ui/command_palette.spec.ts b/src/ui/command_palette.spec.ts new file mode 100644 index 0000000000..a988097162 --- /dev/null +++ b/src/ui/command_palette.spec.ts @@ -0,0 +1,102 @@ +/** + * @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, + CommandCatalog, +} from "#src/ui/command_palette.js"; +import { EventActionMap } from "#src/util/event_action_map.js"; +import type { Viewer } from "#src/viewer.js"; + +function makeViewer( + global: EventActionMap, + sliceView = new EventActionMap(), + perspectiveView = new EventActionMap(), +): Viewer { + return { + inputEventBindings: { global, sliceView, perspectiveView }, + } as unknown as Viewer; +} + +describe("collectActionBindings", () => { + it("collects keyboard bindings", () => { + const map = new EventActionMap(); + map.set("keya", "some-action"); + const bindings = collectActionBindings(makeViewer(map)); + expect(bindings.map((binding) => binding.actionId)).toContain("some-action"); + }); + + it("excludes mouse and wheel events", () => { + const map = new EventActionMap(); + map.set("at:mousedown0", "mouse-action"); + map.set("at:wheel", "wheel-action"); + map.set("keya", "keyboard-action"); + const ids = collectActionBindings(makeViewer(map)).map((b) => b.actionId); + expect(ids).toContain("keyboard-action"); + expect(ids).not.toContain("mouse-action"); + expect(ids).not.toContain("wheel-action"); + }); + + it("keeps only the first binding when an action appears in multiple maps", () => { + const globalMap = new EventActionMap(); + globalMap.set("keya", "shared-action"); + const sliceMap = new EventActionMap(); + sliceMap.set("keyb", "shared-action"); + const bindings = collectActionBindings(makeViewer(globalMap, sliceMap)); + const forAction = bindings.filter((b) => b.actionId === "shared-action"); + expect(forAction).toHaveLength(1); + expect(forAction[0].eventAction.originalEventIdentifier).toBe("keya"); + }); + + it("excludes open-command-palette", () => { + const map = new EventActionMap(); + map.set("f1", "open-command-palette"); + map.set("keya", "some-action"); + const ids = collectActionBindings(makeViewer(map)).map((b) => b.actionId); + expect(ids).not.toContain("open-command-palette"); + expect(ids).toContain("some-action"); + }); +}); + +describe("CommandCatalog.filter", () => { + // With empty bindings the catalog contains only the two supplemental commands: + // "Edit JSON State" and "Screenshot". + function makeCatalog() { + return new CommandCatalog(null as unknown as Viewer, []); + } + + it("returns all commands for an empty query", () => { + const catalog = makeCatalog(); + expect(catalog.filter("")).toStrictEqual(catalog.commands); + }); + + it("is case-insensitive", () => { + expect(makeCatalog().filter("EDIT")).toHaveLength(1); + expect(makeCatalog().filter("edit")).toHaveLength(1); + }); + + it("ranks prefix matches before substring matches", () => { + // "s": "Screenshot" is a prefix match; "Edit JSON State" contains 's' as a substring + const results = makeCatalog().filter("s"); + expect(results[0].label).toBe("Screenshot"); + expect(results[1].label).toBe("Edit JSON State"); + }); + + it("returns empty for a non-matching query", () => { + expect(makeCatalog().filter("xyz")).toHaveLength(0); + }); +}); diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts new file mode 100644 index 0000000000..0a5d0ced30 --- /dev/null +++ b/src/ui/command_palette.ts @@ -0,0 +1,355 @@ +/** + * @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 "#src/ui/command_palette.css"; +import { UserLayer } from "#src/layer/index.js"; +import { Overlay } from "#src/overlay.js"; +import type { + ActionIdentifier, + EventAction, + NormalizedEventIdentifier, +} from "#src/util/event_action_map.js"; +import { friendlyEventIdentifier } from "#src/util/event_action_map.js"; +import type { Viewer } from "#src/viewer.js"; + +/** + * We automatically collect actions from keyboard bindings on the + * viewer. However, there may be some actions desired that have no keybinds. + * Those can be listed here to be included in the command palette. + */ +const SUPPLEMENTAL_COMMANDS: readonly { + actionId: ActionIdentifier; + label: string; +}[] = [ + { actionId: "edit-json-state", label: "Edit JSON State" }, + { actionId: "screenshot", label: "Screenshot" }, +]; + +// Numeric enum so `typeof result === "number"` +// reliably distinguishes a skip result from a resolved label string. +enum ActionSkipReason { + UnoccupiedTool = 0, // tool-X is bound to a key but no tool is assigned to that slot + MissingLayer = 1, // layer index is out of range for this binding +} + +// string - resolved label; include this entry +// ActionSkipReason - matched this action type but no resource exists; exclude the entry +// undefined - this resolver does not handle this action type; try the next +type ResolvedAction = string | ActionSkipReason | undefined; + +function shouldSkip(result: ResolvedAction): result is ActionSkipReason { + return typeof result === "number"; +} + +export interface ActionBinding { + readonly actionId: ActionIdentifier; + readonly eventAction: EventAction; +} + +export interface CommandPaletteEntry { + readonly label: string; + readonly shortcut: string; + readonly actionId: ActionIdentifier; +} + +function formatKeyStroke(stroke: string): string { + return stroke + .split("+") + .map((part) => { + if (part.startsWith("key")) return part.substring(3); + if (part.startsWith("digit")) return part.substring(5); + if (part.startsWith("arrow")) return part.substring(5); + return part; + }) + .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") && + !normalizedId.includes("wheel") && + !normalizedId.includes("touch") && + !normalizedId.includes("click") + ); +} + +/** + * Walk the event action maps available on the viewer and produce a list of + * every action with any keyboard binding. The first binding found for each + * action is kept; subsequent bindings for the same action are ignored. + */ +export function collectActionBindings( + viewer: Viewer, +): readonly ActionBinding[] { + const seenBindings = new Map(); + + const collect = ( + bindings: Iterable<[NormalizedEventIdentifier, EventAction]>, + ) => { + for (const [normalizedId, eventAction] of bindings) { + if (!isKeyboardEvent(normalizedId)) continue; + if (eventAction.action === "open-command-palette") continue; + if (!seenBindings.has(eventAction.action)) { + seenBindings.set(eventAction.action, eventAction); + } + } + }; + + collect(viewer.inputEventBindings.global.entries()); + collect(viewer.inputEventBindings.sliceView.entries()); + collect(viewer.inputEventBindings.perspectiveView.entries()); + + return Array.from(seenBindings.entries(), ([actionId, eventAction]) => ({ + actionId, + eventAction, + })); +} + +/** + * Take raw ActionBindings and map them to user-facing CommandPaletteEntries + * with formatted labels and shortcuts. Actions with no meaningful label + * (unoccupied tool slots, out-of-range layer indices) are dropped. + */ +export class CommandCatalog { + readonly commands: CommandPaletteEntry[] = []; + + constructor(viewer: Viewer, bindings: readonly ActionBinding[]) { + for (const { actionId, eventAction } of bindings) { + const toolLabel = this.resolveToolLabel(viewer, actionId); + if (shouldSkip(toolLabel)) continue; + + const layerLabel = + toolLabel === undefined + ? this.resolveLayerLabel(viewer, actionId) + : undefined; + if (shouldSkip(layerLabel)) continue; + + const label = toolLabel ?? layerLabel ?? actionIdToLabel(actionId); + const shortcut = formatKeyStroke( + friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), + ); + this.commands.push({ label, shortcut, actionId }); + } + + for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { + this.commands.push({ label, shortcut: "", actionId }); + } + } + + /** + * Name-based filtering. Prefix matches are ranked before substring matches. + */ + filter(searchString: string): readonly CommandPaletteEntry[] { + if (searchString === "") return this.commands; + + const query = searchString.toLowerCase(); + const prefixMatches: CommandPaletteEntry[] = []; + const substringMatches: CommandPaletteEntry[] = []; + + for (const command of this.commands) { + const label = command.label.toLowerCase(); + if (label.startsWith(query)) prefixMatches.push(command); + else if (label.includes(query)) substringMatches.push(command); + } + + return [...prefixMatches, ...substringMatches]; + } + + private resolveToolLabel( + viewer: Viewer, + actionId: ActionIdentifier, + ): ResolvedAction { + const toolMatch = actionId.match(/^tool-([A-Z])$/); + if (toolMatch === null) return undefined; + const tool = viewer.globalToolBinder.bindings.get(toolMatch[1]); + if (tool === undefined) return ActionSkipReason.UnoccupiedTool; + return tool.context instanceof UserLayer + ? `${tool.description} — ${tool.context.managedLayer.name}` + : tool.description; + } + + private resolveLayerLabel( + viewer: Viewer, + actionId: ActionIdentifier, + ): ResolvedAction { + const toggleMatch = actionId.match(/^toggle-layer-(\d+)$/); + const selectMatch = actionId.match(/^select-layer-(\d+)$/); + const pickMatch = actionId.match(/^toggle-pick-layer-(\d+)$/); + const match = toggleMatch ?? selectMatch ?? pickMatch; + if (match === null) return undefined; + + const layerIndex = parseInt(match[1], 10); + const layer = viewer.layerManager.getLayerByNonArchivedIndex( + layerIndex - 1, + ); + if (layer === undefined) return ActionSkipReason.MissingLayer; + + const prefix = toggleMatch + ? "Toggle Layer" + : selectMatch + ? "Select Layer" + : "Toggle Pick Layer"; + return `${prefix} ${layerIndex}: ${layer.name}`; + } +} + +export class CommandPalette extends Overlay { + private readonly searchInput: HTMLInputElement; + private readonly resultsList: HTMLElement; + private readonly catalog: CommandCatalog; + private readonly rowByCommand = new Map(); + private readonly emptyElement: HTMLElement; + private filteredCommands: readonly CommandPaletteEntry[] = []; + private filteredRows: HTMLElement[] = []; + private activeIndex = 0; + + private readonly keyHandlers: Partial< + Record void> + > = { + ArrowDown: (event) => { + event.preventDefault(); + this.setActive(this.activeIndex + 1); + }, + ArrowUp: (event) => { + event.preventDefault(); + this.setActive(this.activeIndex - 1); + }, + Enter: (event) => { + event.preventDefault(); + event.stopPropagation(); + if (this.filteredCommands.length > 0) + this.run(this.filteredCommands[this.activeIndex]); + }, + Escape: () => this.close(), + }; + + constructor( + viewer: Viewer, + private readonly actionDispatchTarget: HTMLElement, + ) { + super(); + this.content.classList.add("neuroglancer-command-palette"); + + const bindings = collectActionBindings(viewer); + this.catalog = new CommandCatalog(viewer, bindings); + + for (const command of this.catalog.commands) { + const commandRow = document.createElement("div"); + commandRow.className = "neuroglancer-command-palette-row"; + commandRow.addEventListener("click", () => this.run(command)); + + const labelElement = document.createElement("span"); + labelElement.textContent = command.label; + commandRow.appendChild(labelElement); + + if (command.shortcut) { + const shortcutElement = document.createElement("span"); + shortcutElement.className = "neuroglancer-command-palette-shortcut"; + shortcutElement.textContent = command.shortcut; + commandRow.appendChild(shortcutElement); + } + + this.rowByCommand.set(command, commandRow); + } + + const emptyElement = (this.emptyElement = document.createElement("div")); + emptyElement.className = "neuroglancer-command-palette-empty"; + emptyElement.textContent = "No commands found."; + + const inputContainer = document.createElement("div"); + inputContainer.className = "neuroglancer-command-palette-input-row"; + const searchInput = (this.searchInput = document.createElement("input")); + searchInput.type = "text"; + searchInput.className = "neuroglancer-command-palette-input"; + searchInput.placeholder = "Type a command..."; + searchInput.autocomplete = "off"; + searchInput.spellcheck = false; + inputContainer.appendChild(searchInput); + this.content.appendChild(inputContainer); + + const resultsList = (this.resultsList = document.createElement("div")); + resultsList.className = "neuroglancer-command-palette-results"; + this.content.appendChild(resultsList); + + searchInput.addEventListener("input", () => { + this.activeIndex = 0; + this.render(); + }); + + resultsList.addEventListener("mousedown", (event) => + event.preventDefault(), + ); + + this.content.addEventListener( + "keydown", + (event: KeyboardEvent) => this.keyHandlers[event.key]?.(event), + { capture: true }, + ); + + this.render(); + searchInput.focus(); + } + + private render() { + this.filteredCommands = this.catalog.filter(this.searchInput.value); + if (this.activeIndex >= this.filteredCommands.length) { + this.activeIndex = Math.max(0, this.filteredCommands.length - 1); + } + + if (this.filteredCommands.length === 0) { + this.resultsList.replaceChildren(this.emptyElement); + return; + } + + this.filteredRows = this.filteredCommands.map( + (command) => this.rowByCommand.get(command)!, + ); + this.filteredRows.forEach((commandRow, rowIndex) => { + commandRow.toggleAttribute("data-active", rowIndex === this.activeIndex); + }); + this.resultsList.replaceChildren(...this.filteredRows); + } + + private setActive(targetIndex: number) { + if (this.filteredRows.length === 0) return; + this.activeIndex = + ((targetIndex % this.filteredRows.length) + this.filteredRows.length) % + this.filteredRows.length; + this.filteredRows.forEach((commandRow, rowIndex) => { + commandRow.toggleAttribute("data-active", rowIndex === this.activeIndex); + if (rowIndex === this.activeIndex) + commandRow.scrollIntoView({ block: "nearest" }); + }); + } + + private run(command: CommandPaletteEntry) { + this.close(); + this.actionDispatchTarget.dispatchEvent( + new CustomEvent(`action:${command.actionId}`, { + bubbles: true, + cancelable: true, + detail: {}, + }), + ); + } +} diff --git a/src/ui/default_input_event_bindings.ts b/src/ui/default_input_event_bindings.ts index c041bc0335..3a1fc8429f 100644 --- a/src/ui/default_input_event_bindings.ts +++ b/src/ui/default_input_event_bindings.ts @@ -48,6 +48,7 @@ export function getDefaultGlobalBindings() { map.set("space", "toggle-layout"); map.set("shift+space", "toggle-layout-alternative"); map.set("backslash", "toggle-show-statistics"); + map.set("control+keyp", "open-command-palette"); defaultGlobalBindings = map; } return defaultGlobalBindings; diff --git a/src/util/event_action_map.ts b/src/util/event_action_map.ts index bddf12a6f0..912c7074d9 100644 --- a/src/util/event_action_map.ts +++ b/src/util/event_action_map.ts @@ -475,7 +475,7 @@ export function dispatchEventWithModifiers( /** * DOM Event type used for dispatching actions. * - * Additional information relevant to the acction is specified as the `detail` property. + * Additional information relevant to the action is specified as the `detail` property. */ export interface ActionEvent extends CustomEvent { detail: Info; diff --git a/src/viewer.ts b/src/viewer.ts index 4b171fdf06..d3df44744b 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -82,6 +82,7 @@ import { observeWatchable, TrackableValue, } from "#src/trackable_value.js"; +import { CommandPalette } from "#src/ui/command_palette.js"; import { LayerArchiveCountWidget, LayerListPanel, @@ -1138,6 +1139,17 @@ export class Viewer extends RefCounted implements ViewerState { this.showPerspectiveSliceViews.toggle(), ); this.bindAction("toggle-show-statistics", () => this.showStatistics()); + + this.bindAction("open-command-palette", () => { + const prevFocused = document.activeElement; + const dispatchTarget = + prevFocused instanceof HTMLElement && this.element.contains(prevFocused) + ? prevFocused + : this.element; + new CommandPalette(this, dispatchTarget); + }); + this.bindAction("edit-json-state", () => this.editJsonState()); + this.bindAction("screenshot", () => this.showScreenshotDialog()); } toggleHelpPanel() { From f18b14710bfaa0d4d4f86e89d5f615358d653cb7 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 8 Jun 2026 10:02:19 +0200 Subject: [PATCH 02/30] fix: stop tools eating command palette inputs --- src/ui/command_palette.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 0a5d0ced30..a926587b34 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -306,6 +306,11 @@ export class CommandPalette extends Overlay { { capture: true }, ); + // Tools register keydown on window (bubble); stop propagation here after searchInput receives the event. + this.content.addEventListener("keydown", (event) => { + event.stopPropagation(); + }); + this.render(); searchInput.focus(); } From 0722c90e654085223629de1792ac96bf06ace8ec Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 8 Jun 2026 12:25:20 +0200 Subject: [PATCH 03/30] feat: allow unbound tools to activate and doc level command --- src/ui/command_palette.ts | 283 ++++++++++++++++++++++++++++++++------ src/viewer.ts | 31 ++++- 2 files changed, 266 insertions(+), 48 deletions(-) diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index a926587b34..cff87a734b 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -17,6 +17,8 @@ import "#src/ui/command_palette.css"; import { UserLayer } from "#src/layer/index.js"; import { Overlay } from "#src/overlay.js"; +import { getMatchingTools, restoreTool } from "#src/ui/tool.js"; +import { parseToolQuery } from "#src/ui/tool_query.js"; import type { ActionIdentifier, EventAction, @@ -25,11 +27,6 @@ import type { import { friendlyEventIdentifier } from "#src/util/event_action_map.js"; import type { Viewer } from "#src/viewer.js"; -/** - * We automatically collect actions from keyboard bindings on the - * viewer. However, there may be some actions desired that have no keybinds. - * Those can be listed here to be included in the command palette. - */ const SUPPLEMENTAL_COMMANDS: readonly { actionId: ActionIdentifier; label: string; @@ -38,11 +35,10 @@ const SUPPLEMENTAL_COMMANDS: readonly { { actionId: "screenshot", label: "Screenshot" }, ]; -// Numeric enum so `typeof result === "number"` -// reliably distinguishes a skip result from a resolved label string. +// Numeric enum so `typeof result === "number"` reliably distinguishes a skip +// result from a resolved label string. enum ActionSkipReason { - UnoccupiedTool = 0, // tool-X is bound to a key but no tool is assigned to that slot - MissingLayer = 1, // layer index is out of range for this binding + MissingLayer = 0, } // string - resolved label; include this entry @@ -63,6 +59,7 @@ export interface CommandPaletteEntry { readonly label: string; readonly shortcut: string; readonly actionId: ActionIdentifier; + readonly execute?: () => void; } function formatKeyStroke(stroke: string): string { @@ -93,6 +90,144 @@ function isKeyboardEvent(normalizedId: NormalizedEventIdentifier): boolean { ); } +// Creates a Tool instance from a palette-form JSON object (with optional "layer" field). +// Caller is responsible for disposing the returned tool. +function createToolFromJson(viewer: Viewer, toolJson: unknown) { + try { + const json = + typeof toolJson === "object" && toolJson !== null + ? (toolJson as Record) + : undefined; + const layerName = typeof json?.layer === "string" ? json.layer : undefined; + if (layerName !== undefined) { + const { layer: _ignored, ...rest } = json!; + const managedLayer = viewer.layerManager.getLayerByName(layerName); + const userLayer = managedLayer?.layer ?? null; + if (userLayer === null) return undefined; + return restoreTool(userLayer, rest); + } + return restoreTool(viewer, toolJson); + } catch { + return undefined; + } +} + +function getToolDescription(viewer: Viewer, toolJson: unknown): string { + const tool = createToolFromJson(viewer, toolJson); + if (tool === undefined) return toolJsonToLabel(toolJson); + const label = + tool.context instanceof UserLayer + ? `${tool.description} — ${tool.context.managedLayer.name}` + : tool.description; + tool.dispose(); + return label; +} + +// Fallback label derived purely from the JSON structure (no instantiation). +function toolJsonToLabel(toolJson: unknown): string { + const json = + typeof toolJson === "object" && toolJson !== null + ? (toolJson as Record) + : undefined; + const typeName = + typeof toolJson === "string" + ? toolJson + : typeof json?.type === "string" + ? json.type + : undefined; + const layerName = typeof json?.layer === "string" ? json.layer : undefined; + const base = + typeName !== undefined + ? typeName + .replace(/([A-Z])/g, " $1") + .replace(/-./g, (s) => " " + s[1].toUpperCase()) + .replace(/^./, (s) => s.toUpperCase()) + .trim() + : "Unknown Tool"; + return layerName !== undefined ? `${base} — ${layerName}` : base; +} + +// Tracks letter keys that were temporarily bound by the palette (viewer → key → tool). +// WeakMap allows GC if the viewer is destroyed. +const paletteActivatedKeys = new WeakMap>(); + +// Removes any palette-activated temp bindings whose tool is no longer the active tool. +// Called each time the palette opens or before activating a new unbound tool. +function sweepPaletteActivatedKeys(viewer: Viewer): void { + const tracked = paletteActivatedKeys.get(viewer as object); + if (tracked === undefined) return; + const activeTool = viewer.globalToolBinder.activeTool_?.tool; + for (const [key, trackedTool] of tracked) { + const currentTool = viewer.globalToolBinder.bindings.get(key); + if (currentTool !== trackedTool) { + // Our tool was replaced or removed at this key by something else — stop tracking. + tracked.delete(key); + } else if (currentTool !== activeTool) { + // Our tool is still bound here but no longer active — clean up the temp binding. + viewer.globalToolBinder.set(key, undefined); + tracked.delete(key); + } + // currentTool === trackedTool === activeTool: still active, keep tracking. + } +} + +function activateUnboundTool(viewer: Viewer, toolJson: unknown): void { + const tool = createToolFromJson(viewer, toolJson); + if (tool === undefined) return; + + // GlobalToolBinder.set deduplicates by JSON string within a localBinder: + // it removes any existing binding with the same serialized tool JSON before + // adding the new one. If the same tool type is already bound to a key + // (e.g. the tool appeared as "unbound" in the palette due to a JSON mismatch), + // activate that existing key rather than calling set and clobbering the user's binding. + const localBinder = tool.localBinder; + const existingKey = localBinder.jsonToKey.get(JSON.stringify(tool.toJSON())); + if (existingKey !== undefined) { + tool.dispose(); + viewer.globalToolBinder.activate(existingKey); + return; + } + + // Prefer reusing the active palette-tool's key slot: it will be deactivated + // when the new tool activates anyway, so taking a new letter would just cause + // the slots to bounce (A → B → A → B …) as each old binding lingers until + // the next sweep. + const tracked = paletteActivatedKeys.get(viewer as object); + const activeTool = viewer.globalToolBinder.activeTool_?.tool; + let targetKey: string | undefined; + if (tracked !== undefined && activeTool !== undefined) { + for (const [key, trackedTool] of tracked) { + if (trackedTool === activeTool) { + targetKey = key; + break; + } + } + } + + if (targetKey === undefined) { + // No active palette slot to reuse; sweep stale entries then find a free key. + sweepPaletteActivatedKeys(viewer); + targetKey = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + .split("") + .find((key) => !viewer.globalToolBinder.bindings.has(key)); + if (targetKey === undefined) { + tool.dispose(); + return; + } + } + + let newTracked = paletteActivatedKeys.get(viewer as object); + if (newTracked === undefined) { + newTracked = new Map(); + paletteActivatedKeys.set(viewer as object, newTracked); + } + newTracked.delete(targetKey); + newTracked.set(targetKey, tool as object); + + viewer.globalToolBinder.set(targetKey, tool); + viewer.globalToolBinder.activate(targetKey); +} + /** * Walk the event action maps available on the viewer and produce a list of * every action with any keyboard binding. The first binding found for each @@ -126,39 +261,97 @@ export function collectActionBindings( } /** - * Take raw ActionBindings and map them to user-facing CommandPaletteEntries - * with formatted labels and shortcuts. Actions with no meaningful label - * (unoccupied tool slots, out-of-range layer indices) are dropped. + * Take raw ActionBindings and map them to user-facing CommandPaletteEntries. + * All available tools are discovered via getMatchingTools. Unbound tools + * are included with an execute callback that temporarily binds them to the + * first available letter slot and activates them. */ export class CommandCatalog { readonly commands: CommandPaletteEntry[] = []; constructor(viewer: Viewer, bindings: readonly ActionBinding[]) { + sweepPaletteActivatedKeys(viewer); + + // "Deactivate Active Tool" goes first so it's always one keystroke away + // when a tool is running. + if (viewer.globalToolBinder.activeTool_ !== undefined) { + this.commands.push({ + label: "Deactivate Active Tool", + shortcut: "", + actionId: "deactivate-active-tool", + }); + } + + const shortcutByAction = new Map(); + for (const { actionId, eventAction } of bindings) { + shortcutByAction.set( + actionId, + formatKeyStroke( + friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), + ), + ); + } + for (const { actionId, eventAction } of bindings) { - const toolLabel = this.resolveToolLabel(viewer, actionId); - if (shouldSkip(toolLabel)) continue; + if (/^tool-[A-Z]$/.test(actionId)) continue; - const layerLabel = - toolLabel === undefined - ? this.resolveLayerLabel(viewer, actionId) - : undefined; + const layerLabel = this.resolveLayerLabel(viewer, actionId); if (shouldSkip(layerLabel)) continue; - const label = toolLabel ?? layerLabel ?? actionIdToLabel(actionId); + const label = layerLabel ?? actionIdToLabel(actionId); const shortcut = formatKeyStroke( friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), ); this.commands.push({ label, shortcut, actionId }); } + const toolQueryResult = parseToolQuery("+"); + if ("query" in toolQueryResult) { + const toolMatches = getMatchingTools( + viewer.globalToolBinder, + toolQueryResult.query, + ); + + // Build a reverse lookup from palette-JSON key to letter for currently-bound tools. + const boundByJsonKey = new Map(); + for (const [letter, tool] of viewer.globalToolBinder.bindings) { + const paletteJson = tool.localBinder.convertLocalJSONToPaletteJSON( + tool.toJSON(), + ); + boundByJsonKey.set(JSON.stringify(paletteJson), letter); + } + + for (const [jsonKey, toolJson] of toolMatches) { + const boundLetter = boundByJsonKey.get(jsonKey); + if (boundLetter !== undefined) { + const actionId: ActionIdentifier = `tool-${boundLetter}`; + const tool = viewer.globalToolBinder.bindings.get(boundLetter)!; + const label = + tool.context instanceof UserLayer + ? `${tool.description} — ${tool.context.managedLayer.name}` + : tool.description; + this.commands.push({ + label, + shortcut: shortcutByAction.get(actionId) ?? "", + actionId, + }); + } else { + const capturedToolJson = toolJson; + this.commands.push({ + label: getToolDescription(viewer, toolJson), + shortcut: "", + actionId: `tool-json:${jsonKey}` as ActionIdentifier, + execute: () => activateUnboundTool(viewer, capturedToolJson), + }); + } + } + } + for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { this.commands.push({ label, shortcut: "", actionId }); } } - /** - * Name-based filtering. Prefix matches are ranked before substring matches. - */ filter(searchString: string): readonly CommandPaletteEntry[] { if (searchString === "") return this.commands; @@ -175,19 +368,6 @@ export class CommandCatalog { return [...prefixMatches, ...substringMatches]; } - private resolveToolLabel( - viewer: Viewer, - actionId: ActionIdentifier, - ): ResolvedAction { - const toolMatch = actionId.match(/^tool-([A-Z])$/); - if (toolMatch === null) return undefined; - const tool = viewer.globalToolBinder.bindings.get(toolMatch[1]); - if (tool === undefined) return ActionSkipReason.UnoccupiedTool; - return tool.context instanceof UserLayer - ? `${tool.description} — ${tool.context.managedLayer.name}` - : tool.description; - } - private resolveLayerLabel( viewer: Viewer, actionId: ActionIdentifier, @@ -240,7 +420,7 @@ export class CommandPalette extends Overlay { if (this.filteredCommands.length > 0) this.run(this.filteredCommands[this.activeIndex]); }, - Escape: () => this.close(), + Escape: () => this.closeAndRestoreFocus(), }; constructor( @@ -347,14 +527,29 @@ export class CommandPalette extends Overlay { }); } - private run(command: CommandPaletteEntry) { + // Non-toggle tools register a window bubble-phase keydown handler that + // calls preventDefault() on all keys. Restoring focus to the viewer element + // before the next keydown ensures F1 bubbles through the viewer's + // KeyboardEventBinder and can reopen the palette. + private closeAndRestoreFocus() { + const target = this.actionDispatchTarget; this.close(); - this.actionDispatchTarget.dispatchEvent( - new CustomEvent(`action:${command.actionId}`, { - bubbles: true, - cancelable: true, - detail: {}, - }), - ); + target.focus({ preventScroll: true }); + } + + private run(command: CommandPaletteEntry) { + this.closeAndRestoreFocus(); + + if (command.execute !== undefined) { + command.execute(); + } else { + this.actionDispatchTarget.dispatchEvent( + new CustomEvent(`action:${command.actionId}`, { + bubbles: true, + cancelable: true, + detail: {}, + }), + ); + } } } diff --git a/src/viewer.ts b/src/viewer.ts index d3df44744b..868d700704 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -108,7 +108,7 @@ import { import { AutomaticallyFocusedElement } from "#src/util/automatic_focus.js"; import { TrackableRGB } from "#src/util/color.js"; import type { Borrowed, Owned } from "#src/util/disposable.js"; -import { RefCounted } from "#src/util/disposable.js"; +import { RefCounted, registerEventListener } from "#src/util/disposable.js"; import { removeFromParent } from "#src/util/dom.js"; import type { ActionEvent } from "#src/util/event_action_map.js"; import { registerActionListener } from "#src/util/event_action_map.js"; @@ -1140,14 +1140,37 @@ export class Viewer extends RefCounted implements ViewerState { ); this.bindAction("toggle-show-statistics", () => this.showStatistics()); - this.bindAction("open-command-palette", () => { + // Guard prevents double-open when both the element-level action listener and + // the document capture listener fire for the same keypress. + let openPalette: CommandPalette | undefined; + const openCommandPalette = () => { + if (openPalette !== undefined && !openPalette.wasDisposed) return; const prevFocused = document.activeElement; const dispatchTarget = prevFocused instanceof HTMLElement && this.element.contains(prevFocused) ? prevFocused : this.element; - new CommandPalette(this, dispatchTarget); - }); + openPalette = new CommandPalette(this, dispatchTarget); + }; + this.bindAction("open-command-palette", openCommandPalette); + // Document-level capture fires before bubble handlers, ensuring F1 works + // even when focus is inside a tool's input element outside viewer.element. + this.registerDisposer( + registerEventListener( + document, + "keydown", + (event: KeyboardEvent) => { + if (event.code === "F1") { + event.preventDefault(); + openCommandPalette(); + } + }, + { capture: true }, + ), + ); + this.bindAction("deactivate-active-tool", () => + this.globalToolBinder.deactivate(), + ); this.bindAction("edit-json-state", () => this.editJsonState()); this.bindAction("screenshot", () => this.showScreenshotDialog()); } From ecea5e3cded3de992c3f5f4ecea5bc982c8fa2aa Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 8 Jun 2026 12:25:29 +0200 Subject: [PATCH 04/30] test: update tests --- src/ui/command_palette.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ui/command_palette.spec.ts b/src/ui/command_palette.spec.ts index a988097162..ee4555a727 100644 --- a/src/ui/command_palette.spec.ts +++ b/src/ui/command_palette.spec.ts @@ -29,6 +29,7 @@ function makeViewer( ): Viewer { return { inputEventBindings: { global, sliceView, perspectiveView }, + globalToolBinder: { bindings: new Map(), localBinders: new Set() }, } as unknown as Viewer; } @@ -76,7 +77,12 @@ describe("CommandCatalog.filter", () => { // With empty bindings the catalog contains only the two supplemental commands: // "Edit JSON State" and "Screenshot". function makeCatalog() { - return new CommandCatalog(null as unknown as Viewer, []); + return new CommandCatalog( + { + globalToolBinder: { bindings: new Map(), localBinders: new Set() }, + } as unknown as Viewer, + [], + ); } it("returns all commands for an empty query", () => { From 91ad7752007eae9bb251107fefbbd37acfe1ce41 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 8 Jun 2026 12:30:34 +0200 Subject: [PATCH 05/30] feat: don't show tools for hidden layers --- src/ui/command_palette.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index cff87a734b..dfef0f6093 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -147,6 +147,17 @@ function toolJsonToLabel(toolJson: unknown): string { return layerName !== undefined ? `${base} — ${layerName}` : base; } +function isToolLayerVisible(viewer: Viewer, toolJson: unknown): boolean { + const json = + typeof toolJson === "object" && toolJson !== null + ? (toolJson as Record) + : undefined; + const layerName = typeof json?.layer === "string" ? json.layer : undefined; + if (layerName === undefined) return true; + const managedLayer = viewer.layerManager.getLayerByName(layerName); + return managedLayer !== undefined && managedLayer.visible; +} + // Tracks letter keys that were temporarily bound by the palette (viewer → key → tool). // WeakMap allows GC if the viewer is destroyed. const paletteActivatedKeys = new WeakMap>(); @@ -322,6 +333,7 @@ export class CommandCatalog { } for (const [jsonKey, toolJson] of toolMatches) { + if (!isToolLayerVisible(viewer, toolJson)) continue; const boundLetter = boundByJsonKey.get(jsonKey); if (boundLetter !== undefined) { const actionId: ActionIdentifier = `tool-${boundLetter}`; From 6950aa002a9939f6619f9cb5af48a5082ff8f0ce Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 8 Jun 2026 13:13:59 +0200 Subject: [PATCH 06/30] feat: add heirarchy to command palette --- src/ui/command_palette.css | 12 ++ src/ui/command_palette.spec.ts | 10 +- src/ui/command_palette.ts | 245 ++++++++++++++++++++++++--------- 3 files changed, 197 insertions(+), 70 deletions(-) diff --git a/src/ui/command_palette.css b/src/ui/command_palette.css index 57e2edaba0..8ca31464b0 100644 --- a/src/ui/command_palette.css +++ b/src/ui/command_palette.css @@ -66,3 +66,15 @@ padding: 8px; color: #888; } + +.neuroglancer-command-palette-picker-header { + padding: 4px 8px; + color: #aaa; + font-size: 12px; + cursor: pointer; + border-bottom: 1px solid #333; +} + +.neuroglancer-command-palette-picker-header:hover { + color: #fff; +} diff --git a/src/ui/command_palette.spec.ts b/src/ui/command_palette.spec.ts index ee4555a727..af532113fe 100644 --- a/src/ui/command_palette.spec.ts +++ b/src/ui/command_palette.spec.ts @@ -96,10 +96,14 @@ describe("CommandCatalog.filter", () => { }); it("ranks prefix matches before substring matches", () => { - // "s": "Screenshot" is a prefix match; "Edit JSON State" contains 's' as a substring + // "Screenshot" is a prefix match; "Edit JSON State" is a substring match. + // Verify all prefix matches appear before all substring matches. const results = makeCatalog().filter("s"); - expect(results[0].label).toBe("Screenshot"); - expect(results[1].label).toBe("Edit JSON State"); + const screenshotIndex = results.findIndex((r) => r.label === "Screenshot"); + const stateIndex = results.findIndex((r) => r.label === "Edit JSON State"); + expect(screenshotIndex).toBeGreaterThanOrEqual(0); + expect(stateIndex).toBeGreaterThanOrEqual(0); + expect(screenshotIndex).toBeLessThan(stateIndex); }); it("returns empty for a non-matching query", () => { diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index dfef0f6093..f928cc45ae 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -35,21 +35,6 @@ const SUPPLEMENTAL_COMMANDS: readonly { { actionId: "screenshot", label: "Screenshot" }, ]; -// Numeric enum so `typeof result === "number"` reliably distinguishes a skip -// result from a resolved label string. -enum ActionSkipReason { - MissingLayer = 0, -} - -// string - resolved label; include this entry -// ActionSkipReason - matched this action type but no resource exists; exclude the entry -// undefined - this resolver does not handle this action type; try the next -type ResolvedAction = string | ActionSkipReason | undefined; - -function shouldSkip(result: ResolvedAction): result is ActionSkipReason { - return typeof result === "number"; -} - export interface ActionBinding { readonly actionId: ActionIdentifier; readonly eventAction: EventAction; @@ -60,6 +45,7 @@ export interface CommandPaletteEntry { readonly shortcut: string; readonly actionId: ActionIdentifier; readonly execute?: () => void; + readonly children?: readonly CommandPaletteEntry[]; } function formatKeyStroke(stroke: string): string { @@ -276,6 +262,12 @@ export function collectActionBindings( * All available tools are discovered via getMatchingTools. Unbound tools * are included with an execute callback that temporarily binds them to the * first available letter slot and activates them. + * + * Actions can be represented hierarchically, with parent entries that + * expand to show child entries when activated. For example, + * layer actions (toggle-layer-N, select-layer-N, toggle-pick-layer-N) are + * replaced by three hierarchical entries whose children are the individual + * layer rows, enabling a two-step layer picker instead of a flat list. */ export class CommandCatalog { readonly commands: CommandPaletteEntry[] = []; @@ -293,6 +285,52 @@ export class CommandCatalog { }); } + // 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. + const layers = viewer.layerManager?.managedLayers ?? []; + + this.commands.push({ + label: "Toggle Layer", + shortcut: "1–9", + actionId: "toggle-layer-group" as ActionIdentifier, + children: layers.map((layer, index) => ({ + label: layer.name, + shortcut: index < 9 ? String(index + 1) : "", + actionId: `toggle-layer-name:${layer.name}` as ActionIdentifier, + execute: () => layer.setVisible(!layer.visible), + })), + }); + + this.commands.push({ + label: "Select Layer", + shortcut: "Ctrl+1–9", + actionId: "select-layer-group" as ActionIdentifier, + children: layers.map((layer, index) => ({ + label: layer.name, + shortcut: index < 9 ? `Ctrl+${index + 1}` : "", + actionId: `select-layer-name:${layer.name}` as ActionIdentifier, + execute: () => { + viewer.selectedLayer.layer = layer; + viewer.selectedLayer.visible = true; + }, + })), + }); + + this.commands.push({ + label: "Toggle Pick Layer", + shortcut: "Alt+1–9", + actionId: "toggle-pick-layer-group" as ActionIdentifier, + children: layers.map((layer, index) => ({ + label: layer.name, + shortcut: index < 9 ? `Alt+${index + 1}` : "", + actionId: `toggle-pick-layer-name:${layer.name}` as ActionIdentifier, + execute: () => { + layer.pickEnabled = !layer.pickEnabled; + }, + })), + }); + const shortcutByAction = new Map(); for (const { actionId, eventAction } of bindings) { shortcutByAction.set( @@ -305,17 +343,20 @@ export class CommandCatalog { for (const { actionId, eventAction } of bindings) { if (/^tool-[A-Z]$/.test(actionId)) continue; + // Layer-index actions are replaced by hierarchical group entries below. + if (/^(toggle|select|toggle-pick)-layer-\d+$/.test(actionId)) continue; - const layerLabel = this.resolveLayerLabel(viewer, actionId); - if (shouldSkip(layerLabel)) continue; - - const label = layerLabel ?? actionIdToLabel(actionId); + const label = actionIdToLabel(actionId); const shortcut = formatKeyStroke( friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), ); this.commands.push({ label, shortcut, actionId }); } + for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { + this.commands.push({ label, shortcut: "", actionId }); + } + const toolQueryResult = parseToolQuery("+"); if ("query" in toolQueryResult) { const toolMatches = getMatchingTools( @@ -358,10 +399,6 @@ export class CommandCatalog { } } } - - for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { - this.commands.push({ label, shortcut: "", actionId }); - } } filter(searchString: string): readonly CommandPaletteEntry[] { @@ -379,30 +416,6 @@ export class CommandCatalog { return [...prefixMatches, ...substringMatches]; } - - private resolveLayerLabel( - viewer: Viewer, - actionId: ActionIdentifier, - ): ResolvedAction { - const toggleMatch = actionId.match(/^toggle-layer-(\d+)$/); - const selectMatch = actionId.match(/^select-layer-(\d+)$/); - const pickMatch = actionId.match(/^toggle-pick-layer-(\d+)$/); - const match = toggleMatch ?? selectMatch ?? pickMatch; - if (match === null) return undefined; - - const layerIndex = parseInt(match[1], 10); - const layer = viewer.layerManager.getLayerByNonArchivedIndex( - layerIndex - 1, - ); - if (layer === undefined) return ActionSkipReason.MissingLayer; - - const prefix = toggleMatch - ? "Toggle Layer" - : selectMatch - ? "Select Layer" - : "Toggle Pick Layer"; - return `${prefix} ${layerIndex}: ${layer.name}`; - } } export class CommandPalette extends Overlay { @@ -411,9 +424,15 @@ export class CommandPalette extends Overlay { private readonly catalog: CommandCatalog; private readonly rowByCommand = new Map(); private readonly emptyElement: HTMLElement; + private readonly pickerHeaderElement: HTMLElement; private filteredCommands: readonly CommandPaletteEntry[] = []; private filteredRows: HTMLElement[] = []; private activeIndex = 0; + private currentCommands: readonly CommandPaletteEntry[]; + private readonly levelStack: { + commands: readonly CommandPaletteEntry[]; + label: string; + }[] = []; private readonly keyHandlers: Partial< Record void> @@ -432,7 +451,28 @@ export class CommandPalette extends Overlay { if (this.filteredCommands.length > 0) this.run(this.filteredCommands[this.activeIndex]); }, - Escape: () => this.closeAndRestoreFocus(), + Backspace: () => { + if (this.levelStack.length > 0 && this.searchInput.value === "") { + this.goBack(); + } + }, + ArrowLeft: (event) => { + if ( + this.levelStack.length > 0 && + this.searchInput.selectionStart === 0 && + this.searchInput.selectionEnd === 0 + ) { + event.preventDefault(); + this.goBack(); + } + }, + Escape: () => { + if (this.levelStack.length > 0) { + this.goBack(); + } else { + this.closeAndRestoreFocus(); + } + }, }; constructor( @@ -444,25 +484,13 @@ export class CommandPalette extends Overlay { const bindings = collectActionBindings(viewer); this.catalog = new CommandCatalog(viewer, bindings); + this.currentCommands = this.catalog.commands; - for (const command of this.catalog.commands) { - const commandRow = document.createElement("div"); - commandRow.className = "neuroglancer-command-palette-row"; - commandRow.addEventListener("click", () => this.run(command)); - - const labelElement = document.createElement("span"); - labelElement.textContent = command.label; - commandRow.appendChild(labelElement); - - if (command.shortcut) { - const shortcutElement = document.createElement("span"); - shortcutElement.className = "neuroglancer-command-palette-shortcut"; - shortcutElement.textContent = command.shortcut; - commandRow.appendChild(shortcutElement); - } - - this.rowByCommand.set(command, commandRow); - } + const pickerHeader = (this.pickerHeaderElement = + document.createElement("div")); + pickerHeader.className = "neuroglancer-command-palette-picker-header"; + pickerHeader.setAttribute("hidden", ""); + pickerHeader.addEventListener("click", () => this.goBack()); const emptyElement = (this.emptyElement = document.createElement("div")); emptyElement.className = "neuroglancer-command-palette-empty"; @@ -470,6 +498,7 @@ export class CommandPalette extends Overlay { const inputContainer = document.createElement("div"); inputContainer.className = "neuroglancer-command-palette-input-row"; + inputContainer.appendChild(pickerHeader); const searchInput = (this.searchInput = document.createElement("input")); searchInput.type = "text"; searchInput.className = "neuroglancer-command-palette-input"; @@ -483,6 +512,8 @@ export class CommandPalette extends Overlay { resultsList.className = "neuroglancer-command-palette-results"; this.content.appendChild(resultsList); + this.buildRows(this.catalog.commands); + searchInput.addEventListener("input", () => { this.activeIndex = 0; this.render(); @@ -507,8 +538,51 @@ export class CommandPalette extends Overlay { searchInput.focus(); } + private buildRows(commands: readonly CommandPaletteEntry[]) { + for (const command of commands) { + if (this.rowByCommand.has(command)) continue; + + const commandRow = document.createElement("div"); + commandRow.className = "neuroglancer-command-palette-row"; + commandRow.addEventListener("click", () => this.run(command)); + + const labelElement = document.createElement("span"); + labelElement.textContent = command.label; + commandRow.appendChild(labelElement); + + if (command.shortcut) { + const shortcutElement = document.createElement("span"); + shortcutElement.className = "neuroglancer-command-palette-shortcut"; + shortcutElement.textContent = command.shortcut; + commandRow.appendChild(shortcutElement); + } + + this.rowByCommand.set(command, commandRow); + + if (command.children !== undefined) { + this.buildRows(command.children); + } + } + } + + private filterCurrentLevel(): readonly CommandPaletteEntry[] { + if (this.levelStack.length === 0) { + return this.catalog.filter(this.searchInput.value); + } + const query = this.searchInput.value.toLowerCase(); + if (query === "") return this.currentCommands; + const prefixMatches: CommandPaletteEntry[] = []; + const substringMatches: CommandPaletteEntry[] = []; + for (const entry of this.currentCommands) { + const label = entry.label.toLowerCase(); + if (label.startsWith(query)) prefixMatches.push(entry); + else if (label.includes(query)) substringMatches.push(entry); + } + return [...prefixMatches, ...substringMatches]; + } + private render() { - this.filteredCommands = this.catalog.filter(this.searchInput.value); + this.filteredCommands = this.filterCurrentLevel(); if (this.activeIndex >= this.filteredCommands.length) { this.activeIndex = Math.max(0, this.filteredCommands.length - 1); } @@ -539,6 +613,29 @@ export class CommandPalette extends Overlay { }); } + private updateHeader() { + if (this.levelStack.length > 0) { + this.pickerHeaderElement.textContent = `← ${this.levelStack.at(-1)!.label}`; + this.pickerHeaderElement.removeAttribute("hidden"); + } else { + this.pickerHeaderElement.setAttribute("hidden", ""); + } + } + + private goBack() { + if (this.levelStack.length === 0) { + this.closeAndRestoreFocus(); + return; + } + const previous = this.levelStack.pop()!; + this.currentCommands = previous.commands; + this.searchInput.value = ""; + this.searchInput.placeholder = "Type a command..."; + this.updateHeader(); + this.activeIndex = 0; + this.render(); + } + // Non-toggle tools register a window bubble-phase keydown handler that // calls preventDefault() on all keys. Restoring focus to the viewer element // before the next keydown ensures F1 bubbles through the viewer's @@ -550,6 +647,20 @@ export class CommandPalette extends Overlay { } private run(command: CommandPaletteEntry) { + if (command.children !== undefined && command.children.length > 0) { + this.levelStack.push({ + commands: this.currentCommands, + label: command.label, + }); + this.currentCommands = command.children; + this.searchInput.value = ""; + this.searchInput.placeholder = `Filter ${command.label}…`; + this.updateHeader(); + this.activeIndex = 0; + this.render(); + return; + } + this.closeAndRestoreFocus(); if (command.execute !== undefined) { From 7d68aa490fb9b8ed42da5505e564cebf32d20bc9 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 8 Jun 2026 13:39:13 +0200 Subject: [PATCH 07/30] fix: correct global bind key --- src/viewer.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/viewer.ts b/src/viewer.ts index 868d700704..91d14ade76 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -1153,14 +1153,13 @@ export class Viewer extends RefCounted implements ViewerState { openPalette = new CommandPalette(this, dispatchTarget); }; this.bindAction("open-command-palette", openCommandPalette); - // Document-level capture fires before bubble handlers, ensuring F1 works - // even when focus is inside a tool's input element outside viewer.element. + // Document-level capture to ensure that the command palette opens even when focus is inside a tool's input element outside viewer.element. this.registerDisposer( registerEventListener( document, "keydown", (event: KeyboardEvent) => { - if (event.code === "F1") { + if (event.code === "KeyP" && event.ctrlKey) { event.preventDefault(); openCommandPalette(); } From 9fa5c4a3ef528637e172e7b6684b812ed6d00b6a Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 22 Jun 2026 14:56:59 +0200 Subject: [PATCH 08/30] fix: correct tool binding and correct pattern pattern now follows default viewer_setup binding --- src/ui/command_palette.ts | 50 ++++++++++++++++++++++++++++++++++ src/ui/default_viewer_setup.ts | 2 ++ src/viewer.ts | 30 +------------------- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index f928cc45ae..4de058dc39 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -223,6 +223,21 @@ function activateUnboundTool(viewer: Viewer, toolJson: unknown): void { viewer.globalToolBinder.set(targetKey, tool); viewer.globalToolBinder.activate(targetKey); + + // Eagerly clean up the temp binding when the tool deactivates, rather than + // waiting for the next palette open. + const capturedKey = targetKey; + const capturedTool = tool as object; + const removeListener = viewer.globalToolBinder.changed.add(() => { + const active = viewer.globalToolBinder.activeTool_?.tool; + if ( + viewer.globalToolBinder.bindings.get(capturedKey) !== capturedTool || + active !== capturedTool + ) { + sweepPaletteActivatedKeys(viewer); + removeListener(); + } + }); } /** @@ -676,3 +691,38 @@ export class CommandPalette extends Overlay { } } } + +/** + * Binds the command palette to a viewer: registers the "open-command-palette" + * action and a document-level Ctrl+P capture listener so the palette opens + * regardless of where focus currently sits. + * + * Call from the standalone setup (e.g. setupDefaultViewer). Embedders who do + * not want the document-level key capture simply omit this call. + */ +export function bindCommandPalette(viewer: Viewer): void { + // Guard prevents double-open when both the element-level action listener and + // the document capture listener fire for the same keypress. + let openPalette: CommandPalette | undefined; + const openCommandPalette = () => { + if (openPalette !== undefined && !openPalette.wasDisposed) return; + const prevFocused = document.activeElement; + const dispatchTarget = + prevFocused instanceof HTMLElement && viewer.element.contains(prevFocused) + ? prevFocused + : viewer.element; + openPalette = new CommandPalette(viewer, dispatchTarget); + }; + viewer.bindAction("open-command-palette", openCommandPalette); + viewer.registerEventListener( + document, + "keydown", + (event: KeyboardEvent) => { + if (event.code === "KeyP" && event.ctrlKey) { + event.preventDefault(); + openCommandPalette(); + } + }, + { capture: true }, + ); +} diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index fc6bb1a406..ecc1f929df 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -15,6 +15,7 @@ */ import { StatusMessage } from "#src/status.js"; +import { bindCommandPalette } from "#src/ui/command_palette.js"; import { bindDefaultCopyHandler, bindDefaultPasteHandler, @@ -62,6 +63,7 @@ export function setupDefaultViewer(options?: Partial) { bindDefaultCopyHandler(viewer); bindDefaultPasteHandler(viewer); + bindCommandPalette(viewer); return viewer; } diff --git a/src/viewer.ts b/src/viewer.ts index 91d14ade76..ce9b115946 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -82,7 +82,6 @@ import { observeWatchable, TrackableValue, } from "#src/trackable_value.js"; -import { CommandPalette } from "#src/ui/command_palette.js"; import { LayerArchiveCountWidget, LayerListPanel, @@ -108,7 +107,7 @@ import { import { AutomaticallyFocusedElement } from "#src/util/automatic_focus.js"; import { TrackableRGB } from "#src/util/color.js"; import type { Borrowed, Owned } from "#src/util/disposable.js"; -import { RefCounted, registerEventListener } from "#src/util/disposable.js"; +import { RefCounted } from "#src/util/disposable.js"; import { removeFromParent } from "#src/util/dom.js"; import type { ActionEvent } from "#src/util/event_action_map.js"; import { registerActionListener } from "#src/util/event_action_map.js"; @@ -1140,33 +1139,6 @@ export class Viewer extends RefCounted implements ViewerState { ); this.bindAction("toggle-show-statistics", () => this.showStatistics()); - // Guard prevents double-open when both the element-level action listener and - // the document capture listener fire for the same keypress. - let openPalette: CommandPalette | undefined; - const openCommandPalette = () => { - if (openPalette !== undefined && !openPalette.wasDisposed) return; - const prevFocused = document.activeElement; - const dispatchTarget = - prevFocused instanceof HTMLElement && this.element.contains(prevFocused) - ? prevFocused - : this.element; - openPalette = new CommandPalette(this, dispatchTarget); - }; - this.bindAction("open-command-palette", openCommandPalette); - // Document-level capture to ensure that the command palette opens even when focus is inside a tool's input element outside viewer.element. - this.registerDisposer( - registerEventListener( - document, - "keydown", - (event: KeyboardEvent) => { - if (event.code === "KeyP" && event.ctrlKey) { - event.preventDefault(); - openCommandPalette(); - } - }, - { capture: true }, - ), - ); this.bindAction("deactivate-active-tool", () => this.globalToolBinder.deactivate(), ); From aae0de64b4fd4a17d572b5e0ee937a8822573ab3 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 22 Jun 2026 17:49:57 +0200 Subject: [PATCH 09/30] feat: bind to non-key tool also fixes the lifetime and binding locations to be more consistent with the default viewer setup and the input event bindings to help panel --- src/ui/command_palette.spec.ts | 46 ++++--- src/ui/command_palette.ts | 236 ++++++++++++--------------------- src/ui/default_viewer_setup.ts | 5 +- src/ui/tool.ts | 14 ++ 4 files changed, 135 insertions(+), 166 deletions(-) diff --git a/src/ui/command_palette.spec.ts b/src/ui/command_palette.spec.ts index af532113fe..f1e3464b54 100644 --- a/src/ui/command_palette.spec.ts +++ b/src/ui/command_palette.spec.ts @@ -18,26 +18,45 @@ import { describe, expect, it } from "vitest"; import { collectActionBindings, CommandCatalog, + type CommandCatalogContext, } from "#src/ui/command_palette.js"; import { EventActionMap } from "#src/util/event_action_map.js"; -import type { Viewer } from "#src/viewer.js"; +import type { InputEventBindings } from "#src/viewer.js"; -function makeViewer( +function makeInputEventBindings( global: EventActionMap, sliceView = new EventActionMap(), perspectiveView = new EventActionMap(), -): Viewer { +): InputEventBindings { + return { global, sliceView, perspectiveView } as unknown as InputEventBindings; +} + +const noopSignal = { add: () => () => {} }; + +function makeContext( + inputEventBindings = makeInputEventBindings(new EventActionMap()), +): CommandCatalogContext { return { - inputEventBindings: { global, sliceView, perspectiveView }, - globalToolBinder: { bindings: new Map(), localBinders: new Set() }, - } as unknown as Viewer; + globalToolBinder: { + changed: noopSignal, + bindings: new Map(), + localBinders: new Set(), + }, + layerManager: { + layersChanged: noopSignal, + managedLayers: [], + getLayerByName: () => undefined, + }, + selectedLayer: {}, + inputEventBindings, + } as unknown as CommandCatalogContext; } describe("collectActionBindings", () => { it("collects keyboard bindings", () => { const map = new EventActionMap(); map.set("keya", "some-action"); - const bindings = collectActionBindings(makeViewer(map)); + const bindings = collectActionBindings(makeInputEventBindings(map)); expect(bindings.map((binding) => binding.actionId)).toContain("some-action"); }); @@ -46,7 +65,7 @@ describe("collectActionBindings", () => { map.set("at:mousedown0", "mouse-action"); map.set("at:wheel", "wheel-action"); map.set("keya", "keyboard-action"); - const ids = collectActionBindings(makeViewer(map)).map((b) => b.actionId); + const ids = collectActionBindings(makeInputEventBindings(map)).map((b) => b.actionId); expect(ids).toContain("keyboard-action"); expect(ids).not.toContain("mouse-action"); expect(ids).not.toContain("wheel-action"); @@ -57,7 +76,7 @@ describe("collectActionBindings", () => { globalMap.set("keya", "shared-action"); const sliceMap = new EventActionMap(); sliceMap.set("keyb", "shared-action"); - const bindings = collectActionBindings(makeViewer(globalMap, sliceMap)); + const bindings = collectActionBindings(makeInputEventBindings(globalMap, sliceMap)); const forAction = bindings.filter((b) => b.actionId === "shared-action"); expect(forAction).toHaveLength(1); expect(forAction[0].eventAction.originalEventIdentifier).toBe("keya"); @@ -67,7 +86,7 @@ describe("collectActionBindings", () => { const map = new EventActionMap(); map.set("f1", "open-command-palette"); map.set("keya", "some-action"); - const ids = collectActionBindings(makeViewer(map)).map((b) => b.actionId); + const ids = collectActionBindings(makeInputEventBindings(map)).map((b) => b.actionId); expect(ids).not.toContain("open-command-palette"); expect(ids).toContain("some-action"); }); @@ -77,12 +96,7 @@ describe("CommandCatalog.filter", () => { // With empty bindings the catalog contains only the two supplemental commands: // "Edit JSON State" and "Screenshot". function makeCatalog() { - return new CommandCatalog( - { - globalToolBinder: { bindings: new Map(), localBinders: new Set() }, - } as unknown as Viewer, - [], - ); + return new CommandCatalog(makeContext()); } it("returns all commands for an empty query", () => { diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 4de058dc39..c7c5a11dfb 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -15,17 +15,27 @@ */ import "#src/ui/command_palette.css"; -import { UserLayer } from "#src/layer/index.js"; +import { LayerManager, SelectedLayerState, UserLayer } from "#src/layer/index.js"; import { Overlay } from "#src/overlay.js"; -import { getMatchingTools, restoreTool } from "#src/ui/tool.js"; +import { getMatchingTools, restoreTool, type GlobalToolBinder } from "#src/ui/tool.js"; import { parseToolQuery } from "#src/ui/tool_query.js"; +import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; +import { RefCounted } from "#src/util/disposable.js"; import type { ActionIdentifier, EventAction, NormalizedEventIdentifier, } from "#src/util/event_action_map.js"; import { friendlyEventIdentifier } from "#src/util/event_action_map.js"; -import type { Viewer } from "#src/viewer.js"; +import { Signal } from "#src/util/signal.js"; +import type { InputEventBindings, Viewer } from "#src/viewer.js"; + +export interface CommandCatalogContext { + globalToolBinder: GlobalToolBinder; + layerManager: LayerManager; + selectedLayer: SelectedLayerState; + inputEventBindings: InputEventBindings; +} const SUPPLEMENTAL_COMMANDS: readonly { actionId: ActionIdentifier; @@ -78,7 +88,7 @@ function isKeyboardEvent(normalizedId: NormalizedEventIdentifier): boolean { // Creates a Tool instance from a palette-form JSON object (with optional "layer" field). // Caller is responsible for disposing the returned tool. -function createToolFromJson(viewer: Viewer, toolJson: unknown) { +function createToolFromJson(context: CommandCatalogContext, toolJson: unknown) { try { const json = typeof toolJson === "object" && toolJson !== null @@ -87,19 +97,21 @@ function createToolFromJson(viewer: Viewer, toolJson: unknown) { const layerName = typeof json?.layer === "string" ? json.layer : undefined; if (layerName !== undefined) { const { layer: _ignored, ...rest } = json!; - const managedLayer = viewer.layerManager.getLayerByName(layerName); + const managedLayer = context.layerManager.getLayerByName(layerName); const userLayer = managedLayer?.layer ?? null; if (userLayer === null) return undefined; return restoreTool(userLayer, rest); } - return restoreTool(viewer, toolJson); + // context is the viewer instance; restoreTool walks its prototype chain + // to find the registered tool factory. + return restoreTool(context, toolJson); } catch { return undefined; } } -function getToolDescription(viewer: Viewer, toolJson: unknown): string { - const tool = createToolFromJson(viewer, toolJson); +function getToolDescription(context: CommandCatalogContext, toolJson: unknown): string { + const tool = createToolFromJson(context, toolJson); if (tool === undefined) return toolJsonToLabel(toolJson); const label = tool.context instanceof UserLayer @@ -133,111 +145,32 @@ function toolJsonToLabel(toolJson: unknown): string { return layerName !== undefined ? `${base} — ${layerName}` : base; } -function isToolLayerVisible(viewer: Viewer, toolJson: unknown): boolean { +function isToolLayerVisible(context: CommandCatalogContext, toolJson: unknown): boolean { const json = typeof toolJson === "object" && toolJson !== null ? (toolJson as Record) : undefined; const layerName = typeof json?.layer === "string" ? json.layer : undefined; if (layerName === undefined) return true; - const managedLayer = viewer.layerManager.getLayerByName(layerName); + const managedLayer = context.layerManager.getLayerByName(layerName); return managedLayer !== undefined && managedLayer.visible; } -// Tracks letter keys that were temporarily bound by the palette (viewer → key → tool). -// WeakMap allows GC if the viewer is destroyed. -const paletteActivatedKeys = new WeakMap>(); - -// Removes any palette-activated temp bindings whose tool is no longer the active tool. -// Called each time the palette opens or before activating a new unbound tool. -function sweepPaletteActivatedKeys(viewer: Viewer): void { - const tracked = paletteActivatedKeys.get(viewer as object); - if (tracked === undefined) return; - const activeTool = viewer.globalToolBinder.activeTool_?.tool; - for (const [key, trackedTool] of tracked) { - const currentTool = viewer.globalToolBinder.bindings.get(key); - if (currentTool !== trackedTool) { - // Our tool was replaced or removed at this key by something else — stop tracking. - tracked.delete(key); - } else if (currentTool !== activeTool) { - // Our tool is still bound here but no longer active — clean up the temp binding. - viewer.globalToolBinder.set(key, undefined); - tracked.delete(key); - } - // currentTool === trackedTool === activeTool: still active, keep tracking. - } -} - -function activateUnboundTool(viewer: Viewer, toolJson: unknown): void { - const tool = createToolFromJson(viewer, toolJson); +function activateUnboundTool(context: CommandCatalogContext, toolJson: unknown): void { + const tool = createToolFromJson(context, toolJson); if (tool === undefined) return; - - // GlobalToolBinder.set deduplicates by JSON string within a localBinder: - // it removes any existing binding with the same serialized tool JSON before - // adding the new one. If the same tool type is already bound to a key - // (e.g. the tool appeared as "unbound" in the palette due to a JSON mismatch), - // activate that existing key rather than calling set and clobbering the user's binding. - const localBinder = tool.localBinder; - const existingKey = localBinder.jsonToKey.get(JSON.stringify(tool.toJSON())); + // If the same tool is already bound to a key, activate that key directly + // rather than creating a duplicate. + const existingKey = tool.localBinder.jsonToKey.get( + JSON.stringify(tool.toJSON()), + ); if (existingKey !== undefined) { tool.dispose(); - viewer.globalToolBinder.activate(existingKey); + context.globalToolBinder.activate(existingKey); return; } - - // Prefer reusing the active palette-tool's key slot: it will be deactivated - // when the new tool activates anyway, so taking a new letter would just cause - // the slots to bounce (A → B → A → B …) as each old binding lingers until - // the next sweep. - const tracked = paletteActivatedKeys.get(viewer as object); - const activeTool = viewer.globalToolBinder.activeTool_?.tool; - let targetKey: string | undefined; - if (tracked !== undefined && activeTool !== undefined) { - for (const [key, trackedTool] of tracked) { - if (trackedTool === activeTool) { - targetKey = key; - break; - } - } - } - - if (targetKey === undefined) { - // No active palette slot to reuse; sweep stale entries then find a free key. - sweepPaletteActivatedKeys(viewer); - targetKey = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - .split("") - .find((key) => !viewer.globalToolBinder.bindings.has(key)); - if (targetKey === undefined) { - tool.dispose(); - return; - } - } - - let newTracked = paletteActivatedKeys.get(viewer as object); - if (newTracked === undefined) { - newTracked = new Map(); - paletteActivatedKeys.set(viewer as object, newTracked); - } - newTracked.delete(targetKey); - newTracked.set(targetKey, tool as object); - - viewer.globalToolBinder.set(targetKey, tool); - viewer.globalToolBinder.activate(targetKey); - - // Eagerly clean up the temp binding when the tool deactivates, rather than - // waiting for the next palette open. - const capturedKey = targetKey; - const capturedTool = tool as object; - const removeListener = viewer.globalToolBinder.changed.add(() => { - const active = viewer.globalToolBinder.activeTool_?.tool; - if ( - viewer.globalToolBinder.bindings.get(capturedKey) !== capturedTool || - active !== capturedTool - ) { - sweepPaletteActivatedKeys(viewer); - removeListener(); - } - }); + // No key binding — activate directly without allocating a letter slot. + context.globalToolBinder.activateDirect(tool); } /** @@ -246,7 +179,7 @@ function activateUnboundTool(viewer: Viewer, toolJson: unknown): void { * action is kept; subsequent bindings for the same action are ignored. */ export function collectActionBindings( - viewer: Viewer, + inputEventBindings: InputEventBindings, ): readonly ActionBinding[] { const seenBindings = new Map(); @@ -262,9 +195,9 @@ export function collectActionBindings( } }; - collect(viewer.inputEventBindings.global.entries()); - collect(viewer.inputEventBindings.sliceView.entries()); - collect(viewer.inputEventBindings.perspectiveView.entries()); + collect(inputEventBindings.global.entries()); + collect(inputEventBindings.sliceView.entries()); + collect(inputEventBindings.perspectiveView.entries()); return Array.from(seenBindings.entries(), ([actionId, eventAction]) => ({ actionId, @@ -273,10 +206,10 @@ export function collectActionBindings( } /** - * Take raw ActionBindings and map them to user-facing CommandPaletteEntries. - * All available tools are discovered via getMatchingTools. Unbound tools - * are included with an execute callback that temporarily binds them to the - * first available letter slot and activates them. + * 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. * * Actions can be represented hierarchically, with parent entries that * expand to show child entries when activated. For example, @@ -284,28 +217,37 @@ export function collectActionBindings( * replaced by three hierarchical entries whose children are the individual * layer rows, enabling a two-step layer picker instead of a flat list. */ -export class CommandCatalog { - readonly commands: CommandPaletteEntry[] = []; - - constructor(viewer: Viewer, bindings: readonly ActionBinding[]) { - sweepPaletteActivatedKeys(viewer); - - // "Deactivate Active Tool" goes first so it's always one keystroke away - // when a tool is running. - if (viewer.globalToolBinder.activeTool_ !== undefined) { - this.commands.push({ - label: "Deactivate Active Tool", - shortcut: "", - actionId: "deactivate-active-tool", - }); - } +export class CommandCatalog extends RefCounted { + commands: readonly CommandPaletteEntry[] = []; + readonly changed = new Signal(); + + constructor(private readonly context: CommandCatalogContext) { + super(); + const debouncedRebuild = this.registerCancellable( + animationFrameDebounce(() => this.rebuild()), + ); + this.registerDisposer(context.globalToolBinder.changed.add(debouncedRebuild)); + this.registerDisposer(context.layerManager.layersChanged.add(debouncedRebuild)); + this.rebuild(); + } + + private rebuild() { + const { globalToolBinder, layerManager, selectedLayer, inputEventBindings } = this.context; + const commands: CommandPaletteEntry[] = []; + + // "Deactivate Active Tool" is always present — harmless no-op when nothing is active. + commands.push({ + 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. - const layers = viewer.layerManager?.managedLayers ?? []; + const layers = layerManager?.managedLayers ?? []; - this.commands.push({ + commands.push({ label: "Toggle Layer", shortcut: "1–9", actionId: "toggle-layer-group" as ActionIdentifier, @@ -317,7 +259,7 @@ export class CommandCatalog { })), }); - this.commands.push({ + commands.push({ label: "Select Layer", shortcut: "Ctrl+1–9", actionId: "select-layer-group" as ActionIdentifier, @@ -326,13 +268,13 @@ export class CommandCatalog { shortcut: index < 9 ? `Ctrl+${index + 1}` : "", actionId: `select-layer-name:${layer.name}` as ActionIdentifier, execute: () => { - viewer.selectedLayer.layer = layer; - viewer.selectedLayer.visible = true; + selectedLayer.layer = layer; + selectedLayer.visible = true; }, })), }); - this.commands.push({ + commands.push({ label: "Toggle Pick Layer", shortcut: "Alt+1–9", actionId: "toggle-pick-layer-group" as ActionIdentifier, @@ -346,6 +288,7 @@ export class CommandCatalog { })), }); + const bindings = collectActionBindings(inputEventBindings); const shortcutByAction = new Map(); for (const { actionId, eventAction } of bindings) { shortcutByAction.set( @@ -358,30 +301,27 @@ export class CommandCatalog { for (const { actionId, eventAction } of bindings) { if (/^tool-[A-Z]$/.test(actionId)) continue; - // Layer-index actions are replaced by hierarchical group entries below. + // 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 ?? ""), ); - this.commands.push({ label, shortcut, actionId }); + commands.push({ label, shortcut, actionId }); } for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { - this.commands.push({ label, shortcut: "", actionId }); + commands.push({ label, shortcut: "", actionId }); } const toolQueryResult = parseToolQuery("+"); if ("query" in toolQueryResult) { - const toolMatches = getMatchingTools( - viewer.globalToolBinder, - toolQueryResult.query, - ); + const toolMatches = getMatchingTools(globalToolBinder, toolQueryResult.query); // Build a reverse lookup from palette-JSON key to letter for currently-bound tools. const boundByJsonKey = new Map(); - for (const [letter, tool] of viewer.globalToolBinder.bindings) { + for (const [letter, tool] of globalToolBinder.bindings) { const paletteJson = tool.localBinder.convertLocalJSONToPaletteJSON( tool.toJSON(), ); @@ -389,31 +329,34 @@ export class CommandCatalog { } for (const [jsonKey, toolJson] of toolMatches) { - if (!isToolLayerVisible(viewer, toolJson)) continue; + if (!isToolLayerVisible(this.context, toolJson)) continue; const boundLetter = boundByJsonKey.get(jsonKey); if (boundLetter !== undefined) { const actionId: ActionIdentifier = `tool-${boundLetter}`; - const tool = viewer.globalToolBinder.bindings.get(boundLetter)!; + const tool = globalToolBinder.bindings.get(boundLetter)!; const label = tool.context instanceof UserLayer ? `${tool.description} — ${tool.context.managedLayer.name}` : tool.description; - this.commands.push({ + commands.push({ label, shortcut: shortcutByAction.get(actionId) ?? "", actionId, }); } else { const capturedToolJson = toolJson; - this.commands.push({ - label: getToolDescription(viewer, toolJson), + commands.push({ + label: getToolDescription(this.context, toolJson), shortcut: "", actionId: `tool-json:${jsonKey}` as ActionIdentifier, - execute: () => activateUnboundTool(viewer, capturedToolJson), + execute: () => activateUnboundTool(this.context, capturedToolJson), }); } } } + + this.commands = commands; + this.changed.dispatch(); } filter(searchString: string): readonly CommandPaletteEntry[] { @@ -436,7 +379,6 @@ export class CommandCatalog { export class CommandPalette extends Overlay { private readonly searchInput: HTMLInputElement; private readonly resultsList: HTMLElement; - private readonly catalog: CommandCatalog; private readonly rowByCommand = new Map(); private readonly emptyElement: HTMLElement; private readonly pickerHeaderElement: HTMLElement; @@ -491,14 +433,12 @@ export class CommandPalette extends Overlay { }; constructor( - viewer: Viewer, + private readonly catalog: CommandCatalog, private readonly actionDispatchTarget: HTMLElement, ) { super(); this.content.classList.add("neuroglancer-command-palette"); - const bindings = collectActionBindings(viewer); - this.catalog = new CommandCatalog(viewer, bindings); this.currentCommands = this.catalog.commands; const pickerHeader = (this.pickerHeaderElement = @@ -700,7 +640,7 @@ export class CommandPalette extends Overlay { * Call from the standalone setup (e.g. setupDefaultViewer). Embedders who do * not want the document-level key capture simply omit this call. */ -export function bindCommandPalette(viewer: Viewer): void { +export function bindCommandPalette(viewer: Viewer, catalog: CommandCatalog): void { // Guard prevents double-open when both the element-level action listener and // the document capture listener fire for the same keypress. let openPalette: CommandPalette | undefined; @@ -711,7 +651,7 @@ export function bindCommandPalette(viewer: Viewer): void { prevFocused instanceof HTMLElement && viewer.element.contains(prevFocused) ? prevFocused : viewer.element; - openPalette = new CommandPalette(viewer, dispatchTarget); + openPalette = new CommandPalette(catalog, dispatchTarget); }; viewer.bindAction("open-command-palette", openCommandPalette); viewer.registerEventListener( diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index ecc1f929df..19a92ca261 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -15,7 +15,7 @@ */ import { StatusMessage } from "#src/status.js"; -import { bindCommandPalette } from "#src/ui/command_palette.js"; +import { bindCommandPalette, CommandCatalog } from "#src/ui/command_palette.js"; import { bindDefaultCopyHandler, bindDefaultPasteHandler, @@ -63,7 +63,8 @@ export function setupDefaultViewer(options?: Partial) { bindDefaultCopyHandler(viewer); bindDefaultPasteHandler(viewer); - bindCommandPalette(viewer); + const catalog = viewer.registerDisposer(new CommandCatalog(viewer)); + bindCommandPalette(viewer, catalog); return viewer; } diff --git a/src/ui/tool.ts b/src/ui/tool.ts index 847bfe877b..03f870b683 100644 --- a/src/ui/tool.ts +++ b/src/ui/tool.ts @@ -491,6 +491,20 @@ export class GlobalToolBinder extends RefCounted { public deactivate() { this.debounceDeactivate(); } + + // Activate a tool that has no letter-key binding. The tool is treated as + // toggle-mode (stays active until explicitly deactivated). The ToolActivation + // takes ownership of the tool via registerDisposer so the tool is disposed + // automatically when the activation ends. + activateDirect(tool: Owned): void { + this.queuedTool = undefined; // explicit activation clears any queued toggle tool + this.deactivate_(); // cancels debounce + disposes current activation + const activation = new ToolActivation(tool, this.inputEventMapBinder); + activation.registerDisposer(tool); + this.activeTool_ = activation; + tool.activate(activation); + this.changed.dispatch(); + } } export class LocalToolBinder< From a56524737af75c74bb22b59a133e0be5b5995d52 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 22 Jun 2026 20:31:31 +0200 Subject: [PATCH 10/30] fix: show user bind --- src/ui/command_palette.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index c7c5a11dfb..4e13d200f2 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -320,11 +320,14 @@ export class CommandCatalog extends RefCounted { const toolMatches = getMatchingTools(globalToolBinder, toolQueryResult.query); // Build a reverse lookup from palette-JSON key to letter for currently-bound tools. + // Keys must include getCommonToolProperties() to match the keys produced by + // getMatchingTools, which merges commonProperties into every yielded tool JSON. const boundByJsonKey = new Map(); for (const [letter, tool] of globalToolBinder.bindings) { - const paletteJson = tool.localBinder.convertLocalJSONToPaletteJSON( - tool.toJSON(), - ); + const paletteJson = { + ...tool.localBinder.convertLocalJSONToPaletteJSON(tool.toJSON()), + ...tool.localBinder.getCommonToolProperties(), + }; boundByJsonKey.set(JSON.stringify(paletteJson), letter); } From d5c874038a3805d4ecb962791f99cafb9efd284a Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Wed, 8 Jul 2026 17:19:50 +0200 Subject: [PATCH 11/30] feat: add more reliable rebuild and command type identification 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 --- src/ui/command_palette.spec.ts | 52 ++++++++++- src/ui/command_palette.ts | 161 +++++++++++++++++++++++---------- 2 files changed, 162 insertions(+), 51 deletions(-) diff --git a/src/ui/command_palette.spec.ts b/src/ui/command_palette.spec.ts index f1e3464b54..49c4f44f45 100644 --- a/src/ui/command_palette.spec.ts +++ b/src/ui/command_palette.spec.ts @@ -21,14 +21,23 @@ import { type CommandCatalogContext, } from "#src/ui/command_palette.js"; import { EventActionMap } from "#src/util/event_action_map.js"; +import { Signal } from "#src/util/signal.js"; import type { InputEventBindings } from "#src/viewer.js"; +function nextAnimationFrame(): Promise { + return new Promise((resolve) => requestAnimationFrame(() => resolve())); +} + function makeInputEventBindings( global: EventActionMap, sliceView = new EventActionMap(), perspectiveView = new EventActionMap(), ): InputEventBindings { - return { global, sliceView, perspectiveView } as unknown as InputEventBindings; + return { + global, + sliceView, + perspectiveView, + } as unknown as InputEventBindings; } const noopSignal = { add: () => () => {} }; @@ -39,6 +48,7 @@ function makeContext( return { globalToolBinder: { changed: noopSignal, + localBindersChanged: noopSignal, bindings: new Map(), localBinders: new Set(), }, @@ -57,7 +67,9 @@ describe("collectActionBindings", () => { const map = new EventActionMap(); map.set("keya", "some-action"); const bindings = collectActionBindings(makeInputEventBindings(map)); - expect(bindings.map((binding) => binding.actionId)).toContain("some-action"); + expect(bindings.map((binding) => binding.actionId)).toContain( + "some-action", + ); }); it("excludes mouse and wheel events", () => { @@ -65,7 +77,9 @@ describe("collectActionBindings", () => { map.set("at:mousedown0", "mouse-action"); map.set("at:wheel", "wheel-action"); map.set("keya", "keyboard-action"); - const ids = collectActionBindings(makeInputEventBindings(map)).map((b) => b.actionId); + const ids = collectActionBindings(makeInputEventBindings(map)).map( + (b) => b.actionId, + ); expect(ids).toContain("keyboard-action"); expect(ids).not.toContain("mouse-action"); expect(ids).not.toContain("wheel-action"); @@ -76,7 +90,9 @@ describe("collectActionBindings", () => { globalMap.set("keya", "shared-action"); const sliceMap = new EventActionMap(); sliceMap.set("keyb", "shared-action"); - const bindings = collectActionBindings(makeInputEventBindings(globalMap, sliceMap)); + const bindings = collectActionBindings( + makeInputEventBindings(globalMap, sliceMap), + ); const forAction = bindings.filter((b) => b.actionId === "shared-action"); expect(forAction).toHaveLength(1); expect(forAction[0].eventAction.originalEventIdentifier).toBe("keya"); @@ -86,7 +102,9 @@ describe("collectActionBindings", () => { const map = new EventActionMap(); map.set("f1", "open-command-palette"); map.set("keya", "some-action"); - const ids = collectActionBindings(makeInputEventBindings(map)).map((b) => b.actionId); + const ids = collectActionBindings(makeInputEventBindings(map)).map( + (b) => b.actionId, + ); expect(ids).not.toContain("open-command-palette"); expect(ids).toContain("some-action"); }); @@ -124,3 +142,27 @@ describe("CommandCatalog.filter", () => { expect(makeCatalog().filter("xyz")).toHaveLength(0); }); }); + +describe("CommandCatalog reactivity", () => { + it("rebuilds (debounced) when a subscribed change signal fires", async () => { + const layersChanged = new Signal(); + const context = makeContext(); + ( + context.layerManager as unknown as { layersChanged: Signal } + ).layersChanged = layersChanged; + const catalog = new CommandCatalog(context); + try { + let rebuildCount = 0; + catalog.changed.add(() => { + ++rebuildCount; + }); + layersChanged.dispatch(); + // The rebuild is debounced to an animation frame, so nothing fires yet. + expect(rebuildCount).toBe(0); + await nextAnimationFrame(); + expect(rebuildCount).toBe(1); + } finally { + catalog.dispose(); + } + }); +}); diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 4e13d200f2..165792c55d 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -15,10 +15,16 @@ */ import "#src/ui/command_palette.css"; -import { LayerManager, SelectedLayerState, UserLayer } from "#src/layer/index.js"; +import type { LayerManager, SelectedLayerState } from "#src/layer/index.js"; +import { UserLayer } from "#src/layer/index.js"; import { Overlay } from "#src/overlay.js"; -import { getMatchingTools, restoreTool, type GlobalToolBinder } from "#src/ui/tool.js"; +import { + getMatchingTools, + restoreTool, + type GlobalToolBinder, +} from "#src/ui/tool.js"; import { parseToolQuery } from "#src/ui/tool_query.js"; +import type { DebouncedFunction } from "#src/util/animation_frame_debounce.js"; import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; import { RefCounted } from "#src/util/disposable.js"; import type { @@ -50,14 +56,35 @@ export interface ActionBinding { readonly eventAction: EventAction; } -export interface CommandPaletteEntry { +interface CommandPaletteEntryBase { readonly label: string; 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; - readonly execute?: () => void; - readonly children?: readonly CommandPaletteEntry[]; } +// Runs a callback directly (no DOM action exists for it). +export interface ExecuteCommandEntry extends CommandPaletteEntryBase { + readonly kind: "execute"; + readonly execute: () => void; +} + +// Opens a sub-palette of `children` instead of activating anything itself. +export interface GroupCommandEntry extends CommandPaletteEntryBase { + readonly kind: "group"; + readonly children: readonly CommandPaletteEntry[]; +} + +export type CommandPaletteEntry = + | ActionCommandEntry + | ExecuteCommandEntry + | GroupCommandEntry; + function formatKeyStroke(stroke: string): string { return stroke .split("+") @@ -110,7 +137,10 @@ function createToolFromJson(context: CommandCatalogContext, toolJson: unknown) { } } -function getToolDescription(context: CommandCatalogContext, toolJson: unknown): string { +function getToolDescription( + context: CommandCatalogContext, + toolJson: unknown, +): string { const tool = createToolFromJson(context, toolJson); if (tool === undefined) return toolJsonToLabel(toolJson); const label = @@ -145,7 +175,10 @@ function toolJsonToLabel(toolJson: unknown): string { return layerName !== undefined ? `${base} — ${layerName}` : base; } -function isToolLayerVisible(context: CommandCatalogContext, toolJson: unknown): boolean { +function isToolLayerVisible( + context: CommandCatalogContext, + toolJson: unknown, +): boolean { const json = typeof toolJson === "object" && toolJson !== null ? (toolJson as Record) @@ -156,7 +189,10 @@ function isToolLayerVisible(context: CommandCatalogContext, toolJson: unknown): return managedLayer !== undefined && managedLayer.visible; } -function activateUnboundTool(context: CommandCatalogContext, toolJson: unknown): void { +function activateUnboundTool( + context: CommandCatalogContext, + toolJson: unknown, +): void { const tool = createToolFromJson(context, toolJson); if (tool === undefined) return; // If the same tool is already bound to a key, activate that key directly @@ -220,23 +256,37 @@ export function collectActionBindings( export class CommandCatalog extends RefCounted { commands: readonly CommandPaletteEntry[] = []; readonly changed = new Signal(); + private readonly debouncedRebuild: DebouncedFunction; constructor(private readonly context: CommandCatalogContext) { super(); - const debouncedRebuild = this.registerCancellable( + const debouncedRebuild = (this.debouncedRebuild = this.registerCancellable( animationFrameDebounce(() => this.rebuild()), + )); + this.registerDisposer( + context.globalToolBinder.changed.add(debouncedRebuild), + ); + this.registerDisposer( + context.globalToolBinder.localBindersChanged.add(debouncedRebuild), + ); + this.registerDisposer( + context.layerManager.layersChanged.add(debouncedRebuild), ); - this.registerDisposer(context.globalToolBinder.changed.add(debouncedRebuild)); - this.registerDisposer(context.layerManager.layersChanged.add(debouncedRebuild)); this.rebuild(); } private rebuild() { - const { globalToolBinder, layerManager, selectedLayer, inputEventBindings } = this.context; + const { + globalToolBinder, + layerManager, + selectedLayer, + inputEventBindings, + } = 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", @@ -248,25 +298,25 @@ export class CommandCatalog extends RefCounted { const layers = layerManager?.managedLayers ?? []; commands.push({ + kind: "group", label: "Toggle Layer", shortcut: "1–9", - actionId: "toggle-layer-group" as ActionIdentifier, children: layers.map((layer, index) => ({ + kind: "execute", label: layer.name, shortcut: index < 9 ? String(index + 1) : "", - actionId: `toggle-layer-name:${layer.name}` as ActionIdentifier, execute: () => layer.setVisible(!layer.visible), })), }); commands.push({ + kind: "group", label: "Select Layer", shortcut: "Ctrl+1–9", - actionId: "select-layer-group" as ActionIdentifier, children: layers.map((layer, index) => ({ + kind: "execute", label: layer.name, shortcut: index < 9 ? `Ctrl+${index + 1}` : "", - actionId: `select-layer-name:${layer.name}` as ActionIdentifier, execute: () => { selectedLayer.layer = layer; selectedLayer.visible = true; @@ -275,13 +325,13 @@ export class CommandCatalog extends RefCounted { }); commands.push({ + kind: "group", label: "Toggle Pick Layer", shortcut: "Alt+1–9", - actionId: "toggle-pick-layer-group" as ActionIdentifier, children: layers.map((layer, index) => ({ + kind: "execute", label: layer.name, shortcut: index < 9 ? `Alt+${index + 1}` : "", - actionId: `toggle-pick-layer-name:${layer.name}` as ActionIdentifier, execute: () => { layer.pickEnabled = !layer.pickEnabled; }, @@ -308,16 +358,28 @@ export class CommandCatalog extends RefCounted { const shortcut = formatKeyStroke( friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), ); - commands.push({ label, shortcut, actionId }); + commands.push({ kind: "action", label, shortcut, actionId }); } for (const { actionId, label } of SUPPLEMENTAL_COMMANDS) { - commands.push({ label, shortcut: "", actionId }); + commands.push({ kind: "action", label, shortcut: "", actionId }); } const toolQueryResult = parseToolQuery("+"); if ("query" in toolQueryResult) { - const toolMatches = getMatchingTools(globalToolBinder, toolQueryResult.query); + // Tool listers report changes to their available tool set (e.g. controls + // that appear once a data source resolves) via this callback + let toolSetChanged = false; + const onListableToolsChanged = () => { + if (toolSetChanged) return; + toolSetChanged = true; + this.debouncedRebuild(); + }; + const toolMatches = getMatchingTools( + globalToolBinder, + toolQueryResult.query, + onListableToolsChanged, + ); // Build a reverse lookup from palette-JSON key to letter for currently-bound tools. // Keys must include getCommonToolProperties() to match the keys produced by @@ -342,6 +404,7 @@ export class CommandCatalog extends RefCounted { ? `${tool.description} — ${tool.context.managedLayer.name}` : tool.description; commands.push({ + kind: "action", label, shortcut: shortcutByAction.get(actionId) ?? "", actionId, @@ -349,9 +412,9 @@ export class CommandCatalog extends RefCounted { } else { const capturedToolJson = toolJson; commands.push({ + kind: "execute", label: getToolDescription(this.context, toolJson), shortcut: "", - actionId: `tool-json:${jsonKey}` as ActionIdentifier, execute: () => activateUnboundTool(this.context, capturedToolJson), }); } @@ -492,6 +555,19 @@ export class CommandPalette extends Overlay { event.stopPropagation(); }); + // The catalog may rebuild while this palette is open (a layer or tool + // change, or an async lister resolving). Build rows for the new entries so + // top-level filtering can be applied to the new entries. + this.registerDisposer( + this.catalog.changed.add(() => { + this.buildRows(this.catalog.commands); + if (this.levelStack.length === 0) { + this.currentCommands = this.catalog.commands; + this.render(); + } + }), + ); + this.render(); searchInput.focus(); } @@ -517,7 +593,7 @@ export class CommandPalette extends Overlay { this.rowByCommand.set(command, commandRow); - if (command.children !== undefined) { + if (command.kind === "group") { this.buildRows(command.children); } } @@ -605,7 +681,7 @@ export class CommandPalette extends Overlay { } private run(command: CommandPaletteEntry) { - if (command.children !== undefined && command.children.length > 0) { + if (command.kind === "group" && command.children.length > 0) { this.levelStack.push({ commands: this.currentCommands, label: command.label, @@ -621,9 +697,9 @@ export class CommandPalette extends Overlay { this.closeAndRestoreFocus(); - if (command.execute !== undefined) { + if (command.kind === "execute") { command.execute(); - } else { + } else if (command.kind === "action") { this.actionDispatchTarget.dispatchEvent( new CustomEvent(`action:${command.actionId}`, { bubbles: true, @@ -636,20 +712,24 @@ export class CommandPalette extends Overlay { } /** - * Binds the command palette to a viewer: registers the "open-command-palette" - * action and a document-level Ctrl+P capture listener so the palette opens - * regardless of where focus currently sits. - * - * Call from the standalone setup (e.g. setupDefaultViewer). Embedders who do - * not want the document-level key capture simply omit this call. + * Binds the command palette to a viewer by handling the "open-command-palette" + * action at the viewer element level, the same way every other global action + * (e.g. "help") is bound. The action is dispatched by the configured + * `control+keyp` binding in the viewer's input event map. This intentionally + * does not install any document-level key listener, so the palette opens from + * the main viewer UI but does not intercept keystrokes globally. */ -export function bindCommandPalette(viewer: Viewer, catalog: CommandCatalog): void { - // Guard prevents double-open when both the element-level action listener and - // the document capture listener fire for the same keypress. +export function bindCommandPalette( + viewer: Viewer, + catalog: CommandCatalog, +): void { let openPalette: CommandPalette | undefined; const openCommandPalette = () => { if (openPalette !== undefined && !openPalette.wasDisposed) return; const prevFocused = document.activeElement; + // Tracking the dispatch target lets an activated command target the + // specific element that had focus (e.g. the "snap" action in the panel the + // user was in), falling back to the viewer element. const dispatchTarget = prevFocused instanceof HTMLElement && viewer.element.contains(prevFocused) ? prevFocused @@ -657,15 +737,4 @@ export function bindCommandPalette(viewer: Viewer, catalog: CommandCatalog): voi openPalette = new CommandPalette(catalog, dispatchTarget); }; viewer.bindAction("open-command-palette", openCommandPalette); - viewer.registerEventListener( - document, - "keydown", - (event: KeyboardEvent) => { - if (event.code === "KeyP" && event.ctrlKey) { - event.preventDefault(); - openCommandPalette(); - } - }, - { capture: true }, - ); } From 4ba9bd21072852595e0746555a9d3106c7419c96 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 13 Jul 2026 16:33:31 +0300 Subject: [PATCH 12/30] refactor: split CommandCatalog into a DOM-free command_catalog module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ...alette.spec.ts => command_catalog.spec.ts} | 2 +- src/ui/command_catalog.ts | 441 ++++++++++++++++++ src/ui/command_palette.ts | 428 +---------------- src/ui/default_viewer_setup.ts | 3 +- 4 files changed, 448 insertions(+), 426 deletions(-) rename src/ui/{command_palette.spec.ts => command_catalog.spec.ts} (99%) create mode 100644 src/ui/command_catalog.ts diff --git a/src/ui/command_palette.spec.ts b/src/ui/command_catalog.spec.ts similarity index 99% rename from src/ui/command_palette.spec.ts rename to src/ui/command_catalog.spec.ts index 49c4f44f45..19480e561a 100644 --- a/src/ui/command_palette.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -19,7 +19,7 @@ import { collectActionBindings, CommandCatalog, type CommandCatalogContext, -} from "#src/ui/command_palette.js"; +} from "#src/ui/command_catalog.js"; import { EventActionMap } from "#src/util/event_action_map.js"; import { Signal } from "#src/util/signal.js"; import type { InputEventBindings } from "#src/viewer.js"; diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts new file mode 100644 index 0000000000..aae27d53a1 --- /dev/null +++ b/src/ui/command_catalog.ts @@ -0,0 +1,441 @@ +/** + * @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 type { LayerManager, SelectedLayerState } from "#src/layer/index.js"; +import { UserLayer } from "#src/layer/index.js"; +import { + getMatchingTools, + restoreTool, + type GlobalToolBinder, +} from "#src/ui/tool.js"; +import { parseToolQuery } from "#src/ui/tool_query.js"; +import type { DebouncedFunction } from "#src/util/animation_frame_debounce.js"; +import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; +import { RefCounted } from "#src/util/disposable.js"; +import type { + ActionIdentifier, + EventAction, + NormalizedEventIdentifier, +} from "#src/util/event_action_map.js"; +import { friendlyEventIdentifier } from "#src/util/event_action_map.js"; +import { Signal } from "#src/util/signal.js"; +import type { InputEventBindings } from "#src/viewer.js"; + +export interface CommandCatalogContext { + globalToolBinder: GlobalToolBinder; + layerManager: LayerManager; + selectedLayer: SelectedLayerState; + inputEventBindings: InputEventBindings; +} + +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; +} + +interface CommandPaletteEntryBase { + readonly label: string; + 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; +} + +// Runs a callback directly (no DOM action exists for it). +export interface ExecuteCommandEntry extends CommandPaletteEntryBase { + readonly kind: "execute"; + readonly execute: () => void; +} + +// Opens a sub-palette of `children` instead of activating anything itself. +export interface GroupCommandEntry extends CommandPaletteEntryBase { + readonly kind: "group"; + readonly children: readonly CommandPaletteEntry[]; +} + +export type CommandPaletteEntry = + | ActionCommandEntry + | ExecuteCommandEntry + | GroupCommandEntry; + +function formatKeyStroke(stroke: string): string { + return stroke + .split("+") + .map((part) => { + if (part.startsWith("key")) return part.substring(3); + if (part.startsWith("digit")) return part.substring(5); + if (part.startsWith("arrow")) return part.substring(5); + return part; + }) + .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") && + !normalizedId.includes("wheel") && + !normalizedId.includes("touch") && + !normalizedId.includes("click") + ); +} + +// Creates a Tool instance from a palette-form JSON object (with optional "layer" field). +// Caller is responsible for disposing the returned tool. +function createToolFromJson(context: CommandCatalogContext, toolJson: unknown) { + try { + const json = + typeof toolJson === "object" && toolJson !== null + ? (toolJson as Record) + : undefined; + const layerName = typeof json?.layer === "string" ? json.layer : undefined; + if (layerName !== undefined) { + const { layer: _ignored, ...rest } = json!; + const managedLayer = context.layerManager.getLayerByName(layerName); + const userLayer = managedLayer?.layer ?? null; + if (userLayer === null) return undefined; + return restoreTool(userLayer, rest); + } + // context is the viewer instance; restoreTool walks its prototype chain + // to find the registered tool factory. + return restoreTool(context, toolJson); + } catch { + return undefined; + } +} + +function getToolDescription( + context: CommandCatalogContext, + toolJson: unknown, +): string { + const tool = createToolFromJson(context, toolJson); + if (tool === undefined) return toolJsonToLabel(toolJson); + const label = + tool.context instanceof UserLayer + ? `${tool.description} — ${tool.context.managedLayer.name}` + : tool.description; + tool.dispose(); + return label; +} + +// Fallback label derived purely from the JSON structure (no instantiation). +function toolJsonToLabel(toolJson: unknown): string { + const json = + typeof toolJson === "object" && toolJson !== null + ? (toolJson as Record) + : undefined; + const typeName = + typeof toolJson === "string" + ? toolJson + : typeof json?.type === "string" + ? json.type + : undefined; + const layerName = typeof json?.layer === "string" ? json.layer : undefined; + const base = + typeName !== undefined + ? typeName + .replace(/([A-Z])/g, " $1") + .replace(/-./g, (s) => " " + s[1].toUpperCase()) + .replace(/^./, (s) => s.toUpperCase()) + .trim() + : "Unknown Tool"; + return layerName !== undefined ? `${base} — ${layerName}` : base; +} + +function isToolLayerVisible( + context: CommandCatalogContext, + toolJson: unknown, +): boolean { + const json = + typeof toolJson === "object" && toolJson !== null + ? (toolJson as Record) + : undefined; + const layerName = typeof json?.layer === "string" ? json.layer : undefined; + if (layerName === undefined) return true; + const managedLayer = context.layerManager.getLayerByName(layerName); + return managedLayer !== undefined && managedLayer.visible; +} + +function activateUnboundTool( + context: CommandCatalogContext, + toolJson: unknown, +): void { + const tool = createToolFromJson(context, toolJson); + if (tool === undefined) return; + // If the same tool is already bound to a key, activate that key directly + // rather than creating a duplicate. + const existingKey = tool.localBinder.jsonToKey.get( + JSON.stringify(tool.toJSON()), + ); + if (existingKey !== undefined) { + tool.dispose(); + context.globalToolBinder.activate(existingKey); + return; + } + // No key binding — activate directly without allocating a letter slot. + context.globalToolBinder.activateDirect(tool); +} + +/** + * Walk the event action maps available on the viewer and produce a list of + * every action with any keyboard binding. The first binding found for each + * action is kept; subsequent bindings for the same action are ignored. + */ +export function collectActionBindings( + inputEventBindings: InputEventBindings, +): readonly ActionBinding[] { + const seenBindings = new Map(); + + const collect = ( + bindings: Iterable<[NormalizedEventIdentifier, EventAction]>, + ) => { + for (const [normalizedId, eventAction] of bindings) { + if (!isKeyboardEvent(normalizedId)) continue; + if (eventAction.action === "open-command-palette") continue; + if (!seenBindings.has(eventAction.action)) { + seenBindings.set(eventAction.action, eventAction); + } + } + }; + + collect(inputEventBindings.global.entries()); + collect(inputEventBindings.sliceView.entries()); + collect(inputEventBindings.perspectiveView.entries()); + + return Array.from(seenBindings.entries(), ([actionId, eventAction]) => ({ + actionId, + eventAction, + })); +} + +/** + * 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. + * + * Actions can be represented hierarchically, with parent entries that + * expand to show child entries when activated. For example, + * layer actions (toggle-layer-N, select-layer-N, toggle-pick-layer-N) are + * replaced by three hierarchical entries whose children are the individual + * layer rows, enabling a two-step layer picker instead of a flat list. + */ +export class CommandCatalog extends RefCounted { + commands: readonly CommandPaletteEntry[] = []; + readonly changed = new Signal(); + private readonly debouncedRebuild: DebouncedFunction; + + constructor(private readonly context: CommandCatalogContext) { + super(); + const debouncedRebuild = (this.debouncedRebuild = this.registerCancellable( + animationFrameDebounce(() => this.rebuild()), + )); + this.registerDisposer( + context.globalToolBinder.changed.add(debouncedRebuild), + ); + this.registerDisposer( + context.globalToolBinder.localBindersChanged.add(debouncedRebuild), + ); + this.registerDisposer( + context.layerManager.layersChanged.add(debouncedRebuild), + ); + this.rebuild(); + } + + private rebuild() { + const { + globalToolBinder, + layerManager, + selectedLayer, + inputEventBindings, + } = 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. + const layers = layerManager?.managedLayers ?? []; + + commands.push({ + kind: "group", + label: "Toggle Layer", + shortcut: "1–9", + children: layers.map((layer, index) => ({ + kind: "execute", + label: layer.name, + shortcut: index < 9 ? String(index + 1) : "", + execute: () => layer.setVisible(!layer.visible), + })), + }); + + commands.push({ + kind: "group", + label: "Select Layer", + shortcut: "Ctrl+1–9", + children: layers.map((layer, index) => ({ + kind: "execute", + label: layer.name, + shortcut: index < 9 ? `Ctrl+${index + 1}` : "", + execute: () => { + selectedLayer.layer = layer; + selectedLayer.visible = true; + }, + })), + }); + + commands.push({ + kind: "group", + label: "Toggle Pick Layer", + shortcut: "Alt+1–9", + children: layers.map((layer, index) => ({ + kind: "execute", + label: layer.name, + shortcut: index < 9 ? `Alt+${index + 1}` : "", + execute: () => { + layer.pickEnabled = !layer.pickEnabled; + }, + })), + }); + + const bindings = collectActionBindings(inputEventBindings); + const shortcutByAction = new Map(); + for (const { actionId, eventAction } of bindings) { + shortcutByAction.set( + actionId, + formatKeyStroke( + friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), + ), + ); + } + + 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 }); + } + + const toolQueryResult = parseToolQuery("+"); + if ("query" in toolQueryResult) { + // Tool listers report changes to their available tool set (e.g. controls + // that appear once a data source resolves) via this callback + let toolSetChanged = false; + const onListableToolsChanged = () => { + if (toolSetChanged) return; + toolSetChanged = true; + this.debouncedRebuild(); + }; + const toolMatches = getMatchingTools( + globalToolBinder, + toolQueryResult.query, + onListableToolsChanged, + ); + + // Build a reverse lookup from palette-JSON key to letter for currently-bound tools. + // Keys must include getCommonToolProperties() to match the keys produced by + // getMatchingTools, which merges commonProperties into every yielded tool JSON. + const boundByJsonKey = new Map(); + for (const [letter, tool] of globalToolBinder.bindings) { + const paletteJson = { + ...tool.localBinder.convertLocalJSONToPaletteJSON(tool.toJSON()), + ...tool.localBinder.getCommonToolProperties(), + }; + boundByJsonKey.set(JSON.stringify(paletteJson), letter); + } + + for (const [jsonKey, toolJson] of toolMatches) { + if (!isToolLayerVisible(this.context, toolJson)) continue; + const boundLetter = boundByJsonKey.get(jsonKey); + if (boundLetter !== undefined) { + const actionId: ActionIdentifier = `tool-${boundLetter}`; + const tool = globalToolBinder.bindings.get(boundLetter)!; + const label = + tool.context instanceof UserLayer + ? `${tool.description} — ${tool.context.managedLayer.name}` + : tool.description; + commands.push({ + kind: "action", + label, + shortcut: shortcutByAction.get(actionId) ?? "", + actionId, + }); + } else { + const capturedToolJson = toolJson; + commands.push({ + kind: "execute", + label: getToolDescription(this.context, toolJson), + shortcut: "", + execute: () => activateUnboundTool(this.context, capturedToolJson), + }); + } + } + } + + this.commands = commands; + this.changed.dispatch(); + } + + filter(searchString: string): readonly CommandPaletteEntry[] { + if (searchString === "") return this.commands; + + const query = searchString.toLowerCase(); + const prefixMatches: CommandPaletteEntry[] = []; + const substringMatches: CommandPaletteEntry[] = []; + + for (const command of this.commands) { + const label = command.label.toLowerCase(); + if (label.startsWith(query)) prefixMatches.push(command); + else if (label.includes(query)) substringMatches.push(command); + } + + return [...prefixMatches, ...substringMatches]; + } +} diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 165792c55d..bee4a0743e 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -15,432 +15,12 @@ */ import "#src/ui/command_palette.css"; -import type { LayerManager, SelectedLayerState } from "#src/layer/index.js"; -import { UserLayer } from "#src/layer/index.js"; import { Overlay } from "#src/overlay.js"; -import { - getMatchingTools, - restoreTool, - type GlobalToolBinder, -} from "#src/ui/tool.js"; -import { parseToolQuery } from "#src/ui/tool_query.js"; -import type { DebouncedFunction } from "#src/util/animation_frame_debounce.js"; -import { animationFrameDebounce } from "#src/util/animation_frame_debounce.js"; -import { RefCounted } from "#src/util/disposable.js"; import type { - ActionIdentifier, - EventAction, - NormalizedEventIdentifier, -} from "#src/util/event_action_map.js"; -import { friendlyEventIdentifier } from "#src/util/event_action_map.js"; -import { Signal } from "#src/util/signal.js"; -import type { InputEventBindings, Viewer } from "#src/viewer.js"; - -export interface CommandCatalogContext { - globalToolBinder: GlobalToolBinder; - layerManager: LayerManager; - selectedLayer: SelectedLayerState; - inputEventBindings: InputEventBindings; -} - -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; -} - -interface CommandPaletteEntryBase { - readonly label: string; - 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; -} - -// Runs a callback directly (no DOM action exists for it). -export interface ExecuteCommandEntry extends CommandPaletteEntryBase { - readonly kind: "execute"; - readonly execute: () => void; -} - -// Opens a sub-palette of `children` instead of activating anything itself. -export interface GroupCommandEntry extends CommandPaletteEntryBase { - readonly kind: "group"; - readonly children: readonly CommandPaletteEntry[]; -} - -export type CommandPaletteEntry = - | ActionCommandEntry - | ExecuteCommandEntry - | GroupCommandEntry; - -function formatKeyStroke(stroke: string): string { - return stroke - .split("+") - .map((part) => { - if (part.startsWith("key")) return part.substring(3); - if (part.startsWith("digit")) return part.substring(5); - if (part.startsWith("arrow")) return part.substring(5); - return part; - }) - .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") && - !normalizedId.includes("wheel") && - !normalizedId.includes("touch") && - !normalizedId.includes("click") - ); -} - -// Creates a Tool instance from a palette-form JSON object (with optional "layer" field). -// Caller is responsible for disposing the returned tool. -function createToolFromJson(context: CommandCatalogContext, toolJson: unknown) { - try { - const json = - typeof toolJson === "object" && toolJson !== null - ? (toolJson as Record) - : undefined; - const layerName = typeof json?.layer === "string" ? json.layer : undefined; - if (layerName !== undefined) { - const { layer: _ignored, ...rest } = json!; - const managedLayer = context.layerManager.getLayerByName(layerName); - const userLayer = managedLayer?.layer ?? null; - if (userLayer === null) return undefined; - return restoreTool(userLayer, rest); - } - // context is the viewer instance; restoreTool walks its prototype chain - // to find the registered tool factory. - return restoreTool(context, toolJson); - } catch { - return undefined; - } -} - -function getToolDescription( - context: CommandCatalogContext, - toolJson: unknown, -): string { - const tool = createToolFromJson(context, toolJson); - if (tool === undefined) return toolJsonToLabel(toolJson); - const label = - tool.context instanceof UserLayer - ? `${tool.description} — ${tool.context.managedLayer.name}` - : tool.description; - tool.dispose(); - return label; -} - -// Fallback label derived purely from the JSON structure (no instantiation). -function toolJsonToLabel(toolJson: unknown): string { - const json = - typeof toolJson === "object" && toolJson !== null - ? (toolJson as Record) - : undefined; - const typeName = - typeof toolJson === "string" - ? toolJson - : typeof json?.type === "string" - ? json.type - : undefined; - const layerName = typeof json?.layer === "string" ? json.layer : undefined; - const base = - typeName !== undefined - ? typeName - .replace(/([A-Z])/g, " $1") - .replace(/-./g, (s) => " " + s[1].toUpperCase()) - .replace(/^./, (s) => s.toUpperCase()) - .trim() - : "Unknown Tool"; - return layerName !== undefined ? `${base} — ${layerName}` : base; -} - -function isToolLayerVisible( - context: CommandCatalogContext, - toolJson: unknown, -): boolean { - const json = - typeof toolJson === "object" && toolJson !== null - ? (toolJson as Record) - : undefined; - const layerName = typeof json?.layer === "string" ? json.layer : undefined; - if (layerName === undefined) return true; - const managedLayer = context.layerManager.getLayerByName(layerName); - return managedLayer !== undefined && managedLayer.visible; -} - -function activateUnboundTool( - context: CommandCatalogContext, - toolJson: unknown, -): void { - const tool = createToolFromJson(context, toolJson); - if (tool === undefined) return; - // If the same tool is already bound to a key, activate that key directly - // rather than creating a duplicate. - const existingKey = tool.localBinder.jsonToKey.get( - JSON.stringify(tool.toJSON()), - ); - if (existingKey !== undefined) { - tool.dispose(); - context.globalToolBinder.activate(existingKey); - return; - } - // No key binding — activate directly without allocating a letter slot. - context.globalToolBinder.activateDirect(tool); -} - -/** - * Walk the event action maps available on the viewer and produce a list of - * every action with any keyboard binding. The first binding found for each - * action is kept; subsequent bindings for the same action are ignored. - */ -export function collectActionBindings( - inputEventBindings: InputEventBindings, -): readonly ActionBinding[] { - const seenBindings = new Map(); - - const collect = ( - bindings: Iterable<[NormalizedEventIdentifier, EventAction]>, - ) => { - for (const [normalizedId, eventAction] of bindings) { - if (!isKeyboardEvent(normalizedId)) continue; - if (eventAction.action === "open-command-palette") continue; - if (!seenBindings.has(eventAction.action)) { - seenBindings.set(eventAction.action, eventAction); - } - } - }; - - collect(inputEventBindings.global.entries()); - collect(inputEventBindings.sliceView.entries()); - collect(inputEventBindings.perspectiveView.entries()); - - return Array.from(seenBindings.entries(), ([actionId, eventAction]) => ({ - actionId, - eventAction, - })); -} - -/** - * 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. - * - * Actions can be represented hierarchically, with parent entries that - * expand to show child entries when activated. For example, - * layer actions (toggle-layer-N, select-layer-N, toggle-pick-layer-N) are - * replaced by three hierarchical entries whose children are the individual - * layer rows, enabling a two-step layer picker instead of a flat list. - */ -export class CommandCatalog extends RefCounted { - commands: readonly CommandPaletteEntry[] = []; - readonly changed = new Signal(); - private readonly debouncedRebuild: DebouncedFunction; - - constructor(private readonly context: CommandCatalogContext) { - super(); - const debouncedRebuild = (this.debouncedRebuild = this.registerCancellable( - animationFrameDebounce(() => this.rebuild()), - )); - this.registerDisposer( - context.globalToolBinder.changed.add(debouncedRebuild), - ); - this.registerDisposer( - context.globalToolBinder.localBindersChanged.add(debouncedRebuild), - ); - this.registerDisposer( - context.layerManager.layersChanged.add(debouncedRebuild), - ); - this.rebuild(); - } - - private rebuild() { - const { - globalToolBinder, - layerManager, - selectedLayer, - inputEventBindings, - } = 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. - const layers = layerManager?.managedLayers ?? []; - - commands.push({ - kind: "group", - label: "Toggle Layer", - shortcut: "1–9", - children: layers.map((layer, index) => ({ - kind: "execute", - label: layer.name, - shortcut: index < 9 ? String(index + 1) : "", - execute: () => layer.setVisible(!layer.visible), - })), - }); - - commands.push({ - kind: "group", - label: "Select Layer", - shortcut: "Ctrl+1–9", - children: layers.map((layer, index) => ({ - kind: "execute", - label: layer.name, - shortcut: index < 9 ? `Ctrl+${index + 1}` : "", - execute: () => { - selectedLayer.layer = layer; - selectedLayer.visible = true; - }, - })), - }); - - commands.push({ - kind: "group", - label: "Toggle Pick Layer", - shortcut: "Alt+1–9", - children: layers.map((layer, index) => ({ - kind: "execute", - label: layer.name, - shortcut: index < 9 ? `Alt+${index + 1}` : "", - execute: () => { - layer.pickEnabled = !layer.pickEnabled; - }, - })), - }); - - const bindings = collectActionBindings(inputEventBindings); - const shortcutByAction = new Map(); - for (const { actionId, eventAction } of bindings) { - shortcutByAction.set( - actionId, - formatKeyStroke( - friendlyEventIdentifier(eventAction.originalEventIdentifier ?? ""), - ), - ); - } - - 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 }); - } - - const toolQueryResult = parseToolQuery("+"); - if ("query" in toolQueryResult) { - // Tool listers report changes to their available tool set (e.g. controls - // that appear once a data source resolves) via this callback - let toolSetChanged = false; - const onListableToolsChanged = () => { - if (toolSetChanged) return; - toolSetChanged = true; - this.debouncedRebuild(); - }; - const toolMatches = getMatchingTools( - globalToolBinder, - toolQueryResult.query, - onListableToolsChanged, - ); - - // Build a reverse lookup from palette-JSON key to letter for currently-bound tools. - // Keys must include getCommonToolProperties() to match the keys produced by - // getMatchingTools, which merges commonProperties into every yielded tool JSON. - const boundByJsonKey = new Map(); - for (const [letter, tool] of globalToolBinder.bindings) { - const paletteJson = { - ...tool.localBinder.convertLocalJSONToPaletteJSON(tool.toJSON()), - ...tool.localBinder.getCommonToolProperties(), - }; - boundByJsonKey.set(JSON.stringify(paletteJson), letter); - } - - for (const [jsonKey, toolJson] of toolMatches) { - if (!isToolLayerVisible(this.context, toolJson)) continue; - const boundLetter = boundByJsonKey.get(jsonKey); - if (boundLetter !== undefined) { - const actionId: ActionIdentifier = `tool-${boundLetter}`; - const tool = globalToolBinder.bindings.get(boundLetter)!; - const label = - tool.context instanceof UserLayer - ? `${tool.description} — ${tool.context.managedLayer.name}` - : tool.description; - commands.push({ - kind: "action", - label, - shortcut: shortcutByAction.get(actionId) ?? "", - actionId, - }); - } else { - const capturedToolJson = toolJson; - commands.push({ - kind: "execute", - label: getToolDescription(this.context, toolJson), - shortcut: "", - execute: () => activateUnboundTool(this.context, capturedToolJson), - }); - } - } - } - - this.commands = commands; - this.changed.dispatch(); - } - - filter(searchString: string): readonly CommandPaletteEntry[] { - if (searchString === "") return this.commands; - - const query = searchString.toLowerCase(); - const prefixMatches: CommandPaletteEntry[] = []; - const substringMatches: CommandPaletteEntry[] = []; - - for (const command of this.commands) { - const label = command.label.toLowerCase(); - if (label.startsWith(query)) prefixMatches.push(command); - else if (label.includes(query)) substringMatches.push(command); - } - - return [...prefixMatches, ...substringMatches]; - } -} + CommandCatalog, + CommandPaletteEntry, +} from "#src/ui/command_catalog.js"; +import type { Viewer } from "#src/viewer.js"; export class CommandPalette extends Overlay { private readonly searchInput: HTMLInputElement; diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index 19a92ca261..8adeb5e8b3 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -15,7 +15,8 @@ */ import { StatusMessage } from "#src/status.js"; -import { bindCommandPalette, CommandCatalog } from "#src/ui/command_palette.js"; +import { CommandCatalog } from "#src/ui/command_catalog.js"; +import { bindCommandPalette } from "#src/ui/command_palette.js"; import { bindDefaultCopyHandler, bindDefaultPasteHandler, From d923d18f86ee4068011b9589350406e7f83c60da Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 20 Jul 2026 10:53:08 +0300 Subject: [PATCH 13/30] 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 14/30] 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 15/30] 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 16/30] 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 17/30] 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 18/30] 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 19/30] 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( From f997b25908f90b905de90aa5bf1ac48c7774f0b9 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Sun, 16 Aug 2026 21:33:48 +0200 Subject: [PATCH 20/30] refactor: change command catalog to be owned by viewer also binds to that command palette in the default setup --- src/ui/command_palette.ts | 10 +++------- src/ui/default_viewer_setup.ts | 3 +-- src/viewer.ts | 8 ++++++++ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 0fec7e3f20..6857723c85 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -288,15 +288,11 @@ export class CommandPalette extends Overlay { /** * Binds the command palette to a viewer by handling the "open-command-palette" * action at the viewer element level, the same way every other global action - * (e.g. "help") is bound. The action is dispatched by the configured - * `control+keyp` binding in the viewer's input event map. This intentionally + * (e.g. "help") is bound. This intentionally * does not install any document-level key listener, so the palette opens from * the main viewer UI but does not intercept keystrokes globally. */ -export function bindCommandPalette( - viewer: Viewer, - catalog: CommandCatalog, -): void { +export function bindCommandPalette(viewer: Viewer): void { let openPalette: CommandPalette | undefined; const openCommandPalette = () => { if (openPalette !== undefined && !openPalette.wasDisposed) return; @@ -308,7 +304,7 @@ export function bindCommandPalette( prevFocused instanceof HTMLElement && viewer.element.contains(prevFocused) ? prevFocused : viewer.element; - openPalette = new CommandPalette(catalog, dispatchTarget); + openPalette = new CommandPalette(viewer.commandCatalog, dispatchTarget); }; viewer.bindAction("open-command-palette", openCommandPalette); } diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index b00de52e3a..aee0ea6c69 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -66,8 +66,7 @@ export function setupDefaultViewer(options?: Partial) { bindDefaultCopyHandler(viewer); bindDefaultPasteHandler(viewer); registerDefaultCommands(viewer.commandRegistry); - const catalog = viewer.registerDisposer(new CommandCatalog(viewer)); - bindCommandPalette(viewer, catalog); + bindCommandPalette(viewer); return viewer; } diff --git a/src/viewer.ts b/src/viewer.ts index 2f75fa1e2a..b243281408 100644 --- a/src/viewer.ts +++ b/src/viewer.ts @@ -82,6 +82,7 @@ import { observeWatchable, TrackableValue, } from "#src/trackable_value.js"; +import { CommandCatalog } from "#src/ui/command_catalog.js"; import { CommandRegistry } from "#src/ui/command_registry.js"; import { LayerArchiveCountWidget, @@ -1182,6 +1183,13 @@ export class Viewer extends RefCounted implements ViewerState { // commands it was told about, not every action that exists - see // docs/concepts/commands.rst. public commandRegistry = this.registerDisposer(new CommandRegistry()); + private commandCatalog_: CommandCatalog | undefined; + + get commandCatalog(): CommandCatalog { + return (this.commandCatalog_ ??= this.registerDisposer( + new CommandCatalog(this), + )); + } public toolBinder = this.registerDisposer( new LocalToolBinder(this, this.globalToolBinder), From 11628914bd79c5f1ed242b8220af6f589ddf2adf Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 17 Aug 2026 13:32:08 +0300 Subject: [PATCH 21/30] fix: remove unused CommandCatalog import 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. --- src/ui/default_viewer_setup.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ui/default_viewer_setup.ts b/src/ui/default_viewer_setup.ts index aee0ea6c69..f42d9cbaaa 100644 --- a/src/ui/default_viewer_setup.ts +++ b/src/ui/default_viewer_setup.ts @@ -15,7 +15,6 @@ */ import { StatusMessage } from "#src/status.js"; -import { CommandCatalog } from "#src/ui/command_catalog.js"; import { bindCommandPalette } from "#src/ui/command_palette.js"; import { bindDefaultCopyHandler, From 7b87129b2bc59da0612bfee00f59d53898370308 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 14:37:27 +0200 Subject: [PATCH 22/30] refactor: extract formatKeyStroke to common --- src/help/input_event_bindings.ts | 19 +------------------ src/ui/command.ts | 18 ++++++++++++++++++ src/ui/command_catalog.ts | 14 +------------- 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/help/input_event_bindings.ts b/src/help/input_event_bindings.ts index ff5ec6b1b2..3fdcf41d0a 100644 --- a/src/help/input_event_bindings.ts +++ b/src/help/input_event_bindings.ts @@ -17,6 +17,7 @@ import "#src/help/input_event_bindings.css"; import type { LayerManager } from "#src/layer/index.js"; import { UserLayer } from "#src/layer/index.js"; +import { formatKeyStroke } from "#src/ui/command.js"; import type { SidePanelManager } from "#src/ui/side_panel.js"; import { SidePanel } from "#src/ui/side_panel.js"; import type { SidePanelLocation } from "#src/ui/side_panel_location.js"; @@ -37,24 +38,6 @@ declare let NEUROGLANCER_BUILD_INFO: | { tag: string; url?: string; timestamp?: string } | undefined; -export function formatKeyName(name: string) { - if (name.startsWith("key")) { - return name.substring(3); - } - if (name.startsWith("digit")) { - return name.substring(5); - } - if (name.startsWith("arrow")) { - return name.substring(5); - } - return name; -} - -export function formatKeyStroke(stroke: string) { - const parts = stroke.split("+"); - return parts.map(formatKeyName).join("+"); -} - const DEFAULT_HELP_PANEL_LOCATION: SidePanelLocation = { ...DEFAULT_SIDE_PANEL_LOCATION, side: "left", diff --git a/src/ui/command.ts b/src/ui/command.ts index 4d72688651..6c0b3a1b08 100644 --- a/src/ui/command.ts +++ b/src/ui/command.ts @@ -123,3 +123,21 @@ export class CallbackCommand extends Command { this.callback(context); } } + +export function formatKeyName(name: string) { + if (name.startsWith("key")) { + return name.substring(3); + } + if (name.startsWith("digit")) { + return name.substring(5); + } + if (name.startsWith("arrow")) { + return name.substring(5); + } + return name; +} + +export function formatKeyStroke(stroke: string) { + const parts = stroke.split("+"); + return parts.map(formatKeyName).join("+"); +} diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 72dd83fbd6..8129c578c3 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -17,7 +17,7 @@ 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 { ActionCommand, formatKeyStroke } from "#src/ui/command.js"; import type { CommandRegistry } from "#src/ui/command_registry.js"; import { getMatchingTools, @@ -86,18 +86,6 @@ export type CommandPaletteEntry = | ExecuteCommandEntry | GroupCommandEntry; -function formatKeyStroke(stroke: string): string { - return stroke - .split("+") - .map((part) => { - if (part.startsWith("key")) return part.substring(3); - if (part.startsWith("digit")) return part.substring(5); - if (part.startsWith("arrow")) return part.substring(5); - return part; - }) - .join("+"); -} - // Fallback label for a bound action that no command was registered for. function actionIdToLabel(actionId: ActionIdentifier): string { return actionId From 2dec258f8095bc041d20eff833ecc7386a7fae93 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 14:52:56 +0200 Subject: [PATCH 23/30] refactor: remove execute types in command catalog Instead they are directly CallbackCommands --- src/ui/command_catalog.ts | 82 ++++++++++++++++++++++++--------------- src/ui/command_palette.ts | 4 +- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 8129c578c3..b0e612cb0a 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -17,7 +17,11 @@ 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, formatKeyStroke } from "#src/ui/command.js"; +import { + ActionCommand, + CallbackCommand, + formatKeyStroke, +} from "#src/ui/command.js"; import type { CommandRegistry } from "#src/ui/command_registry.js"; import { getMatchingTools, @@ -56,35 +60,29 @@ export interface ActionBinding { readonly eventAction: EventAction; } -interface CommandPaletteEntryBase { - readonly label: string; - readonly shortcut: string; +export type CommandSource = "registered" | "derived"; + +export interface CommandEntryBase { + kind: string; + shortcut: string; + label: string; } // 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 { +export interface CommandEntry extends CommandEntryBase { readonly kind: "command"; readonly command: Command; -} - -// 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; + readonly source: CommandSource; } // Opens a sub-palette of `children` instead of activating anything itself. -export interface GroupCommandEntry extends CommandPaletteEntryBase { +export interface GroupCommandEntry extends CommandEntryBase { readonly kind: "group"; readonly children: readonly CommandPaletteEntry[]; } -export type CommandPaletteEntry = - | CommandEntry - | ExecuteCommandEntry - | GroupCommandEntry; +export type CommandPaletteEntry = CommandEntry | GroupCommandEntry; // Fallback label for a bound action that no command was registered for. function actionIdToLabel(actionId: ActionIdentifier): string { @@ -288,10 +286,15 @@ export class CommandCatalog extends RefCounted { label: "Toggle Layer", shortcut: "1–9", children: layers.map((layer, index) => ({ - kind: "execute", + kind: "command", label: layer.name, shortcut: index < 9 ? String(index + 1) : "", - execute: () => layer.setVisible(!layer.visible), + source: "derived", + command: new CallbackCommand( + `toggle-layer-${index + 1}`, + layer.name, + () => layer.setVisible(!layer.visible), + ), })), }); @@ -300,13 +303,18 @@ export class CommandCatalog extends RefCounted { label: "Select Layer", shortcut: "Ctrl+1–9", children: layers.map((layer, index) => ({ - kind: "execute", + kind: "command", label: layer.name, shortcut: index < 9 ? `Ctrl+${index + 1}` : "", - execute: () => { - selectedLayer.layer = layer; - selectedLayer.visible = true; - }, + source: "derived", + command: new CallbackCommand( + `select-layer-${index + 1}`, + layer.name, + () => { + selectedLayer.layer = layer; + selectedLayer.visible = true; + }, + ), })), }); @@ -315,12 +323,17 @@ export class CommandCatalog extends RefCounted { label: "Toggle Pick Layer", shortcut: "Alt+1–9", children: layers.map((layer, index) => ({ - kind: "execute", + kind: "command", label: layer.name, shortcut: index < 9 ? `Alt+${index + 1}` : "", - execute: () => { - layer.pickEnabled = !layer.pickEnabled; - }, + source: "derived", + command: new CallbackCommand( + `toggle-pick-layer-${index + 1}`, + layer.name, + () => { + layer.pickEnabled = !layer.pickEnabled; + }, + ), })), }); @@ -343,6 +356,7 @@ export class CommandCatalog extends RefCounted { kind: "command", label: command.label, shortcut: shortcutByAction.get(command.id) ?? "", + source: "registered", command, }); } @@ -361,6 +375,7 @@ export class CommandCatalog extends RefCounted { kind: "command", label, shortcut: shortcutByAction.get(actionId) ?? "", + source: "derived", command: new ActionCommand(actionId, label), }); } @@ -407,15 +422,20 @@ export class CommandCatalog extends RefCounted { kind: "command", label, shortcut: shortcutByAction.get(actionId) ?? "", + source: "derived", command: new ActionCommand(actionId, label), }); } else { const capturedToolJson = toolJson; + const label = getToolDescription(this.context, toolJson); commands.push({ - kind: "execute", - label: getToolDescription(this.context, toolJson), + kind: "command", + label, shortcut: "", - execute: () => activateUnboundTool(this.context, capturedToolJson), + source: "derived", + command: new CallbackCommand(jsonKey, label, () => + activateUnboundTool(this.context, capturedToolJson), + ), }); } } diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 6857723c85..dfd24320a1 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -277,9 +277,7 @@ export class CommandPalette extends Overlay { this.closeAndRestoreFocus(); - if (command.kind === "execute") { - command.execute(); - } else if (command.kind === "command") { + if (command.kind === "command") { command.command.invoke({ dispatchTarget: this.actionDispatchTarget }); } } From 4157c65b4e509bfbc74b1ef91fc8f6087ca3ec36 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 15:24:21 +0200 Subject: [PATCH 24/30] refactor: remove command group kind 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 --- src/ui/command_catalog.spec.ts | 12 +- src/ui/command_catalog.ts | 114 +++++++++--------- src/ui/command_palette.ts | 209 +++++++++++++++++++-------------- 3 files changed, 179 insertions(+), 156 deletions(-) diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index cb5f2b4fdd..d0d7197c60 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -167,10 +167,6 @@ describe("CommandCatalog command sources", () => { ); } - 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"); @@ -180,7 +176,7 @@ describe("CommandCatalog command sources", () => { ); const catalog = makeCatalog(map, registry); try { - const entries = commandEntries(catalog).filter( + const entries = catalog.commands.filter( (entry) => entry.command.id === "toggle-scale-bar", ); expect(entries).toHaveLength(1); @@ -198,7 +194,7 @@ describe("CommandCatalog command sources", () => { map.set("keyq", "embedder-action"); const catalog = makeCatalog(map, new CommandRegistry()); try { - const entries = commandEntries(catalog).filter( + const entries = catalog.commands.filter( (entry) => entry.command.id === "embedder-action", ); expect(entries).toHaveLength(1); @@ -220,7 +216,7 @@ describe("CommandCatalog command sources", () => { const catalog = makeCatalog(map, registry); try { expect( - commandEntries(catalog).filter( + catalog.commands.filter( (entry) => entry.command.id === "toggle-scale-bar", ), ).toHaveLength(1); @@ -239,7 +235,7 @@ describe("CommandCatalog command sources", () => { const catalog = makeCatalog(map, registry); try { expect( - commandEntries(catalog).filter( + catalog.commands.filter( (entry) => entry.command.id === "toggle-scale-bar", ), ).toHaveLength(0); diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index b0e612cb0a..93df599199 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -62,28 +62,22 @@ export interface ActionBinding { export type CommandSource = "registered" | "derived"; -export interface CommandEntryBase { - kind: string; - shortcut: string; - label: string; +// Can identify the sub-palette an entry belongs to. +export interface CommandGroup { + readonly label: string; + readonly shortcut: string; } // 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 CommandEntryBase { - readonly kind: "command"; +export interface CommandEntry { + readonly shortcut: string; + readonly label: string; readonly command: Command; readonly source: CommandSource; + readonly group?: CommandGroup; } -// Opens a sub-palette of `children` instead of activating anything itself. -export interface GroupCommandEntry extends CommandEntryBase { - readonly kind: "group"; - readonly children: readonly CommandPaletteEntry[]; -} - -export type CommandPaletteEntry = CommandEntry | GroupCommandEntry; - // Fallback label for a bound action that no command was registered for. function actionIdToLabel(actionId: ActionIdentifier): string { return actionId @@ -125,6 +119,7 @@ function createToolFromJson(context: CommandCatalogContext, toolJson: unknown) { } } +// Attemp for full description of tool by creating then disposing function getToolDescription( context: CommandCatalogContext, toolJson: unknown, @@ -139,7 +134,7 @@ function getToolDescription( return label; } -// Fallback label derived purely from the JSON structure (no instantiation). +// Fallback label derived purely from the JSON structure function toolJsonToLabel(toolJson: unknown): string { const json = typeof toolJson === "object" && toolJson !== null @@ -183,18 +178,16 @@ function activateUnboundTool( ): void { const tool = createToolFromJson(context, toolJson); if (tool === undefined) return; - // If the same tool is already bound to a key, activate that key directly - // rather than creating a duplicate. + const existingKey = tool.localBinder.jsonToKey.get( JSON.stringify(tool.toJSON()), ); if (existingKey !== undefined) { tool.dispose(); context.globalToolBinder.activate(existingKey); - return; + } else { + context.globalToolBinder.activateDirect(tool); } - // No key binding — activate directly without allocating a letter slot. - context.globalToolBinder.activateDirect(tool); } /** @@ -235,14 +228,12 @@ export function collectActionBindings( * animationFrameDebounce so the palette always reflects current viewer state * without rebuilding from scratch on every open. * - * Actions can be represented hierarchically, with parent entries that - * expand to show child entries when activated. For example, - * layer actions (toggle-layer-N, select-layer-N, toggle-pick-layer-N) are - * replaced by three hierarchical entries whose children are the individual - * layer rows, enabling a two-step layer picker instead of a flat list. + * `commands` is always flat: entries that belong together (e.g. the per-layer + * toggle-layer-N actions) share a `group`. It is up to the consumer + * if and how they wish to use this group. */ export class CommandCatalog extends RefCounted { - commands: readonly CommandPaletteEntry[] = []; + commands: readonly CommandEntry[] = []; readonly changed = new Signal(); private readonly debouncedRebuild: DebouncedFunction; @@ -274,39 +265,38 @@ export class CommandCatalog extends RefCounted { inputEventBindings, commandRegistry, } = this.context; - const commands: CommandPaletteEntry[] = []; + const commands: CommandEntry[] = []; - // 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. const layers = layerManager?.managedLayers ?? []; - commands.push({ - kind: "group", + const toggleLayerGroup: CommandGroup = { label: "Toggle Layer", shortcut: "1–9", - children: layers.map((layer, index) => ({ - kind: "command", + }; + for (const [index, layer] of layers.entries()) { + commands.push({ label: layer.name, shortcut: index < 9 ? String(index + 1) : "", source: "derived", + group: toggleLayerGroup, command: new CallbackCommand( `toggle-layer-${index + 1}`, layer.name, () => layer.setVisible(!layer.visible), ), - })), - }); + }); + } - commands.push({ - kind: "group", + const selectLayerGroup: CommandGroup = { label: "Select Layer", shortcut: "Ctrl+1–9", - children: layers.map((layer, index) => ({ - kind: "command", + }; + for (const [index, layer] of layers.entries()) { + commands.push({ label: layer.name, shortcut: index < 9 ? `Ctrl+${index + 1}` : "", source: "derived", + group: selectLayerGroup, command: new CallbackCommand( `select-layer-${index + 1}`, layer.name, @@ -315,18 +305,19 @@ export class CommandCatalog extends RefCounted { selectedLayer.visible = true; }, ), - })), - }); + }); + } - commands.push({ - kind: "group", + const togglePickLayerGroup: CommandGroup = { label: "Toggle Pick Layer", shortcut: "Alt+1–9", - children: layers.map((layer, index) => ({ - kind: "command", + }; + for (const [index, layer] of layers.entries()) { + commands.push({ label: layer.name, shortcut: index < 9 ? `Alt+${index + 1}` : "", source: "derived", + group: togglePickLayerGroup, command: new CallbackCommand( `toggle-pick-layer-${index + 1}`, layer.name, @@ -334,8 +325,8 @@ export class CommandCatalog extends RefCounted { layer.pickEnabled = !layer.pickEnabled; }, ), - })), - }); + }); + } const bindings = collectActionBindings(inputEventBindings); const shortcutByAction = new Map(); @@ -353,7 +344,6 @@ export class CommandCatalog extends RefCounted { for (const command of commandRegistry.values()) { if (!command.enabled) continue; commands.push({ - kind: "command", label: command.label, shortcut: shortcutByAction.get(command.id) ?? "", source: "registered", @@ -368,11 +358,9 @@ export class CommandCatalog extends RefCounted { 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) ?? "", source: "derived", @@ -419,7 +407,6 @@ export class CommandCatalog extends RefCounted { ? `${tool.description} — ${tool.context.managedLayer.name}` : tool.description; commands.push({ - kind: "command", label, shortcut: shortcutByAction.get(actionId) ?? "", source: "derived", @@ -429,7 +416,6 @@ export class CommandCatalog extends RefCounted { const capturedToolJson = toolJson; const label = getToolDescription(this.context, toolJson); commands.push({ - kind: "command", label, shortcut: "", source: "derived", @@ -445,14 +431,26 @@ export class CommandCatalog extends RefCounted { this.changed.dispatch(); } - filter(searchString: string): readonly CommandPaletteEntry[] { - if (searchString === "") return this.commands; + // Restricting to `groupLabel` scopes the search to a single group's entries + filter( + searchString: string, + groupLabel?: string, + ignoreGroupsForGlobalSearch: boolean = true, + ): readonly CommandEntry[] { + let pool = this.commands; + if (groupLabel !== undefined) { + pool = pool.filter((entry) => entry.group?.label === groupLabel); + } else if (ignoreGroupsForGlobalSearch) { + pool = pool.filter((entry) => entry.group === undefined); + } + + if (searchString === "") return pool; const query = searchString.toLowerCase(); - const prefixMatches: CommandPaletteEntry[] = []; - const substringMatches: CommandPaletteEntry[] = []; + const prefixMatches: CommandEntry[] = []; + const substringMatches: CommandEntry[] = []; - for (const command of this.commands) { + for (const command of pool) { const label = command.label.toLowerCase(); if (label.startsWith(query)) prefixMatches.push(command); else if (label.includes(query)) substringMatches.push(command); diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index dfd24320a1..0ed7b42310 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -18,24 +18,28 @@ import "#src/ui/command_palette.css"; import { Overlay } from "#src/overlay.js"; import type { CommandCatalog, - CommandPaletteEntry, + CommandEntry, + CommandGroup, } from "#src/ui/command_catalog.js"; import type { Viewer } from "#src/viewer.js"; +type PaletteRow = + | { readonly kind: "command"; readonly entry: CommandEntry } + | { readonly kind: "group-header"; readonly group: CommandGroup }; + export class CommandPalette extends Overlay { private readonly searchInput: HTMLInputElement; private readonly resultsList: HTMLElement; - private readonly rowByCommand = new Map(); + private readonly rowElementByKey = new Map< + CommandEntry | CommandGroup, + HTMLElement + >(); private readonly emptyElement: HTMLElement; private readonly pickerHeaderElement: HTMLElement; - private filteredCommands: readonly CommandPaletteEntry[] = []; - private filteredRows: HTMLElement[] = []; + private filteredRows: readonly PaletteRow[] = []; + private filteredRowElements: HTMLElement[] = []; private activeIndex = 0; - private currentCommands: readonly CommandPaletteEntry[]; - private readonly levelStack: { - commands: readonly CommandPaletteEntry[]; - label: string; - }[] = []; + private currentGroup: CommandGroup | undefined; private readonly keyHandlers: Partial< Record void> @@ -51,17 +55,17 @@ export class CommandPalette extends Overlay { Enter: (event) => { event.preventDefault(); event.stopPropagation(); - if (this.filteredCommands.length > 0) - this.run(this.filteredCommands[this.activeIndex]); + if (this.filteredRows.length > 0) + this.run(this.filteredRows[this.activeIndex]); }, Backspace: () => { - if (this.levelStack.length > 0 && this.searchInput.value === "") { + if (this.currentGroup !== undefined && this.searchInput.value === "") { this.goBack(); } }, ArrowLeft: (event) => { if ( - this.levelStack.length > 0 && + this.currentGroup !== undefined && this.searchInput.selectionStart === 0 && this.searchInput.selectionEnd === 0 ) { @@ -70,7 +74,7 @@ export class CommandPalette extends Overlay { } }, Escape: () => { - if (this.levelStack.length > 0) { + if (this.currentGroup !== undefined) { this.goBack(); } else { this.closeAndRestoreFocus(); @@ -85,8 +89,6 @@ export class CommandPalette extends Overlay { super(); this.content.classList.add("neuroglancer-command-palette"); - this.currentCommands = this.catalog.commands; - const pickerHeader = (this.pickerHeaderElement = document.createElement("div")); pickerHeader.className = "neuroglancer-command-palette-picker-header"; @@ -113,7 +115,7 @@ export class CommandPalette extends Overlay { resultsList.className = "neuroglancer-command-palette-results"; this.content.appendChild(resultsList); - this.buildRows(this.catalog.commands); + this.buildRows(); searchInput.addEventListener("input", () => { this.activeIndex = 0; @@ -136,15 +138,13 @@ export class CommandPalette extends Overlay { }); // The catalog may rebuild while this palette is open (a layer or tool - // change, or an async lister resolving). Build rows for the new entries so - // top-level filtering can be applied to the new entries. + // change, or an async lister resolving). Build rows for the new entries + // and re-render, since the current view (grouped or not) is always + // derived live from the catalog rather than a snapshot. this.registerDisposer( this.catalog.changed.add(() => { - this.buildRows(this.catalog.commands); - if (this.levelStack.length === 0) { - this.currentCommands = this.catalog.commands; - this.render(); - } + this.buildRows(); + this.render(); }), ); @@ -152,84 +152,121 @@ export class CommandPalette extends Overlay { searchInput.focus(); } - private buildRows(commands: readonly CommandPaletteEntry[]) { - for (const command of commands) { - if (this.rowByCommand.has(command)) continue; - - const commandRow = document.createElement("div"); - commandRow.className = "neuroglancer-command-palette-row"; - commandRow.addEventListener("click", () => this.run(command)); - - const labelElement = document.createElement("span"); - labelElement.textContent = command.label; - commandRow.appendChild(labelElement); - - if (command.shortcut) { - const shortcutElement = document.createElement("span"); - shortcutElement.className = "neuroglancer-command-palette-shortcut"; - shortcutElement.textContent = command.shortcut; - commandRow.appendChild(shortcutElement); + private buildRows() { + for (const entry of this.catalog.commands) { + if (!this.rowElementByKey.has(entry)) { + this.rowElementByKey.set( + entry, + this.createRowElement(entry.label, entry.shortcut, () => + this.run({ kind: "command", entry }), + ), + ); } - - this.rowByCommand.set(command, commandRow); - - if (command.kind === "group") { - this.buildRows(command.children); + const { group } = entry; + if (group !== undefined && !this.rowElementByKey.has(group)) { + this.rowElementByKey.set( + group, + this.createRowElement(group.label, group.shortcut, () => + this.run({ kind: "group-header", group }), + ), + ); } } } - private filterCurrentLevel(): readonly CommandPaletteEntry[] { - if (this.levelStack.length === 0) { - return this.catalog.filter(this.searchInput.value); + private createRowElement( + label: string, + shortcut: string, + onActivate: () => void, + ): HTMLElement { + const rowElement = document.createElement("div"); + rowElement.className = "neuroglancer-command-palette-row"; + rowElement.addEventListener("click", onActivate); + + const labelElement = document.createElement("span"); + labelElement.textContent = label; + rowElement.appendChild(labelElement); + + if (shortcut) { + const shortcutElement = document.createElement("span"); + shortcutElement.className = "neuroglancer-command-palette-shortcut"; + shortcutElement.textContent = shortcut; + rowElement.appendChild(shortcutElement); } - const query = this.searchInput.value.toLowerCase(); - if (query === "") return this.currentCommands; - const prefixMatches: CommandPaletteEntry[] = []; - const substringMatches: CommandPaletteEntry[] = []; - for (const entry of this.currentCommands) { - const label = entry.label.toLowerCase(); - if (label.startsWith(query)) prefixMatches.push(entry); - else if (label.includes(query)) substringMatches.push(entry); + + return rowElement; + } + + // Inside a group, or while searching, the view is a flat list of matching + // commands. Otherwise, entries sharing a group collapse into one header row. + private computeDisplayRows(): readonly PaletteRow[] { + const searchValue = this.searchInput.value; + if (this.currentGroup !== undefined) { + return this.catalog + .filter(searchValue, this.currentGroup.label) + .map((entry) => ({ kind: "command", entry }) as const); + } + if (searchValue !== "") { + return this.catalog + .filter(searchValue) + .map((entry) => ({ kind: "command", entry }) as const); + } + const rows: PaletteRow[] = []; + const seenGroups = new Set(); + for (const entry of this.catalog.commands) { + const { group } = entry; + if (group === undefined) { + rows.push({ kind: "command", entry }); + } else if (!seenGroups.has(group.label)) { + seenGroups.add(group.label); + rows.push({ kind: "group-header", group }); + } } - return [...prefixMatches, ...substringMatches]; + return rows; + } + + private rowElementFor(row: PaletteRow): HTMLElement { + return this.rowElementByKey.get( + row.kind === "command" ? row.entry : row.group, + )!; } private render() { - this.filteredCommands = this.filterCurrentLevel(); - if (this.activeIndex >= this.filteredCommands.length) { - this.activeIndex = Math.max(0, this.filteredCommands.length - 1); + this.filteredRows = this.computeDisplayRows(); + if (this.activeIndex >= this.filteredRows.length) { + this.activeIndex = Math.max(0, this.filteredRows.length - 1); } - if (this.filteredCommands.length === 0) { + if (this.filteredRows.length === 0) { this.resultsList.replaceChildren(this.emptyElement); return; } - this.filteredRows = this.filteredCommands.map( - (command) => this.rowByCommand.get(command)!, + this.filteredRowElements = this.filteredRows.map((row) => + this.rowElementFor(row), ); - this.filteredRows.forEach((commandRow, rowIndex) => { - commandRow.toggleAttribute("data-active", rowIndex === this.activeIndex); + this.filteredRowElements.forEach((rowElement, rowIndex) => { + rowElement.toggleAttribute("data-active", rowIndex === this.activeIndex); }); - this.resultsList.replaceChildren(...this.filteredRows); + this.resultsList.replaceChildren(...this.filteredRowElements); } private setActive(targetIndex: number) { - if (this.filteredRows.length === 0) return; + if (this.filteredRowElements.length === 0) return; this.activeIndex = - ((targetIndex % this.filteredRows.length) + this.filteredRows.length) % - this.filteredRows.length; - this.filteredRows.forEach((commandRow, rowIndex) => { - commandRow.toggleAttribute("data-active", rowIndex === this.activeIndex); + ((targetIndex % this.filteredRowElements.length) + + this.filteredRowElements.length) % + this.filteredRowElements.length; + this.filteredRowElements.forEach((rowElement, rowIndex) => { + rowElement.toggleAttribute("data-active", rowIndex === this.activeIndex); if (rowIndex === this.activeIndex) - commandRow.scrollIntoView({ block: "nearest" }); + rowElement.scrollIntoView({ block: "nearest" }); }); } private updateHeader() { - if (this.levelStack.length > 0) { - this.pickerHeaderElement.textContent = `← ${this.levelStack.at(-1)!.label}`; + if (this.currentGroup !== undefined) { + this.pickerHeaderElement.textContent = `← ${this.currentGroup.label}`; this.pickerHeaderElement.removeAttribute("hidden"); } else { this.pickerHeaderElement.setAttribute("hidden", ""); @@ -237,12 +274,11 @@ export class CommandPalette extends Overlay { } private goBack() { - if (this.levelStack.length === 0) { + if (this.currentGroup === undefined) { this.closeAndRestoreFocus(); return; } - const previous = this.levelStack.pop()!; - this.currentCommands = previous.commands; + this.currentGroup = undefined; this.searchInput.value = ""; this.searchInput.placeholder = "Type a command..."; this.updateHeader(); @@ -260,15 +296,11 @@ export class CommandPalette extends Overlay { target.focus({ preventScroll: true }); } - private run(command: CommandPaletteEntry) { - if (command.kind === "group" && command.children.length > 0) { - this.levelStack.push({ - commands: this.currentCommands, - label: command.label, - }); - this.currentCommands = command.children; + private run(row: PaletteRow) { + if (row.kind === "group-header") { + this.currentGroup = row.group; this.searchInput.value = ""; - this.searchInput.placeholder = `Filter ${command.label}…`; + this.searchInput.placeholder = `Filter ${row.group.label}…`; this.updateHeader(); this.activeIndex = 0; this.render(); @@ -276,10 +308,7 @@ export class CommandPalette extends Overlay { } this.closeAndRestoreFocus(); - - if (command.kind === "command") { - command.command.invoke({ dispatchTarget: this.actionDispatchTarget }); - } + row.entry.command.invoke({ dispatchTarget: this.actionDispatchTarget }); } } From 79dda43660d14a2b75b6c1fc0d37843acea59315 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 15:30:13 +0200 Subject: [PATCH 25/30] feat: give layer command labels more info since they can be used standalone --- src/ui/command_catalog.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 93df599199..9d7c92ffe2 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -281,7 +281,7 @@ export class CommandCatalog extends RefCounted { group: toggleLayerGroup, command: new CallbackCommand( `toggle-layer-${index + 1}`, - layer.name, + `Show/hide ${layer.name}`, () => layer.setVisible(!layer.visible), ), }); @@ -299,7 +299,7 @@ export class CommandCatalog extends RefCounted { group: selectLayerGroup, command: new CallbackCommand( `select-layer-${index + 1}`, - layer.name, + `Select ${layer.name}`, () => { selectedLayer.layer = layer; selectedLayer.visible = true; @@ -320,7 +320,7 @@ export class CommandCatalog extends RefCounted { group: togglePickLayerGroup, command: new CallbackCommand( `toggle-pick-layer-${index + 1}`, - layer.name, + `Toggle pick ${layer.name}`, () => { layer.pickEnabled = !layer.pickEnabled; }, From edce418294342858405d3cf0418b7731c5da8669 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 16:30:48 +0200 Subject: [PATCH 26/30] refactor: make context requirement for tools explicit --- src/ui/command_catalog.spec.ts | 1 + src/ui/command_catalog.ts | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index d0d7197c60..00c6a88588 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -63,6 +63,7 @@ function makeContext( bindings: new Map(), localBinders: new Set(), }, + toolBinder: { context: {} }, layerManager: { layersChanged: noopSignal, managedLayers: [], diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 9d7c92ffe2..5c5bc5af0b 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -27,6 +27,7 @@ import { getMatchingTools, restoreTool, type GlobalToolBinder, + type LocalToolBinder, } from "#src/ui/tool.js"; import { parseToolQuery } from "#src/ui/tool_query.js"; import type { DebouncedFunction } from "#src/util/animation_frame_debounce.js"; @@ -42,17 +43,23 @@ import { Signal } from "#src/util/signal.js"; import type { InputEventBindings } from "#src/viewer.js"; export interface CommandCatalogContext { - globalToolBinder: GlobalToolBinder; - layerManager: LayerManager; - selectedLayer: SelectedLayerState; - inputEventBindings: InputEventBindings; + readonly globalToolBinder: GlobalToolBinder; + /** + * Tool context for tools that are not scoped to a layer. Tool factories are + * registered against a class prototype, so the binder's own context has to be + * a real instance of such a class rather than an arbitrary object. + */ + readonly toolBinder: LocalToolBinder; + readonly layerManager: LayerManager; + readonly selectedLayer: SelectedLayerState; + readonly inputEventBindings: InputEventBindings; /** * 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; + readonly commandRegistry: CommandRegistry; } export interface ActionBinding { @@ -111,9 +118,7 @@ function createToolFromJson(context: CommandCatalogContext, toolJson: unknown) { if (userLayer === null) return undefined; return restoreTool(userLayer, rest); } - // context is the viewer instance; restoreTool walks its prototype chain - // to find the registered tool factory. - return restoreTool(context, toolJson); + return restoreTool(context.toolBinder.context, toolJson); } catch { return undefined; } From 0ff24df3c27cdabf0e8c74fd064351ec0986caf1 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 16:44:15 +0200 Subject: [PATCH 27/30] test: small extra test on source --- src/ui/command_catalog.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index 00c6a88588..0b9060b2aa 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -183,6 +183,7 @@ describe("CommandCatalog command sources", () => { expect(entries).toHaveLength(1); expect(entries[0].label).toBe("Toggle Scale Bar"); expect(entries[0].shortcut).toBe("b"); + expect(entries[0].source).toBe("registered"); } finally { catalog.dispose(); } @@ -202,6 +203,7 @@ describe("CommandCatalog command sources", () => { // Falls back to a label derived from the action id. expect(entries[0].label).toBe("Embedder Action"); expect(entries[0].shortcut).toBe("q"); + expect(entries[0].source).toBe("derived") } finally { catalog.dispose(); } From 5ff58f623e76a6bb4aa5315f6b49d40dc739745b Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Mon, 17 Aug 2026 16:46:47 +0200 Subject: [PATCH 28/30] feat: add bind for deactivate tool --- src/ui/default_input_event_bindings.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ui/default_input_event_bindings.ts b/src/ui/default_input_event_bindings.ts index 3a1fc8429f..88f49af78b 100644 --- a/src/ui/default_input_event_bindings.ts +++ b/src/ui/default_input_event_bindings.ts @@ -49,6 +49,7 @@ export function getDefaultGlobalBindings() { map.set("shift+space", "toggle-layout-alternative"); map.set("backslash", "toggle-show-statistics"); map.set("control+keyp", "open-command-palette"); + map.set("escape", "deactivate-active-tool"); defaultGlobalBindings = map; } return defaultGlobalBindings; From b2e3415586a3c2397fd3e165fa98e8061a909729 Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Tue, 18 Aug 2026 17:31:32 +0200 Subject: [PATCH 29/30] chore: lint format --- src/ui/command_catalog.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/command_catalog.spec.ts b/src/ui/command_catalog.spec.ts index 0b9060b2aa..a24b66ec94 100644 --- a/src/ui/command_catalog.spec.ts +++ b/src/ui/command_catalog.spec.ts @@ -203,7 +203,7 @@ describe("CommandCatalog command sources", () => { // Falls back to a label derived from the action id. expect(entries[0].label).toBe("Embedder Action"); expect(entries[0].shortcut).toBe("q"); - expect(entries[0].source).toBe("derived") + expect(entries[0].source).toBe("derived"); } finally { catalog.dispose(); } From 05d10d4e9dab6282ed19e6c4a4e5f2fae0ced8db Mon Sep 17 00:00:00 2001 From: Sean Martin Date: Tue, 18 Aug 2026 17:41:22 +0200 Subject: [PATCH 30/30] feat: always rebuild command catalog when palette opens 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. --- src/ui/command_catalog.ts | 12 +----------- src/ui/command_palette.ts | 1 + 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/ui/command_catalog.ts b/src/ui/command_catalog.ts index 5c5bc5af0b..ace6c3d5e4 100644 --- a/src/ui/command_catalog.ts +++ b/src/ui/command_catalog.ts @@ -227,16 +227,6 @@ export function collectActionBindings( })); } -/** - * 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. - * - * `commands` is always flat: entries that belong together (e.g. the per-layer - * toggle-layer-N actions) share a `group`. It is up to the consumer - * if and how they wish to use this group. - */ export class CommandCatalog extends RefCounted { commands: readonly CommandEntry[] = []; readonly changed = new Signal(); @@ -262,7 +252,7 @@ export class CommandCatalog extends RefCounted { this.rebuild(); } - private rebuild() { + public rebuild() { const { globalToolBinder, layerManager, diff --git a/src/ui/command_palette.ts b/src/ui/command_palette.ts index 0ed7b42310..e857ac76f4 100644 --- a/src/ui/command_palette.ts +++ b/src/ui/command_palette.ts @@ -115,6 +115,7 @@ export class CommandPalette extends Overlay { resultsList.className = "neuroglancer-command-palette-results"; this.content.appendChild(resultsList); + catalog.rebuild(); this.buildRows(); searchInput.addEventListener("input", () => {