From 3a3d066071b19f0b90d24264a092c2e23b614574 Mon Sep 17 00:00:00 2001 From: Louis Knight-Webb Date: Wed, 22 Oct 2025 12:28:20 +0100 Subject: [PATCH] Keyboard cleanup (#1071) * Delete keyboard-shortcuts-context and useKeyboardShortcut * Fix handler re-registration * fix --- frontend/src/App.tsx | 11 +- frontend/src/components/shortcuts-help.tsx | 72 ---------- .../contexts/keyboard-shortcuts-context.tsx | 126 ------------------ frontend/src/hooks/index.ts | 1 - frontend/src/hooks/useKeyboardShortcut.ts | 85 ------------ frontend/src/keyboard/index.ts | 3 - frontend/src/keyboard/useSemanticKey.ts | 55 ++++---- 7 files changed, 32 insertions(+), 321 deletions(-) delete mode 100644 frontend/src/components/shortcuts-help.tsx delete mode 100644 frontend/src/contexts/keyboard-shortcuts-context.tsx delete mode 100644 frontend/src/hooks/useKeyboardShortcut.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 7bbd6b3d..5197bc7d 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -20,9 +20,7 @@ import { } from '@/components/config-provider'; import { ThemeProvider } from '@/components/theme-provider'; import { SearchProvider } from '@/contexts/search-context'; -import { KeyboardShortcutsProvider } from '@/contexts/keyboard-shortcuts-context'; -import { ShortcutsHelp } from '@/components/shortcuts-help'; import { HotkeysProvider } from 'react-hotkeys-hook'; import { ProjectProvider } from '@/contexts/project-context'; @@ -194,7 +192,6 @@ function AppContent() { - @@ -208,11 +205,9 @@ function App() { - - - - - + + + diff --git a/frontend/src/components/shortcuts-help.tsx b/frontend/src/components/shortcuts-help.tsx deleted file mode 100644 index b9ab4a79..00000000 --- a/frontend/src/components/shortcuts-help.tsx +++ /dev/null @@ -1,72 +0,0 @@ -import { useState } from 'react'; -import { useKeyboardShortcutsRegistry } from '@/contexts/keyboard-shortcuts-context'; -import { useKeyShowHelp, Scope } from '@/keyboard'; -import { - Dialog, - DialogContent, - DialogHeader, - DialogTitle, -} from '@/components/ui/dialog'; - -export function ShortcutsHelp() { - const [isOpen, setIsOpen] = useState(false); - const { shortcuts } = useKeyboardShortcutsRegistry(); - - // Global shortcut to open help using semantic hook - useKeyShowHelp(() => setIsOpen(true), { scope: Scope.GLOBAL }); - - const groupedShortcuts = shortcuts.reduce( - (acc, shortcut) => { - const group = shortcut.group || 'Other'; - if (!acc[group]) acc[group] = []; - acc[group].push(shortcut); - return acc; - }, - {} as Record - ); - - const formatKeys = (keys: string | string[]) => { - if (Array.isArray(keys)) { - return keys.join(' or '); - } - return keys; - }; - - if (!isOpen) return null; - - return ( - - - - Keyboard Shortcuts - - -
- {Object.entries(groupedShortcuts).map(([group, shortcuts]) => ( -
-

{group}

-
- {shortcuts.map((shortcut) => ( -
- {shortcut.description} - - {formatKeys(shortcut.keys)} - -
- ))} -
-
- ))} -
- -
- Press ? to - open this help dialog -
- - - ); -} diff --git a/frontend/src/contexts/keyboard-shortcuts-context.tsx b/frontend/src/contexts/keyboard-shortcuts-context.tsx deleted file mode 100644 index 078c9386..00000000 --- a/frontend/src/contexts/keyboard-shortcuts-context.tsx +++ /dev/null @@ -1,126 +0,0 @@ -import { - createContext, - useContext, - useState, - useRef, - ReactNode, - useCallback, - useEffect, -} from 'react'; - -export interface ShortcutConfig { - keys: string | string[]; // 'c' or ['cmd+k', 'ctrl+k'] - callback: (e: KeyboardEvent) => void; - description: string; // For help documentation - group?: string; // 'Dialog', 'Kanban', 'Global' - scope?: string; // 'global', 'kanban', 'dialog' - when?: boolean | (() => boolean); // Dynamic enabling -} - -export interface RegisteredShortcut extends ShortcutConfig { - id: string; // Auto-generated unique ID -} - -interface KeyboardShortcutsState { - shortcuts: RegisteredShortcut[]; - register: (config: ShortcutConfig) => () => void; // Returns unregister function - getShortcutsByScope: (scope?: string) => RegisteredShortcut[]; - getShortcutsByGroup: (group?: string) => RegisteredShortcut[]; -} - -const KeyboardShortcutsContext = createContext( - null -); - -interface KeyboardShortcutsProviderProps { - children: ReactNode; -} - -export function KeyboardShortcutsProvider({ - children, -}: KeyboardShortcutsProviderProps) { - const [shortcuts, setShortcuts] = useState([]); - const idCounter = useRef(0); - const shortcutsRef = useRef([]); - - // Keep ref in sync with state - useEffect(() => { - shortcutsRef.current = shortcuts; - }, [shortcuts]); - - const register = useCallback( - (config: ShortcutConfig) => { - const id = `shortcut-${idCounter.current++}`; - const registeredShortcut: RegisteredShortcut = { ...config, id }; - - // Development-only conflict detection using ref to avoid dependency cycle - if (import.meta.env.DEV) { - const conflictingShortcut = shortcutsRef.current.find((existing) => { - const sameScope = - (existing.scope || 'global') === (config.scope || 'global'); - const sameKeys = - JSON.stringify(existing.keys) === JSON.stringify(config.keys); - return sameScope && sameKeys; - }); - - if (conflictingShortcut) { - console.warn( - `Keyboard shortcut conflict detected!`, - `\nExisting: ${conflictingShortcut.description} (${conflictingShortcut.keys})`, - `\nNew: ${config.description} (${config.keys})`, - `\nScope: ${config.scope || 'global'}` - ); - } - } - - setShortcuts((prev) => [...prev, registeredShortcut]); - - // Return cleanup function - return () => { - setShortcuts((prev) => prev.filter((shortcut) => shortcut.id !== id)); - }; - }, - [] // Empty dependencies - function stays stable - ); - - const getShortcutsByScope = useCallback( - (scope?: string) => { - const targetScope = scope || 'global'; - return shortcuts.filter( - (shortcut) => (shortcut.scope || 'global') === targetScope - ); - }, - [shortcuts] - ); - - const getShortcutsByGroup = useCallback( - (group?: string) => { - if (!group) return shortcuts; - return shortcuts.filter((shortcut) => shortcut.group === group); - }, - [shortcuts] - ); - - const value: KeyboardShortcutsState = { - shortcuts, - register, - getShortcutsByScope, - getShortcutsByGroup, - }; - - return ( - - {children} - - ); -} - -export function useKeyboardShortcutsRegistry(): KeyboardShortcutsState { - const context = useContext(KeyboardShortcutsContext); - if (!context) { - throw new Error( - 'useKeyboardShortcutsRegistry must be used within a KeyboardShortcutsProvider' - ); - } - return context; -} diff --git a/frontend/src/hooks/index.ts b/frontend/src/hooks/index.ts index d2168577..b0c004b9 100644 --- a/frontend/src/hooks/index.ts +++ b/frontend/src/hooks/index.ts @@ -6,6 +6,5 @@ export { useRebase } from './useRebase'; export { useChangeTargetBranch } from './useChangeTargetBranch'; export { useMerge } from './useMerge'; export { usePush } from './usePush'; -export { useKeyboardShortcut } from './useKeyboardShortcut'; export { useAttemptConflicts } from './useAttemptConflicts'; export { useNavigateWithSearch } from './useNavigateWithSearch'; diff --git a/frontend/src/hooks/useKeyboardShortcut.ts b/frontend/src/hooks/useKeyboardShortcut.ts deleted file mode 100644 index ae29d3b7..00000000 --- a/frontend/src/hooks/useKeyboardShortcut.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { useEffect, useRef } from 'react'; -import { useHotkeys } from 'react-hotkeys-hook'; -import { - useKeyboardShortcutsRegistry, - type ShortcutConfig, -} from '@/contexts/keyboard-shortcuts-context'; -import type { EnableOnFormTags } from '@/keyboard/types'; - -export interface KeyboardShortcutOptions { - enableOnContentEditable?: boolean; - enableOnFormTags?: EnableOnFormTags; - preventDefault?: boolean; -} - -export function useKeyboardShortcut( - config: ShortcutConfig, - options: KeyboardShortcutOptions = {} -): void { - const { register } = useKeyboardShortcutsRegistry(); - const unregisterRef = useRef<(() => void) | null>(null); - - const { keys, callback, when = true, description, group, scope } = config; - const { - enableOnContentEditable = false, - enableOnFormTags, - preventDefault = false, - } = options; - - // Keep latest callback/when without forcing re-register - const callbackRef = useRef(callback); - useEffect(() => { - callbackRef.current = callback; - }, [callback]); - - const whenRef = useRef(when); - useEffect(() => { - whenRef.current = when; - }, [when]); - - // Register once per identity fields (no direct 'config' usage here) - useEffect(() => { - const unregister = register({ - keys, - description, - group, - scope, - // delegate to latest refs - callback: (e: KeyboardEvent) => callbackRef.current?.(e as KeyboardEvent), - when: () => { - const w = whenRef.current; - return typeof w === 'function' ? !!w() : !!w; - }, - }); - unregisterRef.current = unregister; - - return () => { - unregisterRef.current?.(); - unregisterRef.current = null; - }; - }, [register, keys, description, group, scope]); - - // Bind the actual keyboard handling - useHotkeys( - keys, - (event) => { - // Skip if IME composition is in progress (e.g., Japanese, Chinese, Korean input) - // This prevents shortcuts from firing when user is converting text with Enter - if (event.isComposing) { - return; - } - - const w = whenRef.current; - const enabled = typeof w === 'function' ? !!w() : !!w; - if (enabled) callbackRef.current?.(event as KeyboardEvent); - }, - { - enabled: true, // we gate inside handler via whenRef - enableOnContentEditable, - enableOnFormTags, - preventDefault, - scopes: scope ? [scope] : ['*'], - }, - [keys, scope, enableOnContentEditable, enableOnFormTags, preventDefault] // handler uses refs; only rebinding when identity changes - ); -} diff --git a/frontend/src/keyboard/index.ts b/frontend/src/keyboard/index.ts index e8ed89b1..e6226de7 100644 --- a/frontend/src/keyboard/index.ts +++ b/frontend/src/keyboard/index.ts @@ -1,6 +1,3 @@ // Export all semantic keyboard hooks export * from './hooks'; export * from './registry'; - -// Re-export the raw hook for edge cases -export { useKeyboardShortcut } from '@/hooks/useKeyboardShortcut'; diff --git a/frontend/src/keyboard/useSemanticKey.ts b/frontend/src/keyboard/useSemanticKey.ts index eeab8190..fd94f7ed 100644 --- a/frontend/src/keyboard/useSemanticKey.ts +++ b/frontend/src/keyboard/useSemanticKey.ts @@ -1,10 +1,7 @@ import { useMemo } from 'react'; -import { - useKeyboardShortcut, - type KeyboardShortcutOptions, -} from '@/hooks/useKeyboardShortcut'; import type { EnableOnFormTags } from './types'; -import { Action, Scope, getKeysFor, getBindingFor } from './registry'; +import { Action, Scope, getKeysFor } from './registry'; +import { useHotkeys } from 'react-hotkeys-hook'; export interface SemanticKeyOptions { scope?: Scope; @@ -40,29 +37,35 @@ export function createSemanticHook(action: A) { // Memoize to get stable array references and prevent unnecessary re-registrations const keys = useMemo(() => getKeysFor(action, scope), [action, scope]); - const binding = useMemo( - () => getBindingFor(action, scope), - [action, scope] - ); + useHotkeys( + keys, + (event) => { + // Skip if IME composition is in progress (e.g., Japanese, Chinese, Korean input) + // This prevents shortcuts from firing when user is converting text with Enter + if (event.isComposing) { + return; + } - const keyboardShortcutOptions: KeyboardShortcutOptions = {}; - if (enableOnContentEditable !== undefined) - keyboardShortcutOptions.enableOnContentEditable = enableOnContentEditable; - if (enableOnFormTags !== undefined) - keyboardShortcutOptions.enableOnFormTags = enableOnFormTags; - if (preventDefault !== undefined) - keyboardShortcutOptions.preventDefault = preventDefault; - - useKeyboardShortcut( - { - keys: keys.length === 1 ? keys[0] : keys, - callback: keys.length === 0 ? () => {} : handler, - description: binding?.description || `${action} action`, - group: binding?.group || 'Actions', - scope: scope || Scope.GLOBAL, - when: keys.length > 0 && isEnabled, + if (isEnabled) { + handler(event); + } }, - keyboardShortcutOptions + { + enabled, + enableOnContentEditable, + enableOnFormTags, + preventDefault, + scopes: scope ? [scope] : ['*'], + }, + [ + keys, + scope, + enableOnContentEditable, + enableOnFormTags, + preventDefault, + handler, + isEnabled, + ] ); if (keys.length === 0) {