fix(use-keyboard-shortcuts): enforce symmetric modifier matching
Modifiers (shift, alt, meta) are now checked in both directions: when not required, the physical key must NOT be pressed either. Before: Cmd+K shortcut would fire on Cmd+Shift+K or Cmd+Alt+K. After: exact modifier combination is enforced. 4 regression tests added.
This commit is contained in:
parent
31cf7c0c6f
commit
6f4957d821
@ -197,4 +197,44 @@ describe('useKeyboardShortcuts', () => {
|
|||||||
|
|
||||||
expect(handler).toHaveBeenCalledTimes(1);
|
expect(handler).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not fire Cmd+K shortcut when Shift is also pressed', () => {
|
||||||
|
const handler = vi.fn();
|
||||||
|
const shortcuts: ShortcutDef[] = [{ key: 'k', meta: true, handler }];
|
||||||
|
|
||||||
|
renderHook(() => useKeyboardShortcuts(shortcuts));
|
||||||
|
fireKey('k', { metaKey: true, shiftKey: true });
|
||||||
|
|
||||||
|
expect(handler).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not fire Cmd+K shortcut when Alt is also pressed', () => {
|
||||||
|
const handler = vi.fn();
|
||||||
|
const shortcuts: ShortcutDef[] = [{ key: 'k', meta: true, handler }];
|
||||||
|
|
||||||
|
renderHook(() => useKeyboardShortcuts(shortcuts));
|
||||||
|
fireKey('k', { metaKey: true, altKey: true });
|
||||||
|
|
||||||
|
expect(handler).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not fire bare-key shortcut when meta is pressed', () => {
|
||||||
|
const handler = vi.fn();
|
||||||
|
const shortcuts: ShortcutDef[] = [{ key: 'Escape', handler }];
|
||||||
|
|
||||||
|
renderHook(() => useKeyboardShortcuts(shortcuts));
|
||||||
|
fireKey('Escape', { metaKey: true });
|
||||||
|
|
||||||
|
expect(handler).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not fire bare-key shortcut when shift is pressed', () => {
|
||||||
|
const handler = vi.fn();
|
||||||
|
const shortcuts: ShortcutDef[] = [{ key: '/', handler }];
|
||||||
|
|
||||||
|
renderHook(() => useKeyboardShortcuts(shortcuts));
|
||||||
|
fireKey('/', { shiftKey: true });
|
||||||
|
|
||||||
|
expect(handler).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -35,9 +35,9 @@ export function useKeyboardShortcuts(shortcuts: ShortcutDef[]): void {
|
|||||||
const inInput = isInputElement(e.target);
|
const inInput = isInputElement(e.target);
|
||||||
|
|
||||||
for (const shortcut of shortcuts) {
|
for (const shortcut of shortcuts) {
|
||||||
const metaMatch = shortcut.meta ? e.metaKey || e.ctrlKey : true;
|
const metaMatch = shortcut.meta ? e.metaKey || e.ctrlKey : !(e.metaKey || e.ctrlKey);
|
||||||
const shiftMatch = shortcut.shift ? e.shiftKey : !e.shiftKey || !shortcut.meta; // only enforce no-shift when meta is set
|
const shiftMatch = shortcut.shift ? e.shiftKey : !e.shiftKey;
|
||||||
const altMatch = shortcut.alt ? e.altKey : true;
|
const altMatch = shortcut.alt ? e.altKey : !e.altKey;
|
||||||
|
|
||||||
// Normalize key comparison (case-insensitive for letters)
|
// Normalize key comparison (case-insensitive for letters)
|
||||||
const keyMatch = e.key.toLowerCase() === shortcut.key.toLowerCase();
|
const keyMatch = e.key.toLowerCase() === shortcut.key.toLowerCase();
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user