fix(keyboard): stop the NiiVue tab from capturing keys and double-applying c/v (#223, #224)

.changeset/keyboard-shortcut-conflicts.md

@@ -0,0 +1,13 @@
+---
+'@niivue/react': patch
+'niivue': patch
+'@niivue/pwa': patch
+'@niivue/streamlit': patch
+'@niivue/jupyter': patch
+'@niivue/tauri': patch
+---
+
+Fix two keyboard-handling conflicts.
+
+- The VS Code shortcut keybindings (1-5, r, i, b, x, etc.) now require keyboard focus to be in the viewer rather than only requiring the NiiVue tab to be active. They no longer swallow keystrokes meant for the Quick Open box or command palette (#223).
+- NiiVue's built-in `c` (clip plane) and `v` (view mode) hotkeys are disabled so the app's own handler is the single source of truth. The focused canvas is no longer acted on twice (#224). Note: the menu-less unstyled Streamlit embed has no app keyboard handler, so it loses NiiVue's built-in `c`/`v` hotkeys.

apps/pwa/tests/keyboard-shortcuts.spec.ts

@@ -28,6 +28,8 @@ async function getDebugValue(page, request) {
         return appProps.hideUI.value;
       case 'getCrosshairPos':
         return appProps.nvArray.value[0]?.scene?.crosshairPos;
+      case 'getClipPlaneIndex':
+        return appProps.nvArray.value[0]?.currentClipPlaneIndex;
       case 'getPan':
         return appProps.nvArray.value[0]?.scene?.pan2Dxyzmm;
       default:
@@ -59,6 +61,19 @@ test.describe('Keyboard Shortcuts', () => {
     expect(newSliceType).toBeLessThanOrEqual(4);
   });
 
+  test('should cycle the clip plane exactly once per "c" press', async ({ page }) => {
+    // Regression test for #224. The app's keyboard handler advances the clip
+    // plane for every selected canvas on keydown. NiiVue's own canvas handler
+    // used to advance it a second time for the focused canvas on keyup, so a
+    // single "c" landed on index 2 instead of 1. With NiiVue's clipPlaneHotKey
+    // disabled the app is the only handler, so one press == one step.
+    const canvas = page.locator('canvas').first();
+    await canvas.click();
+
+    await page.keyboard.press('c');
+    await poll(page, 'getClipPlaneIndex').toBe(1);
+  });
+
   test('should change view to Axial with "1"', async ({ page }) => {
     const canvas = page.locator('canvas').first();
     await canvas.click();

apps/vscode/package.json

@@ -172,95 +172,95 @@
       {
         "command": "niivue.viewAxial",
         "key": "1",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.viewSagittal",
         "key": "2",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.viewCoronal",
         "key": "3",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.viewRender",
         "key": "4",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.viewMultiplanar",
         "key": "5",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.resetView",
         "key": "r",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.toggleInterpolation",
         "key": "i",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.toggleColorbar",
         "key": "b",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.toggleRadiological",
         "key": "x",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.toggleCrosshair",
         "key": "m",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.toggleZoomMode",
         "key": "z",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.addImage",
         "key": "ctrl+shift+o",
         "mac": "cmd+shift+o",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.addOverlay",
         "key": "ctrl+l",
         "mac": "cmd+l",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.colorscale",
         "key": "s",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.hideUI",
         "key": "u",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.showHeader",
         "key": "ctrl+h",
         "mac": "cmd+shift+h",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.crosshairSuperior",
         "key": "shift+u",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       },
       {
         "command": "niivue.crosshairInferior",
         "key": "shift+d",
-        "when": "activeCustomEditorId == niiVue.default"
+        "when": "activeCustomEditorId == niiVue.default && !inputFocus"
       }
     ],
     "customEditors": [

apps/vscode/test/keybindings.test.ts

@@ -0,0 +1,44 @@
+import { readFileSync } from 'node:fs'
+import path from 'node:path'
+import { fileURLToPath } from 'node:url'
+import { describe, expect, it } from 'vitest'
+
+const dir = path.dirname(fileURLToPath(import.meta.url))
+const pkg = JSON.parse(readFileSync(path.resolve(dir, '../package.json'), 'utf8'))
+
+type Keybinding = { command: string; key: string; mac?: string; when?: string }
+const keybindings: Keybinding[] = pkg.contributes?.keybindings ?? []
+
+describe('contributed keybindings', () => {
+  it('contributes the niivue shortcut keybindings', () => {
+    expect(keybindings.length).toBeGreaterThan(0)
+    expect(keybindings.every((k) => k.command.startsWith('niivue.'))).toBe(true)
+  })
+
+  // Regression test for niivue/niivue-vscode#223: the niivue commands are
+  // no-ops (the webview's own keydown listener does the work), so a binding
+  // that stays active while a VS Code input box is focused merely *swallows*
+  // the key. Without `!inputFocus`, single-key bindings like "1" or "s" eat
+  // keystrokes meant for Quick Open / the command palette ("the top box").
+  it('gates every keybinding behind the editor having focus (not an input box)', () => {
+    for (const kb of keybindings) {
+      expect(kb.when, `keybinding for ${kb.command} must declare a when-clause`).toBeTruthy()
+      expect(
+        kb.when,
+        `keybinding for ${kb.command} ("${kb.key}") must require the niivue editor`,
+      ).toContain('activeCustomEditorId == niiVue.default')
+      expect(
+        kb.when,
+        `keybinding for ${kb.command} ("${kb.key}") must be disabled while a VS Code input is focused (#223)`,
+      ).toContain('!inputFocus')
+    }
+  })
+
+  // The bare printable keys are the ones that actually break typing in the
+  // top box, so guard them explicitly in case the list grows later.
+  it('guards the single printable-key shortcuts in particular', () => {
+    const printable = keybindings.filter((k) => /^[a-z0-9]$/i.test(k.key))
+    expect(printable.length).toBeGreaterThan(0)
+    expect(printable.every((k) => k.when?.includes('!inputFocus'))).toBe(true)
+  })
+})

packages/niivue-react/src/events.ts

@@ -449,6 +449,14 @@ function growNvArrayBy(nvArray: Signal<Niivue[]>, n: number) {
       isResizeCanvas: true,
       dragMode: 1, // contrast
       dragAndDropEnabled: false, // handled by app (Volume component)
+      // 'c' (cycle clip plane) and 'v' (cycle view mode) are handled by the
+      // app's useKeyboardShortcuts hook, which broadcasts to every selected
+      // canvas. Disabling NiiVue's own canvas-level handlers for these keys
+      // (an empty hotkey never matches an event's KeyboardEvent.code) keeps the
+      // app as the single source of truth, so the focused canvas is no longer
+      // acted on a second time on key release. See niivue/niivue-vscode#224.
+      clipPlaneHotKey: '',
+      viewModeHotKey: '',
     })
     nv.key = Math.random()
     nvArray.value = [...nvArray.value, nv]