-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[No QA] [TS migration] Migrate 'useKeyboardShortcut.js' hook to TypeScript #29226
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
Changes from all commits
4966d2f
9220d46
8ba4a08
52ffc1e
e065a1d
d7b1479
01a14f4
03d858f
ff032c6
f45c0a5
9d3d11e
9c1200b
ba5fb64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import {useEffect} from 'react'; | ||
| import {ValueOf} from 'type-fest'; | ||
| import KeyboardShortcut from '@libs/KeyboardShortcut'; | ||
| import CONST from '@src/CONST'; | ||
|
|
||
| type Shortcut = ValueOf<typeof CONST.KEYBOARD_SHORTCUTS>; | ||
| type KeyboardShortcutConfig = { | ||
| /* Should we capture the event on inputs too? */ | ||
| captureOnInputs?: boolean; | ||
| /* Should we bubble the event? */ | ||
| shouldBubble?: boolean; | ||
| /* The position the callback should take in the stack. 0 means top priority, and 1 means less priority than the most recently added. */ | ||
| priority?: number; | ||
| /* Should call event.preventDefault after callback? */ | ||
| shouldPreventDefault?: boolean; | ||
| /* Do not capture key events targeting excluded nodes (i.e. do not prevent default and let the event bubble) */ | ||
| excludedNodes?: string[]; | ||
| /* Is keyboard shortcut is already active */ | ||
| isActive?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * Register a keyboard shortcut handler. | ||
| * Recommendation: To ensure stability, wrap the `callback` function with the useCallback hook before using it with this hook. | ||
| */ | ||
| export default function useKeyboardShortcut(shortcut: Shortcut, callback: () => void, config: KeyboardShortcutConfig | Record<string, never> = {}) { | ||
| const { | ||
| captureOnInputs = true, | ||
| shouldBubble = false, | ||
| priority = 0, | ||
| shouldPreventDefault = true, | ||
|
|
||
| // The "excludedNodes" array needs to be stable to prevent the "useEffect" hook from being recreated unnecessarily. | ||
| // Hence the use of CONST.EMPTY_ARRAY. | ||
| excludedNodes = CONST.EMPTY_ARRAY, | ||
| isActive = true, | ||
| } = config; | ||
|
|
||
| useEffect(() => { | ||
| if (!isActive) { | ||
| return () => {}; | ||
| } | ||
|
|
||
| const unsubscribe = KeyboardShortcut.subscribe( | ||
| shortcut.shortcutKey, | ||
| callback, | ||
| shortcut.descriptionKey ?? '', | ||
| shortcut.modifiers, | ||
| captureOnInputs, | ||
| shouldBubble, | ||
| priority, | ||
| shouldPreventDefault, | ||
| excludedNodes as string[], | ||
| ); | ||
|
|
||
| return () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer no changing it to be sure to not introduce regressions, theoretically there shouldn't be be we never know 😅 |
||
| unsubscribe(); | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isActive, callback, captureOnInputs, excludedNodes, priority, shortcut.descriptionKey, shortcut.modifiers.join(), shortcut.shortcutKey, shouldBubble, shouldPreventDefault]); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,11 +19,13 @@ type EventHandler = { | |
| // Handlers for the various keyboard listeners we set up | ||
| const eventHandlers: Record<string, EventHandler[]> = {}; | ||
|
|
||
| type ShortcutModifiers = readonly ['CTRL'] | readonly ['CTRL', 'SHIFT'] | readonly []; | ||
|
|
||
| type Shortcut = { | ||
| displayName: string; | ||
| shortcutKey: string; | ||
| descriptionKey: string; | ||
| modifiers: string[]; | ||
| modifiers: ShortcutModifiers; | ||
| }; | ||
|
|
||
| // Documentation information for keyboard shortcuts that are displayed in the keyboard shortcuts informational modal | ||
|
|
@@ -102,13 +104,13 @@ function unsubscribe(displayName: string, callbackID: string) { | |
| /** | ||
| * Return platform specific modifiers for keys like Control (CMD on macOS) | ||
| */ | ||
| function getPlatformEquivalentForKeys(keys: string[]): string[] { | ||
| function getPlatformEquivalentForKeys(keys: ShortcutModifiers): string[] { | ||
| return keys.map((key) => { | ||
| if (!(key in CONST.PLATFORM_SPECIFIC_KEYS)) { | ||
| return key; | ||
| } | ||
|
|
||
| const platformModifiers = CONST.PLATFORM_SPECIFIC_KEYS[key as keyof typeof CONST.PLATFORM_SPECIFIC_KEYS]; | ||
| const platformModifiers = CONST.PLATFORM_SPECIFIC_KEYS[key]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice one 👍 |
||
| return platformModifiers?.[operatingSystem as keyof typeof platformModifiers] ?? platformModifiers.DEFAULT ?? key; | ||
| }); | ||
| } | ||
|
|
@@ -130,12 +132,12 @@ function subscribe( | |
| key: string, | ||
| callback: (event?: KeyboardEvent) => void, | ||
| descriptionKey: string, | ||
| modifiers: string[] = ['shift'], | ||
| modifiers: ShortcutModifiers = ['CTRL'], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kubabutkiewicz Why did you change the default value?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @VickyStash Because modifiers are declared here and there is no shortcut with only shift key modifier so it doesn't make sense to default it to that key |
||
| captureOnInputs = false, | ||
| shouldBubble = false, | ||
| priority = 0, | ||
| shouldPreventDefault = true, | ||
| excludedNodes = [], | ||
| excludedNodes: string[] = [], | ||
| shouldStopPropagation = false, | ||
| ) { | ||
| const platformAdjustedModifiers = getPlatformEquivalentForKeys(modifiers); | ||
|
|
@@ -190,4 +192,4 @@ const KeyboardShortcut = { | |
| }; | ||
|
|
||
| export default KeyboardShortcut; | ||
| export type {EventHandler}; | ||
| export type {EventHandler, Shortcut}; | ||
Uh oh!
There was an error while loading. Please reload this page.