From 2f3d0e49991aed9722688c3e73ccf0a151f3f8c2 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 24 Jul 2023 12:42:46 -0400 Subject: [PATCH 1/4] fix hook warnings in hotkeys --- .../src/components/hotkeys/hotkeysTarget2.tsx | 2 +- packages/core/src/hooks/hotkeys/useHotkeys.ts | 62 +++++++++---------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysTarget2.tsx b/packages/core/src/components/hotkeys/hotkeysTarget2.tsx index 9b930e8fc9..eb8393dc1f 100644 --- a/packages/core/src/components/hotkeys/hotkeysTarget2.tsx +++ b/packages/core/src/components/hotkeys/hotkeysTarget2.tsx @@ -54,7 +54,7 @@ export const HotkeysTarget2 = ({ children, hotkeys, options }: HotkeysTarget2Pro console.error(Errors.HOTKEYS_TARGET_CHILDREN_LOCAL_HOTKEYS); } } - }, [hotkeys]); + }, [hotkeys, children]); if (typeof children === "function") { return children({ handleKeyDown, handleKeyUp }); diff --git a/packages/core/src/hooks/hotkeys/useHotkeys.ts b/packages/core/src/hooks/hotkeys/useHotkeys.ts index b386c56efa..c44eabeccf 100644 --- a/packages/core/src/hooks/hotkeys/useHotkeys.ts +++ b/packages/core/src/hooks/hotkeys/useHotkeys.ts @@ -81,43 +81,41 @@ export function useHotkeys(keys: readonly HotkeyConfig[], options: UseHotkeysOpt if (!state.hasProvider) { console.warn(HOTKEYS_PROVIDER_NOT_FOUND); } - }, []); + }, [state.hasProvider]); // we can still bind the hotkeys if there is no HotkeysProvider, they just won't show up in the dialog React.useEffect(() => { const payload = [...globalKeys.map(k => k.config), ...localKeys.map(k => k.config)]; dispatch({ type: "ADD_HOTKEYS", payload }); return () => dispatch({ type: "REMOVE_HOTKEYS", payload }); - }, [keys]); + }, [dispatch, globalKeys, localKeys]); - const invokeNamedCallbackIfComboRecognized = ( - global: boolean, - combo: KeyCombo, - callbackName: "onKeyDown" | "onKeyUp", - e: KeyboardEvent, - ) => { - const isTextInput = elementIsTextInput(e.target as HTMLElement); - for (const key of global ? globalKeys : localKeys) { - const { - allowInInput = false, - disabled = false, - preventDefault = false, - stopPropagation = false, - } = key.config; - const shouldIgnore = (isTextInput && !allowInInput) || disabled; - if (!shouldIgnore && comboMatches(key.combo, combo)) { - if (preventDefault) { - e.preventDefault(); - } - if (stopPropagation) { - // set a flag just for unit testing. not meant to be referenced in feature work. - (e as any).isPropagationStopped = true; - e.stopPropagation(); + const invokeNamedCallbackIfComboRecognized = React.useCallback( + (global: boolean, combo: KeyCombo, callbackName: "onKeyDown" | "onKeyUp", e: KeyboardEvent) => { + const isTextInput = elementIsTextInput(e.target as HTMLElement); + for (const key of global ? globalKeys : localKeys) { + const { + allowInInput = false, + disabled = false, + preventDefault = false, + stopPropagation = false, + } = key.config; + const shouldIgnore = (isTextInput && !allowInInput) || disabled; + if (!shouldIgnore && comboMatches(key.combo, combo)) { + if (preventDefault) { + e.preventDefault(); + } + if (stopPropagation) { + // set a flag just for unit testing. not meant to be referenced in feature work. + (e as any).isPropagationStopped = true; + e.stopPropagation(); + } + key.config[callbackName]?.(e); } - key.config[callbackName]?.(e); } - } - }; + }, + [globalKeys, localKeys], + ); const handleGlobalKeyDown = React.useCallback( (e: KeyboardEvent) => { @@ -130,22 +128,22 @@ export function useHotkeys(keys: readonly HotkeyConfig[], options: UseHotkeysOpt invokeNamedCallbackIfComboRecognized(true, getKeyCombo(e), "onKeyDown", e); } }, - [globalKeys], + [dispatch, invokeNamedCallbackIfComboRecognized, showDialogKeyCombo], ); const handleGlobalKeyUp = React.useCallback( (e: KeyboardEvent) => invokeNamedCallbackIfComboRecognized(true, getKeyCombo(e), "onKeyUp", e), - [globalKeys], + [invokeNamedCallbackIfComboRecognized], ); const handleLocalKeyDown = React.useCallback( (e: React.KeyboardEvent) => invokeNamedCallbackIfComboRecognized(false, getKeyCombo(e.nativeEvent), "onKeyDown", e.nativeEvent), - [localKeys], + [invokeNamedCallbackIfComboRecognized], ); const handleLocalKeyUp = React.useCallback( (e: React.KeyboardEvent) => invokeNamedCallbackIfComboRecognized(false, getKeyCombo(e.nativeEvent), "onKeyUp", e.nativeEvent), - [localKeys], + [invokeNamedCallbackIfComboRecognized], ); React.useEffect(() => { From 263a46493f671d0eec3c06f5064c0116aaebe8c1 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 24 Jul 2023 14:18:22 -0400 Subject: [PATCH 2/4] [core] fix(HotkeyParser): use e.code to get physical keys --- .../src/components/hotkeys/hotkeyParser.ts | 61 +++++++++++------ .../src/components/hotkeys/hotkeys-target2.md | 14 ++-- .../core/src/hooks/hotkeys/hotkeyConfig.ts | 2 +- .../core/src/hooks/hotkeys/use-hotkeys.md | 65 ++++++++++++++++++- packages/core/src/legacy/hotkeys-legacy.md | 62 ------------------ .../core/test/hotkeys/hotkeysParserTests.ts | 49 +++++++++----- .../core-examples/hotkeyTesterExample.tsx | 2 +- packages/docs-app/src/styles/_examples.scss | 2 +- 8 files changed, 148 insertions(+), 109 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index 08f201caed..4df56a676d 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/packages/core/src/components/hotkeys/hotkeyParser.ts @@ -61,18 +61,9 @@ export const CONFIG_ALIASES: KeyMap = { space: " ", }; +/** Key mapping used in getKeyCombo() implementation for physical keys which are not alphabet characters or digits. */ export const SHIFT_KEYS: KeyMap = { "~": "`", - "!": "1", - "@": "2", - "#": "3", - $: "4", - "%": "5", - "^": "6", - "&": "7", - "*": "8", - "(": "9", - ")": "0", _: "-", "+": "=", "{": "[", @@ -130,7 +121,10 @@ export const parseKeyCombo = (combo: string): KeyCombo => { }; /** - * Converts a keyboard event into a valid combo prop string + * Interprets a keyboard event as a valid KeyCoboTag `combo` prop string value. + * + * Note that this function is only used in the docs example and tests; it is not used by `useHotkeys()` or any + * Blueprint consumers that we are currently aware of. */ export const getKeyComboString = (e: KeyboardEvent): string => { const comboParts = [] as string[]; @@ -149,13 +143,17 @@ export const getKeyComboString = (e: KeyboardEvent): string => { comboParts.push("meta"); } - if (e.key !== undefined) { - if (e.key === " ") { - // special case for "space" key, which would otherwise be printed as illegible whitespace + const key = maybeGetKeyFromEventCode(e); + if (key !== undefined) { + comboParts.push(key); + } else { + if (e.code === "Space") { comboParts.push("space"); } else if (MODIFIER_KEYS.has(e.key)) { // do nothing - } else { + } else if (e.shiftKey && SHIFT_KEYS[e.key] !== undefined) { + comboParts.push(SHIFT_KEYS[e.key]); + } else if (e.key !== undefined) { comboParts.push(e.key.toLowerCase()); } } @@ -163,18 +161,39 @@ export const getKeyComboString = (e: KeyboardEvent): string => { return comboParts.join(" + "); }; +const KEY_CODE_PREFIX = "Key"; +const DIGIT_CODE_PREFIX = "Digit"; + +function maybeGetKeyFromEventCode(e: KeyboardEvent) { + if (e.code == null) { + return undefined; + } + + if (e.code.startsWith(KEY_CODE_PREFIX)) { + // Code looks like "KeyA", etc. + return e.code.substring(KEY_CODE_PREFIX.length).toLowerCase(); + } else if (e.code.startsWith(DIGIT_CODE_PREFIX)) { + // Similar to previous case: code looks like "Digit1", etc. + return e.code.substring(DIGIT_CODE_PREFIX.length).toLowerCase(); + } + + return undefined; +} + /** - * Determines the key combo object from the given keyboard event. Again, a key - * combo includes zero or more modifiers (represented by a bitmask) and one - * action key, which we determine from the `e.key` property of the keyboard - * event. + * Determines the key combo object from the given keyboard event. A key combo includes zero or more modifiers + * (represented by a bitmask) and one physical key. For most keys, we prefer dealing with the `code` property of the + * event, since this is not altered by keyboard layout or the state of modifier keys. Fall back to using the `key` + * property. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code */ export const getKeyCombo = (e: KeyboardEvent): KeyCombo => { let key: string | undefined; if (MODIFIER_KEYS.has(e.key)) { - // keep key undefined + // Leave local variable `key` undefined } else { - key = e.key?.toLowerCase(); + key = maybeGetKeyFromEventCode(e) ?? e.key?.toLowerCase(); } let modifiers = 0; diff --git a/packages/core/src/components/hotkeys/hotkeys-target2.md b/packages/core/src/components/hotkeys/hotkeys-target2.md index b76264b504..d86b434aec 100644 --- a/packages/core/src/components/hotkeys/hotkeys-target2.md +++ b/packages/core/src/components/hotkeys/hotkeys-target2.md @@ -75,13 +75,19 @@ event handlers with the `handleKeyDown` and `handleKeyUp` functions in the child you will likely have to set a non-negative `tabIndex` on the DOM node to which these local event handlers are bound for them to work correctly. -__HotkeysTarget2__ takes an optional `options: UseHotkeysOptions` prop which can customize some of the hook's -default behavior. +
+ +See the [useHotkeys hook documentation](#core/hooks/use-hotkeys.key-combos) to understand the semantics of "key combos": +how they are configured and how they will appear in the global dialog. + +
@## Props interface @interface HotkeysTarget2Props -@interface HotkeyConfig - @interface UseHotkeysOptions + +@## Hotkey configuration + +@interface HotkeyConfig diff --git a/packages/core/src/hooks/hotkeys/hotkeyConfig.ts b/packages/core/src/hooks/hotkeys/hotkeyConfig.ts index 658c22af90..df6fbee674 100644 --- a/packages/core/src/hooks/hotkeys/hotkeyConfig.ts +++ b/packages/core/src/hooks/hotkeys/hotkeyConfig.ts @@ -23,7 +23,7 @@ export interface HotkeyConfig { allowInInput?: boolean; /** - * Hotkey combination string, such as "space" or "cmd+n". + * Hotkey combination string (AKA "key combo"), such as "space" or "cmd+n". */ combo: string; diff --git a/packages/core/src/hooks/hotkeys/use-hotkeys.md b/packages/core/src/hooks/hotkeys/use-hotkeys.md index a1727cdd15..a34da05d51 100644 --- a/packages/core/src/hooks/hotkeys/use-hotkeys.md +++ b/packages/core/src/hooks/hotkeys/use-hotkeys.md @@ -20,7 +20,7 @@ The `useHotkeys` hook adds hotkey / keyboard shortcut interactions to your appli Compared to the deprecated [Hotkeys API](#core/legacy/hotkeys-legacy), it works with function components and its corresponding [context provider](#core/context/hotkeys-provider) allows more customization of the hotkeys dialog. -Focus on the piano below to try its hotkeys. The global hotkeys dialog can be shown using the "?" key. +Focus on the piano below to try its hotkeys. The global hotkeys dialog can be shown using the ? key. @reactExample UseHotkeysExample @@ -74,8 +74,71 @@ and configure the ? key to open the generated hotkeys dialog, but it event handlers with the returned `handleKeyDown` and `handleKeyUp` functions. The hook takes an optional second parameter which can customize some of its default behavior. +@## Hook options + @interface UseHotkeysOptions @method useHotkeys +@## Hotkey configuration + @interface HotkeyConfig + +@## Key combos + +Each hotkey must be assigned a key combo that will trigger its events. A key combo consists of zero or more modifier +keys (`alt`, `ctrl`, `shift`, `meta`, `cmd`) and exactly one action key, such as `A`, `return`, or `up`. + +Some key combos have aliases. For example, `shift + 1` can equivalently be expressed as `!` and `cmd` is equal to +`meta`. However, normal alphabetic characters do not have this aliasing, so `X` is equivalent to `x` but is not +equivalent to `shift + x`. + +Examples of valid key combos: + +- `cmd+plus` +- `!` or, equivalently `shift+1` +- `return` or, equivalently `enter` +- `alt + shift + x` +- `ctrl + left` + +Note that spaces are ignored. + +### Named keys + +- `plus` +- `minus` +- `backspace` +- `tab` +- `enter` +- `capslock` +- `esc` +- `space` +- `pageup` +- `pagedown` +- `end` +- `home` +- `left` +- `up` +- `right` +- `down` +- `ins` +- `del` + +### Aliased keys + +- `option` → `alt` +- `cmd` → `meta` +- `command` → `meta` +- `return` → `enter` +- `escape` → `esc` +- `win` → `meta` + +The special modifier `mod` will choose the OS-preferred modifier key: `cmd` for macOS and iOS, or `ctrl` for Windows +and Linux. + +@## Key combo tester + +Below is a little widget to quickly help you try out hotkey combos and see how they will appear in the dialog. See the +[Key combos section](#core/hooks/use-hotkeys.key-combos) above for more info. + +@reactExample HotkeyTesterExample diff --git a/packages/core/src/legacy/hotkeys-legacy.md b/packages/core/src/legacy/hotkeys-legacy.md index 6b55b551ba..8368c71b0c 100644 --- a/packages/core/src/legacy/hotkeys-legacy.md +++ b/packages/core/src/legacy/hotkeys-legacy.md @@ -112,65 +112,3 @@ focused on an element that has any hotkeys, those will be shown as well. If you would like to change the style of the dialog (for example, to apply the dark theme class), call the `setHotkeysDialogProps` function with `IDialogProps`. - -@## Key combos - -Each hotkey must be assigned a key combo that will trigger its events. A key -combo consists of zero or more modifier keys (`alt`, `ctrl`, `shift`, `meta`, -`cmd`) and exactly one action key, such as `A`, `return`, or `up`. - -Some key combos have aliases. For example, `shift + 1` can equivalently be -expressed as `!` and `cmd` is equal to `meta`. However, normal alphabetic -characters do not have this aliasing, so `X` is equivalent to `x` but is not -equivalent to `shift + x`. - -Examples of valid key combos: - -- `cmd+plus` -- `!` or, equivalently `shift+1` -- `return` or, equivalently `enter` -- `alt + shift + x` -- `ctrl + left` - -Note that spaces are ignored. - -### Named keys - -- `plus` -- `minus` -- `backspace` -- `tab` -- `enter` -- `capslock` -- `esc` -- `space` -- `pageup` -- `pagedown` -- `end` -- `home` -- `left` -- `up` -- `right` -- `down` -- `ins` -- `del` - -### Aliased keys - -- `option` → `alt` -- `cmd` → `meta` -- `command` → `meta` -- `return` → `enter` -- `escape` → `esc` -- `win` → `meta` - -The special modifier `mod` will choose the OS-preferred modifier key — `cmd` -for macOS and iOS, or `ctrl` for Windows and Linux. - -### Hotkey tester - -Below is a little widget to quickly help you try out hotkey combos and see how -they will look in the dialog. See the key combos section above for more about -specifying key combo props. - -@reactExample HotkeyTesterExample diff --git a/packages/core/test/hotkeys/hotkeysParserTests.ts b/packages/core/test/hotkeys/hotkeysParserTests.ts index 36f5622d32..2b2ff7936c 100644 --- a/packages/core/test/hotkeys/hotkeysParserTests.ts +++ b/packages/core/test/hotkeys/hotkeysParserTests.ts @@ -25,8 +25,9 @@ import { parseKeyCombo, } from "../../src/components/hotkeys/hotkeyParser"; -describe("HotkeysParser", () => { - describe("KeyCombo parser", () => { +describe.only("HotkeysParser", () => { + // N.B. we test these two functions together to match how they are used in useHotkeys() + describe("getKeyCombo + parseKeyCombo", () => { interface ComboTest { combo: string; stringKeyCombo: string; @@ -58,7 +59,7 @@ describe("HotkeysParser", () => { Array.apply(null, Array(26)).map((_: any, i: number) => { const charString = String.fromCharCode(alpha + i).toLowerCase(); const combo = charString; - return makeComboTest(combo, { key: charString }); + return makeComboTest(combo, { key: charString, code: `Key${charString.toUpperCase()}` }); }), ); }); @@ -69,7 +70,7 @@ describe("HotkeysParser", () => { Array.apply(null, Array(26)).map((_: any, i: number) => { const charString = String.fromCharCode(alpha + i).toLowerCase(); const combo = charString.toUpperCase(); - return makeComboTest(combo, { key: charString }); + return makeComboTest(combo, { key: charString, code: `Key${charString.toUpperCase()}` }); }), false, ); // don't compare string combos @@ -81,23 +82,29 @@ describe("HotkeysParser", () => { Array.apply(null, Array(26)).map((_: any, i: number) => { const charString = String.fromCharCode(alpha + i).toLowerCase(); const combo = "shift + " + charString; - return makeComboTest(combo, { shiftKey: true, key: charString }); + return makeComboTest(combo, { + code: `Key${charString.toUpperCase()}`, + key: charString, + shiftKey: true, + }); }), ); }); it("matches modifiers only", () => { const tests = [] as ComboTest[]; - tests.push(makeComboTest("shift", { shiftKey: true } as any)); + tests.push(makeComboTest("shift", { shiftKey: true, code: "ShiftLeft" } as any)); tests.push( makeComboTest("ctrl + alt + shift", { altKey: true, + code: "ShiftLeft", ctrlKey: true, shiftKey: true, } as any as KeyboardEvent), ); tests.push( makeComboTest("ctrl + meta", { + code: "MetaLeft", ctrlKey: true, metaKey: true, } as any as KeyboardEvent), @@ -108,32 +115,38 @@ describe("HotkeysParser", () => { // these tests no longer make sense with the migration from key codes to named keys, they can likely be deleted it.skip("adds Shift to keys that imply it", () => { const tests = [] as ComboTest[]; - tests.push(makeComboTest("!", { shiftKey: true, key: "!" })); - tests.push(makeComboTest("@", { shiftKey: true, key: "@" })); - tests.push(makeComboTest("{", { shiftKey: true, key: "{" })); + tests.push(makeComboTest("!", { shiftKey: true, key: "!", code: "Digit1" })); + tests.push(makeComboTest("@", { shiftKey: true, key: "@", code: "Digit2" })); + tests.push(makeComboTest("{", { shiftKey: true, key: "{", code: "BracketLeft" })); // don't verify the strings because these will be converted to // `Shift + 1`, etc. verifyCombos(tests, false); }); - it("handles plus", () => { - expect(() => parseKeyCombo("ctrl + +")).to.throw(/failed to parse/i); - - expect(comboMatches(parseKeyCombo("cmd + plus"), parseKeyCombo("meta + plus"))).to.be.true; - }); - it("handles space key", () => { const tests = [] as ComboTest[]; tests.push( - makeComboTest("space", { key: " " }), - makeComboTest("ctrl + space", { ctrlKey: true, key: " " }), + makeComboTest("space", { key: " ", code: "Space" }), + makeComboTest("ctrl + space", { ctrlKey: true, key: " ", code: "Space" }), ); verifyCombos(tests); }); + it("handles alt modifier key", () => { + const tests = [] as ComboTest[]; + tests.push(makeComboTest("alt + a", { altKey: true, key: "a", code: "KeyA" })); + verifyCombos(tests); + }); + }); + + describe("parseKeyCombo", () => { + it("handles 'plus' key identifier", () => { + expect(() => parseKeyCombo("ctrl + +")).to.throw(/failed to parse/i); + expect(comboMatches(parseKeyCombo("cmd + plus"), parseKeyCombo("meta + plus"))).to.be.true; + }); + it("applies aliases", () => { expect(comboMatches(parseKeyCombo("return"), parseKeyCombo("enter"))).to.be.true; - expect(comboMatches(parseKeyCombo("win + F"), parseKeyCombo("meta + f"))).to.be.true; }); }); diff --git a/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx b/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx index 653d53793d..69ef3c8821 100644 --- a/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx +++ b/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx @@ -61,7 +61,7 @@ export class HotkeyTesterExample extends React.PureComponent Date: Mon, 24 Jul 2023 14:32:28 -0400 Subject: [PATCH 3/4] Fix self-review and lint --- packages/core/src/components/hotkeys/hotkeyParser.ts | 10 ++++++++-- packages/core/src/context/hotkeys/hotkeysProvider.tsx | 2 +- packages/core/test/hooks/useHotkeysTests.tsx | 2 +- packages/core/test/hotkeys/hotkeysParserTests.ts | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index 4df56a676d..f0c3b70e07 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/packages/core/src/components/hotkeys/hotkeyParser.ts @@ -61,7 +61,13 @@ export const CONFIG_ALIASES: KeyMap = { space: " ", }; -/** Key mapping used in getKeyCombo() implementation for physical keys which are not alphabet characters or digits. */ +/** + * Key mapping used in getKeyCombo() implementation for physical keys which are not alphabet characters or digits. + * + * N.B. at some point, we should stop using this mapping, since we can implement the desired functionality in a more + * straightforward way by using the `event.code` property, which will always tell us the identifiers represented by the + * _values_ in this object (the default physical keys, unaltered by modifier keys or keyboard layout). + */ export const SHIFT_KEYS: KeyMap = { "~": "`", _: "-", @@ -121,7 +127,7 @@ export const parseKeyCombo = (combo: string): KeyCombo => { }; /** - * Interprets a keyboard event as a valid KeyCoboTag `combo` prop string value. + * Interprets a keyboard event as a valid KeyComboTag `combo` prop string value. * * Note that this function is only used in the docs example and tests; it is not used by `useHotkeys()` or any * Blueprint consumers that we are currently aware of. diff --git a/packages/core/src/context/hotkeys/hotkeysProvider.tsx b/packages/core/src/context/hotkeys/hotkeysProvider.tsx index 65110eed41..0552dc0e65 100644 --- a/packages/core/src/context/hotkeys/hotkeysProvider.tsx +++ b/packages/core/src/context/hotkeys/hotkeysProvider.tsx @@ -111,7 +111,7 @@ export const HotkeysProvider = ({ children, dialogProps, renderDialog, value }: const hasExistingContext = value != null; const fallbackReducer = React.useReducer(hotkeysReducer, { ...initialHotkeysState, hasProvider: true }); const [state, dispatch] = value ?? fallbackReducer; - const handleDialogClose = React.useCallback(() => dispatch({ type: "CLOSE_DIALOG" }), []); + const handleDialogClose = React.useCallback(() => dispatch({ type: "CLOSE_DIALOG" }), [dispatch]); const dialog = renderDialog?.(state, { handleDialogClose }) ?? ( = ({ bindExtraKeys, isInputRea ); } return keys; - }, [bindExtraKeys]); + }, [bindExtraKeys, onKeyA, onKeyB]); const { handleKeyDown, handleKeyUp } = useHotkeys(hotkeys); diff --git a/packages/core/test/hotkeys/hotkeysParserTests.ts b/packages/core/test/hotkeys/hotkeysParserTests.ts index 2b2ff7936c..7531090ea5 100644 --- a/packages/core/test/hotkeys/hotkeysParserTests.ts +++ b/packages/core/test/hotkeys/hotkeysParserTests.ts @@ -25,7 +25,7 @@ import { parseKeyCombo, } from "../../src/components/hotkeys/hotkeyParser"; -describe.only("HotkeysParser", () => { +describe("HotkeysParser", () => { // N.B. we test these two functions together to match how they are used in useHotkeys() describe("getKeyCombo + parseKeyCombo", () => { interface ComboTest { From 7137072daf38fb570826a90577b3813547617170 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 24 Jul 2023 15:15:03 -0400 Subject: [PATCH 4/4] use e.code for space key --- packages/core/src/components/hotkeys/hotkeyParser.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index f0c3b70e07..fa7999970a 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/packages/core/src/components/hotkeys/hotkeyParser.ts @@ -58,7 +58,6 @@ export const CONFIG_ALIASES: KeyMap = { left: "ArrowLeft", down: "ArrowDown", right: "ArrowRight", - space: " ", }; /** @@ -179,8 +178,10 @@ function maybeGetKeyFromEventCode(e: KeyboardEvent) { // Code looks like "KeyA", etc. return e.code.substring(KEY_CODE_PREFIX.length).toLowerCase(); } else if (e.code.startsWith(DIGIT_CODE_PREFIX)) { - // Similar to previous case: code looks like "Digit1", etc. + // Code looks like "Digit1", etc. return e.code.substring(DIGIT_CODE_PREFIX.length).toLowerCase(); + } else if (e.code === "Space") { + return "space"; } return undefined;