fix(core): correctly detect keybinding overrides against defaults (#355)

Author: SaKaNa-Y

packages/core/src/client/webcomponents/components/views-builtin/SettingsShortcuts.vue

@@ -3,7 +3,7 @@ import type { DevToolsCommandEntry, DevToolsCommandKeybinding } from '@vitejs/de
 import type { DocksContext } from '@vitejs/devtools-kit/client'
 import { computed, ref, watch } from 'vue'
 import { sharedStateToRef } from '../../state/docks'
-import { formatKeybinding, isMac, KNOWN_BROWSER_SHORTCUTS } from '../../state/keybindings'
+import { formatKeybinding, isKeybindingOverrideDifferentFromDefault, isMac, KNOWN_BROWSER_SHORTCUTS } from '../../state/keybindings'
 import KeybindingBadge from '../command-palette/KeybindingBadge.vue'
 import DockIcon from '../dock/DockIcon.vue'
 
@@ -55,24 +55,28 @@ function getEffectiveKeybindings(id: string): DevToolsCommandKeybinding[] {
 }
 
 function isOverridden(id: string): boolean {
-  return shortcutOverrides.value[id] !== undefined
+  return isKeybindingOverrideDifferentFromDefault(shortcutOverrides.value[id], getDefaultKeybindings(id))
 }
 
-function getDefaultKey(id: string): string | undefined {
+function getDefaultKeybindings(id: string): DevToolsCommandKeybinding[] {
   for (const cmd of commandsCtx.commands) {
     if (cmd.id === id)
-      return cmd.keybindings?.[0]?.key
+      return cmd.keybindings ?? []
     if (cmd.children) {
       const child = cmd.children.find(c => c.id === id)
       if (child)
-        return child.keybindings?.[0]?.key
+        return child.keybindings ?? []
     }
   }
+  return []
 }
 
 function clearShortcut(commandId: string) {
   commandsCtx.settings.mutate((state) => {
-    state.commandShortcuts[commandId] = []
+    if (getDefaultKeybindings(commandId).length > 0)
+      state.commandShortcuts[commandId] = []
+    else
+      delete state.commandShortcuts[commandId]
   })
 }
 
@@ -199,17 +203,15 @@ function onEditorKeyDown(e: KeyboardEvent) {
 function saveEditor() {
   if (!editorCommandId.value || !editorCanSave.value)
     return
-  const defaultKey = getDefaultKey(editorCommandId.value)
-  if (editorComposedKey.value === defaultKey) {
-    if (isOverridden(editorCommandId.value)) {
-      commandsCtx.settings.mutate((state) => {
-        delete state.commandShortcuts[editorCommandId.value!]
-      })
-    }
+  const override = [{ key: editorComposedKey.value }]
+  if (isKeybindingOverrideDifferentFromDefault(override, getDefaultKeybindings(editorCommandId.value))) {
+    commandsCtx.settings.mutate((state) => {
+      state.commandShortcuts[editorCommandId.value!] = override
+    })
   }
   else {
     commandsCtx.settings.mutate((state) => {
-      state.commandShortcuts[editorCommandId.value!] = [{ key: editorComposedKey.value }]
+      delete state.commandShortcuts[editorCommandId.value!]
     })
   }
   closeEditor()

packages/core/src/client/webcomponents/state/__tests__/keybindings.test.ts

@@ -1,6 +1,6 @@
 import type { DevToolsCommandEntry, DevToolsCommandKeybinding } from '@vitejs/devtools-kit'
 import { describe, expect, it } from 'vitest'
-import { collectAllKeybindings, formatKeybinding, KNOWN_BROWSER_SHORTCUTS, normalizeKeyEvent } from '../keybindings'
+import { areKeybindingsEqual, collectAllKeybindings, formatKeybinding, isKeybindingOverrideDifferentFromDefault, KNOWN_BROWSER_SHORTCUTS, normalizeKeyEvent } from '../keybindings'
 
 describe('formatKeybinding', () => {
   it('splits key string into parts', () => {
@@ -105,6 +105,44 @@ describe('collectAllKeybindings', () => {
   })
 })
 
+describe('areKeybindingsEqual', () => {
+  it('treats undefined and empty arrays as equal', () => {
+    expect(areKeybindingsEqual(undefined, [])).toBe(true)
+    expect(areKeybindingsEqual([], undefined)).toBe(true)
+  })
+
+  it('compares keybinding arrays by length, order, and key', () => {
+    const defaults = [{ key: 'Mod+K' }, { key: 'Alt+K' }]
+
+    expect(areKeybindingsEqual(defaults, [{ key: 'Mod+K' }, { key: 'Alt+K' }])).toBe(true)
+    expect(areKeybindingsEqual(defaults, [{ key: 'Mod+K' }])).toBe(false)
+    expect(areKeybindingsEqual(defaults, [{ key: 'Alt+K' }, { key: 'Mod+K' }])).toBe(false)
+    expect(areKeybindingsEqual(defaults, [{ key: 'Mod+K' }, { key: 'Alt+N' }])).toBe(false)
+  })
+})
+
+describe('isKeybindingOverrideDifferentFromDefault', () => {
+  it('treats a missing override as default state', () => {
+    expect(isKeybindingOverrideDifferentFromDefault(undefined, [{ key: 'Mod+K' }])).toBe(false)
+  })
+
+  it('treats undefined default and empty override as equivalent', () => {
+    expect(isKeybindingOverrideDifferentFromDefault([], undefined)).toBe(false)
+  })
+
+  it('treats empty override as different from non-empty default', () => {
+    expect(isKeybindingOverrideDifferentFromDefault([], [{ key: 'Mod+K' }])).toBe(true)
+  })
+
+  it('treats custom key as different from empty default', () => {
+    expect(isKeybindingOverrideDifferentFromDefault([{ key: 'Alt+N' }], [])).toBe(true)
+  })
+
+  it('treats matching override and defaults as default state', () => {
+    expect(isKeybindingOverrideDifferentFromDefault([{ key: 'Mod+K' }], [{ key: 'Mod+K' }])).toBe(false)
+  })
+})
+
 describe('kNOWN_BROWSER_SHORTCUTS', () => {
   it('has descriptions for all entries', () => {
     for (const [key, description] of Object.entries(KNOWN_BROWSER_SHORTCUTS)) {

packages/core/src/client/webcomponents/state/keybindings.ts

@@ -40,6 +40,23 @@ export function normalizeKeyEvent(e: KeyboardEvent): string {
   return parts.join('+')
 }
 
+export function areKeybindingsEqual(
+  left: DevToolsCommandKeybinding[] | undefined,
+  right: DevToolsCommandKeybinding[] | undefined,
+): boolean {
+  const leftBindings = left ?? []
+  const rightBindings = right ?? []
+  return leftBindings.length === rightBindings.length
+    && leftBindings.every((binding, index) => binding.key === rightBindings[index]?.key)
+}
+
+export function isKeybindingOverrideDifferentFromDefault(
+  override: DevToolsCommandKeybinding[] | undefined,
+  defaults: DevToolsCommandKeybinding[] | undefined,
+): boolean {
+  return override !== undefined && !areKeybindingsEqual(override, defaults)
+}
+
 export function collectAllKeybindings(
   commands: { value: DevToolsCommandEntry[] },
   getKeybindings: (id: string) => DevToolsCommandKeybinding[],