Keyboard cleanup (#1071)

* Delete keyboard-shortcuts-context and useKeyboardShortcut

* Fix handler re-registration

* fix
This commit is contained in:
Louis Knight-Webb
2025-10-22 12:28:20 +01:00
committed by GitHub
parent ad854895fc
commit 3a3d066071
7 changed files with 32 additions and 321 deletions

View File

@@ -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() {
</Route>
</SentryRoutes>
</div>
<ShortcutsHelp />
</SearchProvider>
</ThemeProvider>
</I18nextProvider>
@@ -208,11 +205,9 @@ function App() {
<ClickedElementsProvider>
<ProjectProvider>
<HotkeysProvider initiallyActiveScopes={['*', 'global', 'kanban']}>
<KeyboardShortcutsProvider>
<NiceModal.Provider>
<AppContent />
</NiceModal.Provider>
</KeyboardShortcutsProvider>
<NiceModal.Provider>
<AppContent />
</NiceModal.Provider>
</HotkeysProvider>
</ProjectProvider>
</ClickedElementsProvider>

View File

@@ -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<string, typeof shortcuts>
);
const formatKeys = (keys: string | string[]) => {
if (Array.isArray(keys)) {
return keys.join(' or ');
}
return keys;
};
if (!isOpen) return null;
return (
<Dialog open={isOpen} onOpenChange={setIsOpen}>
<DialogContent className="max-w-2xl max-h-[80vh] overflow-y-auto">
<DialogHeader>
<DialogTitle>Keyboard Shortcuts</DialogTitle>
</DialogHeader>
<div className="space-y-6">
{Object.entries(groupedShortcuts).map(([group, shortcuts]) => (
<div key={group}>
<h3 className="text-lg font-medium mb-3">{group}</h3>
<div className="space-y-2">
{shortcuts.map((shortcut) => (
<div
key={shortcut.id}
className="flex justify-between items-center py-1"
>
<span className="text-sm">{shortcut.description}</span>
<kbd className="px-2 py-1 bg-muted rounded text-xs font-mono">
{formatKeys(shortcut.keys)}
</kbd>
</div>
))}
</div>
</div>
))}
</div>
<div className="text-xs text-muted-foreground mt-4 pt-4 border-t">
Press <kbd className="px-1 py-0.5 bg-muted rounded text-xs">?</kbd> to
open this help dialog
</div>
</DialogContent>
</Dialog>
);
}

View File

@@ -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<KeyboardShortcutsState | null>(
null
);
interface KeyboardShortcutsProviderProps {
children: ReactNode;
}
export function KeyboardShortcutsProvider({
children,
}: KeyboardShortcutsProviderProps) {
const [shortcuts, setShortcuts] = useState<RegisteredShortcut[]>([]);
const idCounter = useRef(0);
const shortcutsRef = useRef<RegisteredShortcut[]>([]);
// 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 (
<KeyboardShortcutsContext.Provider value={value}>
{children}
</KeyboardShortcutsContext.Provider>
);
}
export function useKeyboardShortcutsRegistry(): KeyboardShortcutsState {
const context = useContext(KeyboardShortcutsContext);
if (!context) {
throw new Error(
'useKeyboardShortcutsRegistry must be used within a KeyboardShortcutsProvider'
);
}
return context;
}

View File

@@ -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';

View File

@@ -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
);
}

View File

@@ -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';

View File

@@ -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<A extends Action>(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) {