Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] fix(HotkeyParser): use e.code to get physical keys #6301

Merged
merged 4 commits into from
Jul 24, 2023
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
70 changes: 48 additions & 22 deletions packages/core/src/components/hotkeys/hotkeyParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,17 @@ export const CONFIG_ALIASES: KeyMap = {
left: "ArrowLeft",
down: "ArrowDown",
right: "ArrowRight",
space: " ",
};

/**
* 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 = {
"~": "`",
"!": "1",
Copy link
Contributor Author

@adidahiya adidahiya Jul 24, 2023

Choose a reason for hiding this comment

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

We no longer need the 0-9 digit keys in this mapping because we now handle events with { code: "DigitX" } in a more straightforward way. In a future PR, we can remove this KeyMap entirely by switching to event.code for more keys, but for now I decided to be conservative and make fewer behavioral changes in this PR.

Edit: I added a code comment to reflect this

"@": "2",
"#": "3",
$: "4",
"%": "5",
"^": "6",
"&": "7",
"*": "8",
"(": "9",
")": "0",
_: "-",
"+": "=",
"{": "[",
Expand Down Expand Up @@ -130,7 +126,10 @@ export const parseKeyCombo = (combo: string): KeyCombo => {
};

/**
* Converts a keyboard event into a valid combo prop string
* 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.
*/
export const getKeyComboString = (e: KeyboardEvent): string => {
const comboParts = [] as string[];
Expand All @@ -149,32 +148,59 @@ 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());
}
}

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)) {
// Code looks like "Digit1", etc.
return e.code.substring(DIGIT_CODE_PREFIX.length).toLowerCase();
} else if (e.code === "Space") {
return "space";
}

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;
Expand Down
14 changes: 10 additions & 4 deletions packages/core/src/components/hotkeys/hotkeys-target2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
<div class="@ns-callout @ns-intent-primary @ns-icon-info-sign">

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.

</div>

@## Props interface

@interface HotkeysTarget2Props

@interface HotkeyConfig

@interface UseHotkeysOptions

@## Hotkey configuration

@interface HotkeyConfig
2 changes: 1 addition & 1 deletion packages/core/src/components/hotkeys/hotkeysTarget2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/context/hotkeys/hotkeysProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) ?? (
<HotkeysDialog2
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/hooks/hotkeys/hotkeyConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
65 changes: 64 additions & 1 deletion packages/core/src/hooks/hotkeys/use-hotkeys.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <kbd>?</kbd> key.

@reactExample UseHotkeysExample

Expand Down Expand Up @@ -74,8 +74,71 @@ and configure the <kbd>?</kbd> 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` &rarr; `alt`
- `cmd` &rarr; `meta`
- `command` &rarr; `meta`
- `return` &rarr; `enter`
- `escape` &rarr; `esc`
- `win` &rarr; `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
62 changes: 30 additions & 32 deletions packages/core/src/hooks/hotkeys/useHotkeys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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<HTMLElement>) =>
invokeNamedCallbackIfComboRecognized(false, getKeyCombo(e.nativeEvent), "onKeyDown", e.nativeEvent),
[localKeys],
[invokeNamedCallbackIfComboRecognized],
);
const handleLocalKeyUp = React.useCallback(
(e: React.KeyboardEvent<HTMLElement>) =>
invokeNamedCallbackIfComboRecognized(false, getKeyCombo(e.nativeEvent), "onKeyUp", e.nativeEvent),
[localKeys],
[invokeNamedCallbackIfComboRecognized],
);

React.useEffect(() => {
Expand Down
Loading