From c12d92534e30bfe61d1b7e7a65df12ebd01c19b2 Mon Sep 17 00:00:00 2001 From: yu859 <15715093608@163.com> Date: Wed, 20 May 2026 12:00:23 +0800 Subject: [PATCH] fix(core): correctly detect keybinding overrides against defaults Previously, isOverridden() only checked if an override entry existed, and saveEditor() compared only the first key string. This caused stale overrides to persist and incorrect override indicators in the UI. Introduce areKeybindingsEqual() and isKeybindingOverrideDifferentFromDefault() helpers to properly compare full keybinding arrays, and clean up overrides that match defaults instead of storing redundant entries. --- .../views-builtin/SettingsShortcuts.vue | 30 +++++++------- .../state/__tests__/keybindings.test.ts | 40 ++++++++++++++++++- .../client/webcomponents/state/keybindings.ts | 17 ++++++++ 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/packages/core/src/client/webcomponents/components/views-builtin/SettingsShortcuts.vue b/packages/core/src/client/webcomponents/components/views-builtin/SettingsShortcuts.vue index 9e83df23..78ad1867 100644 --- a/packages/core/src/client/webcomponents/components/views-builtin/SettingsShortcuts.vue +++ b/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() diff --git a/packages/core/src/client/webcomponents/state/__tests__/keybindings.test.ts b/packages/core/src/client/webcomponents/state/__tests__/keybindings.test.ts index b562ec79..df037914 100644 --- a/packages/core/src/client/webcomponents/state/__tests__/keybindings.test.ts +++ b/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)) { diff --git a/packages/core/src/client/webcomponents/state/keybindings.ts b/packages/core/src/client/webcomponents/state/keybindings.ts index 48c0fb12..c0fa1156 100644 --- a/packages/core/src/client/webcomponents/state/keybindings.ts +++ b/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[],