Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions static/app/components/core/drawer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ export function GlobalDrawer({children}: any) {
useHotkeys([
{
match: 'Escape',
enabled: isDrawerOpen,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big refactoring win! Added an enabled option to useHotkeys because the migration to key uncovered a latent bug here that we couldn't detect before.

33452cf migrated the hook from event.keyCode to event.key. GlobalDrawer had been registering an unconditional useHotkeys for Escape—this previously failed silently in tests because userEvent.keyboard('{Escape}') doesn't set keyCode in jsdom, but now it fires (and calls preventDefault) on every Escape—even when no drawer was open. The modal's escape handler bails on e.defaultPrevented (added in PR #109556 to let react-aria child overlays absorb Escape), so the global modal stopped closing entirely.

callback: () => {
handleClose();
},
Expand Down
16 changes: 16 additions & 0 deletions static/app/components/core/hotkey/hotkey.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,22 @@ function MyComponent() {
}
```

Matching is layout-aware: an AZERTY user pressing the key labeled `K` fires `command+k`, and shortcuts like `command+shift+1` continue to work regardless of how `Shift+1` produces a character on the user's layout.

### Conditional hotkeys

Use `enabled` to gate a hotkey on state without rebuilding the array. While `enabled` is `false` the callback never fires and `preventDefault` is never called, so the key reaches other listeners unchanged.

```jsx
useHotkeys([
{
match: 'Escape',
enabled: isPanelOpen,
callback: closePanel,
},
]);
```

## Accessibility

Both `<Hotkey>` and `<Kbd>` render semantic `<kbd>` HTML elements. `<Hotkey>` uses nested `<kbd>` elements (an outer `<kbd>` wrapping inner `<kbd>` elements for each key), which is the correct HTML semantics for a key combination.
144 changes: 88 additions & 56 deletions static/app/components/core/hotkey/keyMappings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,68 +28,100 @@ const aliases: Record<string, string> = {
'\u21EA': 'capslock', // ⇪
};

function canonicalize(keyName: string): string {
export function canonicalize(keyName: string): string {
const lower = keyName.toLowerCase();
return aliases[lower] ?? lower;
}

// Only uses canonical names
const keyCodeMap: Record<string, number> = {
backspace: 8,
tab: 9,
clear: 12,
enter: 13,
escape: 27,
space: 32,
left: 37,
up: 38,
right: 39,
down: 40,
delete: 46,
insert: 45,
home: 36,
end: 35,
pageup: 33,
pagedown: 34,
capslock: 20,
num_0: 96,
num_1: 97,
num_2: 98,
num_3: 99,
num_4: 100,
num_5: 101,
num_6: 102,
num_7: 103,
num_8: 104,
num_9: 105,
num_multiply: 106,
num_add: 107,
num_enter: 108,
num_subtract: 109,
num_decimal: 110,
num_divide: 111,
// Modifiers
shift: 16,
alt: 18,
control: 17,
command: 91,
// Punctuation (explicit entries avoid upper-casing surprises)
',': 188,
'.': 190,
'/': 191,
'`': 192,
'-': 189,
'=': 187,
';': 186,
"'": 222,
'[': 219,
']': 221,
'\\': 220,
// Maps canonical key names to their `event.key` value. Used for keys that
// have a stable cross-layout `event.key` (named keys, arrows, etc.).
const namedKeyMap: Record<string, string> = {
backspace: 'Backspace',
tab: 'Tab',
clear: 'Clear',
enter: 'Enter',
escape: 'Escape',
space: ' ',
left: 'ArrowLeft',
up: 'ArrowUp',
right: 'ArrowRight',
down: 'ArrowDown',
delete: 'Delete',
insert: 'Insert',
home: 'Home',
end: 'End',
pageup: 'PageUp',
pagedown: 'PageDown',
capslock: 'CapsLock',
};

export function getKeyCode(x: string): number {
const key = canonicalize(x);
return keyCodeMap[key] ?? x.toUpperCase().charCodeAt(0);
// Maps single-char punctuation to its `event.code` value. Used as a fallback
// alongside `event.key` so shortcuts work even when shift transforms the
// produced character (e.g. `shift+1` produces '!' on US QWERTY).
const punctuationCodeMap: Record<string, string> = {
',': 'Comma',
'.': 'Period',
'/': 'Slash',
'`': 'Backquote',
'-': 'Minus',
'=': 'Equal',
';': 'Semicolon',
"'": 'Quote',
'[': 'BracketLeft',
']': 'BracketRight',
'\\': 'Backslash',
};

const modifierPredicates: Record<string, (event: KeyboardEvent) => boolean> = {
command: e => e.metaKey,
shift: e => e.shiftKey,
control: e => e.ctrlKey,
alt: e => e.altKey,
};

export const MODIFIER_KEYS = ['command', 'shift', 'control', 'alt'] as const;

function codeForChar(ch: string): string | undefined {
if (ch >= 'a' && ch <= 'z') {
return `Key${ch.toUpperCase()}`;
}
if (ch >= '0' && ch <= '9') {
return `Digit${ch}`;
}
return punctuationCodeMap[ch];
}

/**
* Tests whether a single key name from a hotkey match string is currently
* pressed in the given keyboard event. Layout-aware: matches against
* `event.key` (so an AZERTY user pressing the K-labeled key fires `'k'`
* shortcuts) with `event.code` as a physical-position fallback (so
* `shift+1` works even though `event.key === '!'` on US QWERTY).
*/
export function matchesKey(name: string, event: KeyboardEvent): boolean {
const key = canonicalize(name);

const modifier = modifierPredicates[key];
if (modifier) {
return modifier(event);
}

const namedKey = namedKeyMap[key];
if (namedKey) {
return event.key === namedKey;
}

if (key.length === 1) {
if (event.key.toLowerCase() === key) {
return true;
}
const code = codeForChar(key);
if (code && event.code === code) {
return true;
}
}

return false;
}

type KeyGlyph = {icon: React.ReactNode; label: string} | {label: string};
Expand Down
172 changes: 167 additions & 5 deletions static/app/components/core/hotkey/useHotkeys.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,30 @@ import {renderHook} from 'sentry-test/reactTestingLibrary';

import {useHotkeys} from '@sentry/scraps/hotkey';

// eslint-disable-next-line boundaries/dependencies
import {getKeyCode} from './keyMappings';
function keyToKey(k: string): string {
return k;
}

function keyToCode(k: string): string {
if (k === '/') {
return 'Slash';
}
if (k >= 'a' && k <= 'z') {
return `Key${k.toUpperCase()}`;
}
if (k >= '0' && k <= '9') {
return `Digit${k}`;
}
return k;
}

describe('useHotkeys', () => {
let events: Record<string, (evt: EventListenerOrEventListenerObject) => void> = {};
let events: Record<string, (evt: any) => void> = {};

function makeKeyEventFixture(keyCode: any, options: any) {
function makeKeyEventFixture(keyName: string, options: any = {}) {
return {
keyCode: getKeyCode(keyCode),
key: keyToKey(keyName),
code: keyToCode(keyName),
preventDefault: jest.fn(),
...options,
};
Expand Down Expand Up @@ -170,4 +185,151 @@ describe('useHotkeys', () => {
expect(evt.preventDefault).not.toHaveBeenCalled();
expect(callback).toHaveBeenCalled();
});

it('matches shift+digit when event.key is the shifted symbol', () => {
// On US QWERTY, `shift+1` produces `event.key === '!'` but
// `event.code === 'Digit1'`. The code-arm fallback ensures the shortcut
// still fires.
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'command+shift+1', callback}],
});

events.keydown!({
key: '!',
code: 'Digit1',
metaKey: true,
shiftKey: true,
preventDefault: jest.fn(),
});

expect(callback).toHaveBeenCalled();
});

it('matches a letter via event.key regardless of physical position', () => {
// AZERTY user pressing the K-labeled key: `event.key === 'k'` even
// though the physical position differs from QWERTY.
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'command+k', callback}],
});

events.keydown!({
key: 'k',
code: 'KeyK',
metaKey: true,
preventDefault: jest.fn(),
});

expect(callback).toHaveBeenCalled();
});

it('matches a letter via event.code on non-Latin layouts', () => {
// Cyrillic user pressing the physical K position: `event.key === 'к'`
// (Cyrillic ka), `event.code === 'KeyK'`. The code-arm fallback fires.
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'command+k', callback}],
});

