Skip to content

Commit c12d925

Browse files
committed
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.
1 parent ee667f4 commit c12d925

3 files changed

Lines changed: 72 additions & 15 deletions

File tree

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

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { DevToolsCommandEntry, DevToolsCommandKeybinding } from '@vitejs/de
33
import type { DocksContext } from '@vitejs/devtools-kit/client'
44
import { computed, ref, watch } from 'vue'
55
import { sharedStateToRef } from '../../state/docks'
6-
import { formatKeybinding, isMac, KNOWN_BROWSER_SHORTCUTS } from '../../state/keybindings'
6+
import { formatKeybinding, isKeybindingOverrideDifferentFromDefault, isMac, KNOWN_BROWSER_SHORTCUTS } from '../../state/keybindings'
77
import KeybindingBadge from '../command-palette/KeybindingBadge.vue'
88
import DockIcon from '../dock/DockIcon.vue'
99
@@ -55,24 +55,28 @@ function getEffectiveKeybindings(id: string): DevToolsCommandKeybinding[] {
5555
}
5656
5757
function isOverridden(id: string): boolean {
58-
return shortcutOverrides.value[id] !== undefined
58+
return isKeybindingOverrideDifferentFromDefault(shortcutOverrides.value[id], getDefaultKeybindings(id))
5959
}
6060
61-
function getDefaultKey(id: string): string | undefined {
61+
function getDefaultKeybindings(id: string): DevToolsCommandKeybinding[] {
6262
for (const cmd of commandsCtx.commands) {
6363
if (cmd.id === id)
64-
return cmd.keybindings?.[0]?.key
64+
return cmd.keybindings ?? []
6565
if (cmd.children) {
6666
const child = cmd.children.find(c => c.id === id)
6767
if (child)
68-
return child.keybindings?.[0]?.key
68+
return child.keybindings ?? []
6969
}
7070
}
71+
return []
7172
}
7273
7374
function clearShortcut(commandId: string) {
7475
commandsCtx.settings.mutate((state) => {
75-
state.commandShortcuts[commandId] = []
76+
if (getDefaultKeybindings(commandId).length > 0)
77+
state.commandShortcuts[commandId] = []
78+
else
79+
delete state.commandShortcuts[commandId]
7680
})
7781
}
7882
@@ -199,17 +203,15 @@ function onEditorKeyDown(e: KeyboardEvent) {
199203
function saveEditor() {
200204
if (!editorCommandId.value || !editorCanSave.value)
201205
return
202-
const defaultKey = getDefaultKey(editorCommandId.value)
203-
if (editorComposedKey.value === defaultKey) {
204-
if (isOverridden(editorCommandId.value)) {
205-
commandsCtx.settings.mutate((state) => {
206-
delete state.commandShortcuts[editorCommandId.value!]
207-
})
208-
}
206+
const override = [{ key: editorComposedKey.value }]
207+
if (isKeybindingOverrideDifferentFromDefault(override, getDefaultKeybindings(editorCommandId.value))) {
208+
commandsCtx.settings.mutate((state) => {
209+
state.commandShortcuts[editorCommandId.value!] = override
210+
})
209211
}
210212
else {
211213
commandsCtx.settings.mutate((state) => {
212-
state.commandShortcuts[editorCommandId.value!] = [{ key: editorComposedKey.value }]
214+
delete state.commandShortcuts[editorCommandId.value!]
213215
})
214216
}
215217
closeEditor()

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { DevToolsCommandEntry, DevToolsCommandKeybinding } from '@vitejs/devtools-kit'
22
import { describe, expect, it } from 'vitest'
3-
import { collectAllKeybindings, formatKeybinding, KNOWN_BROWSER_SHORTCUTS, normalizeKeyEvent } from '../keybindings'
3+
import { areKeybindingsEqual, collectAllKeybindings, formatKeybinding, isKeybindingOverrideDifferentFromDefault, KNOWN_BROWSER_SHORTCUTS, normalizeKeyEvent } from '../keybindings'
44

55
describe('formatKeybinding', () => {
66
it('splits key string into parts', () => {
@@ -105,6 +105,44 @@ describe('collectAllKeybindings', () => {
105105
})
106106
})
107107

108+
describe('areKeybindingsEqual', () => {
109+
it('treats undefined and empty arrays as equal', () => {
110+
expect(areKeybindingsEqual(undefined, [])).toBe(true)
111+
expect(areKeybindingsEqual([], undefined)).toBe(true)
112+
})
113+
114+
it('compares keybinding arrays by length, order, and key', () => {
115+
const defaults = [{ key: 'Mod+K' }, { key: 'Alt+K' }]
116+
117+
expect(areKeybindingsEqual(defaults, [{ key: 'Mod+K' }, { key: 'Alt+K' }])).toBe(true)
118+
expect(areKeybindingsEqual(defaults, [{ key: 'Mod+K' }])).toBe(false)
119+
expect(areKeybindingsEqual(defaults, [{ key: 'Alt+K' }, { key: 'Mod+K' }])).toBe(false)
120+
expect(areKeybindingsEqual(defaults, [{ key: 'Mod+K' }, { key: 'Alt+N' }])).toBe(false)
121+
})
122+
})
123+
124+
describe('isKeybindingOverrideDifferentFromDefault', () => {
125+
it('treats a missing override as default state', () => {
126+
expect(isKeybindingOverrideDifferentFromDefault(undefined, [{ key: 'Mod+K' }])).toBe(false)
127+
})
128+
129+
it('treats undefined default and empty override as equivalent', () => {
130+
expect(isKeybindingOverrideDifferentFromDefault([], undefined)).toBe(false)
131+
})
132+
133+
it('treats empty override as different from non-empty default', () => {
134+
expect(isKeybindingOverrideDifferentFromDefault([], [{ key: 'Mod+K' }])).toBe(true)
135+
})
136+
137+
it('treats custom key as different from empty default', () => {
138+
expect(isKeybindingOverrideDifferentFromDefault([{ key: 'Alt+N' }], [])).toBe(true)
139+
})
140+
141+
it('treats matching override and defaults as default state', () => {
142+
expect(isKeybindingOverrideDifferentFromDefault([{ key: 'Mod+K' }], [{ key: 'Mod+K' }])).toBe(false)
143+
})
144+
})
145+
108146
describe('kNOWN_BROWSER_SHORTCUTS', () => {
109147
it('has descriptions for all entries', () => {
110148
for (const [key, description] of Object.entries(KNOWN_BROWSER_SHORTCUTS)) {

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,23 @@ export function normalizeKeyEvent(e: KeyboardEvent): string {
4040
return parts.join('+')
4141
}
4242

43+
export function areKeybindingsEqual(
44+
left: DevToolsCommandKeybinding[] | undefined,
45+
right: DevToolsCommandKeybinding[] | undefined,
46+
): boolean {
47+
const leftBindings = left ?? []
48+
const rightBindings = right ?? []
49+
return leftBindings.length === rightBindings.length
50+
&& leftBindings.every((binding, index) => binding.key === rightBindings[index]?.key)
51+
}
52+
53+
export function isKeybindingOverrideDifferentFromDefault(
54+
override: DevToolsCommandKeybinding[] | undefined,
55+
defaults: DevToolsCommandKeybinding[] | undefined,
56+
): boolean {
57+
return override !== undefined && !areKeybindingsEqual(override, defaults)
58+
}
59+
4360
export function collectAllKeybindings(
4461
commands: { value: DevToolsCommandEntry[] },
4562
getKeybindings: (id: string) => DevToolsCommandKeybinding[],

0 commit comments

Comments
 (0)