fix(hub-ui): warn about shortcut conflicts across modes (#443)

Co-authored-by: Anthony Fu <github@antfu.me>

Author: SaKaNa-Y

packages/hub-ui/src/client/components/views-builtin/SettingsShortcuts.vue

@@ -5,7 +5,7 @@ import type { ShortcutRow } from '../../state/keybindings'
 import DisplayKbd from '@antfu/design/components/Display/DisplayKbd.vue'
 import { computed, nextTick, ref, watch } from 'vue'
 import { t } from '../../i18n'
-import { filterCommandsByWhen, findCommandDeep, formatKeybinding, getShortcutRows, isKeybindingOverrideDifferentFromDefault, isMac, KNOWN_BROWSER_SHORTCUTS } from '../../state/keybindings'
+import { filterCommandsByWhen, findCommandDeep, findKeybindingConflict, formatKeybinding, getShortcutRows, isKeybindingOverrideDifferentFromDefault, isMac, KNOWN_BROWSER_SHORTCUTS } from '../../state/keybindings'
 import { useSettings } from '../../state/settings-defaults'
 import DockIcon from '../dock/DockIcon.vue'
 
@@ -157,17 +157,11 @@ const editorWarnings = computed<string[]>(() => {
     warnings.push(t('shortcuts.warnBrowser', { description: browserDescription, keys: formatKeybinding(key).join('+') }))
   }
 
-  // Conflict with other commands
+  // Hidden commands can share the same mode again after the user switches modes.
   if (editorCommandId.value) {
-    for (const row of shortcutRows.value) {
-      if (row.command.id === editorCommandId.value)
-        continue
-      const bindings = getEffectiveKeybindings(row.command.id)
-      if (bindings.some(b => b.key === key)) {
-        warnings.push(t('shortcuts.warnCommand', { title: row.command.title }))
-        break
-      }
-    }
+    const conflict = findKeybindingConflict(commandsCtx.commands, editorCommandId.value, key, getEffectiveKeybindings)
+    if (conflict)
+      warnings.push(t('shortcuts.warnCommand', { title: conflict.title }))
   }
 
   return warnings

packages/hub-ui/src/client/state/keybindings.test.ts

@@ -1,7 +1,54 @@
 import type { DevframeCommandEntry, DevframeCommandKeybinding } from '@devframes/hub'
 import type { WhenContext } from 'devframe/utils/when'
 import { describe, expect, it } from 'vitest'
-import { collectAllKeybindings, filterCommandsByWhen, findCommandDeep, getShortcutRows, walkCommands } from './keybindings'
+import { collectAllKeybindings, filterCommandsByWhen, findCommandDeep, findKeybindingConflict, getShortcutRows, walkCommands } from './keybindings'
+
+describe('findKeybindingConflict', () => {
+  const commands: DevframeCommandEntry[] = [
+    { id: 'settings', title: 'Open Settings', source: 'client' },
+    {
+      id: 'dock-mode',
+      title: 'Dock Mode',
+      source: 'client',
+      when: '!popupOpen',
+      children: [
+        { id: 'float', title: 'Float Mode', source: 'client', when: '!popupOpen' },
+      ],
+    },
+    { id: 'hidden', title: 'Hide', source: 'client', when: 'clientType == embedded' },
+    { id: 'disabled', title: 'Disabled', source: 'client', allowShortcuts: false },
+  ]
+
+  it.each<[string, Pick<WhenContext, 'popupOpen' | 'clientType'>]>([
+    ['float', { popupOpen: true, clientType: 'embedded' }],
+    ['hidden', { popupOpen: false, clientType: 'standalone' }],
+  ])('warns about %s even when the current mode hides it', (id, context) => {
+    const visibleRows = getShortcutRows(filterCommandsByWhen(commands, {
+      ...context,
+      dockOpen: true,
+      paletteOpen: false,
+      dockSelectedId: '',
+    }))
+    expect(visibleRows.some(row => row.command.id === id)).toBe(false)
+    const getKeybindings = (commandId: string) => commandId === id ? [{ key: 'Mod+Shift+J' }] : []
+    expect(findKeybindingConflict(commands, 'settings', 'Mod+Shift+J', getKeybindings)?.id).toBe(id)
+  })
+
+  it('ignores the edited command and commands that disallow shortcuts', () => {
+    const getKeybindings = (id: string) => ['settings', 'disabled'].includes(id) ? [{ key: 'Mod+J' }] : []
+    expect(findKeybindingConflict(commands, 'settings', 'Mod+J', getKeybindings)).toBeUndefined()
+  })
+
+  it('uses effective bindings, including cleared defaults and multiple bindings', () => {
+    const commands: DevframeCommandEntry[] = [
+      { id: 'cleared', title: 'Cleared', source: 'client', keybindings: [{ key: 'Mod+J' }] },
+      { id: 'overridden', title: 'Overridden', source: 'client', keybindings: [{ key: 'Mod+K' }] },
+    ]
+    const getKeybindings = (id: string) => id === 'overridden' ? [{ key: 'Mod+L' }, { key: 'Mod+J' }] : []
+    expect(findKeybindingConflict(commands, 'settings', 'Mod+J', getKeybindings)?.id).toBe('overridden')
+    expect(findKeybindingConflict(commands, 'settings', 'Mod+K', getKeybindings)).toBeUndefined()
+  })
+})
 
 describe('getShortcutRows', () => {
   it('omits opted-out commands while retaining their bindable descendants and palette-hidden commands', () => {

packages/hub-ui/src/client/state/keybindings.ts

@@ -133,6 +133,19 @@ export function getShortcutRows(commands: DevframeCommandEntry[]): ShortcutRow[]
   return rows
 }
 
+/** Check all bindable commands, including those hidden in the current mode. */
+export function findKeybindingConflict(
+  commands: DevframeCommandEntry[],
+  commandId: string,
+  key: string,
+  getKeybindings: (id: string) => DevframeCommandKeybinding[],
+): DevframeCommandEntry | undefined {
+  return getShortcutRows(commands).find(({ command }) =>
+    command.id !== commandId
+    && getKeybindings(command.id).some(binding => binding.key === key),
+  )?.command
+}
+
 /**
  * Drop the commands whose `when` clause does not hold in the current context,
  * descendants included at every depth; `when` controls palette visibility at