events.keydown!({
key: 'к',
code: 'KeyK',
metaKey: true,
preventDefault: jest.fn(),
});

expect(callback).toHaveBeenCalled();
});

it('matches Escape via event.key', () => {
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'Escape', callback}],
});

events.keydown!({key: 'Escape', code: 'Escape', preventDefault: jest.fn()});

expect(callback).toHaveBeenCalled();
});

it('matches arrow keys via event.key', () => {
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'left', callback}],
});

events.keydown!({key: 'ArrowLeft', code: 'ArrowLeft', preventDefault: jest.fn()});

expect(callback).toHaveBeenCalled();
});

it('matches vim-style alternatives alongside arrow keys', () => {
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: ['left', 'h'], callback}],
});

events.keydown!({key: 'ArrowLeft', code: 'ArrowLeft', preventDefault: jest.fn()});
expect(callback).toHaveBeenCalledTimes(1);

events.keydown!({key: 'h', code: 'KeyH', preventDefault: jest.fn()});
expect(callback).toHaveBeenCalledTimes(2);
});

it('skips a disabled hotkey without preventing default', () => {
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'Escape', enabled: false, callback}],
});

const evt = makeKeyEventFixture('Escape');
events.keydown!(evt);

expect(callback).not.toHaveBeenCalled();
expect(evt.preventDefault).not.toHaveBeenCalled();
});

it('respects toggling enabled between renders', () => {
const callback = jest.fn();

const {rerender} = renderHook(p => useHotkeys(p), {
initialProps: [{match: 'Escape', enabled: false, callback}],
});

events.keydown!(makeKeyEventFixture('Escape'));
expect(callback).not.toHaveBeenCalled();

rerender([{match: 'Escape', enabled: true, callback}]);

events.keydown!(makeKeyEventFixture('Escape'));
expect(callback).toHaveBeenCalledTimes(1);
});

it('matches a letter shortcut even when shift is held (case-insensitive)', () => {
// `command+shift+k` produces `event.key === 'K'` (uppercase). The
// implementation must lowercase before comparing to the match string.
const callback = jest.fn();

renderHook(p => useHotkeys(p), {
initialProps: [{match: 'command+shift+k', callback}],
});

events.keydown!({
key: 'K',
code: 'KeyK',
metaKey: true,
shiftKey: true,
preventDefault: jest.fn(),
});

expect(callback).toHaveBeenCalled();
});
});
Loading
Loading