From 4966d2f12407d8feaf0084e462acf5e0fe055b06 Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Tue, 10 Oct 2023 16:11:42 +0200 Subject: [PATCH 1/7] ref: move useKeyboardShortcut to TS --- src/hooks/useKeyboardShortcut.js | 42 ------------------------ src/hooks/useKeyboardShortcut.ts | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 42 deletions(-) delete mode 100644 src/hooks/useKeyboardShortcut.js create mode 100644 src/hooks/useKeyboardShortcut.ts diff --git a/src/hooks/useKeyboardShortcut.js b/src/hooks/useKeyboardShortcut.js deleted file mode 100644 index b8e297c966e..00000000000 --- a/src/hooks/useKeyboardShortcut.js +++ /dev/null @@ -1,42 +0,0 @@ -import {useEffect} from 'react'; -import KeyboardShortcut from '../libs/KeyboardShortcut'; -import CONST from '../CONST'; - -/** - * Register a keyboard shortcut handler. - * Recommendation: To ensure stability, wrap the `callback` function with the useCallback hook before using it with this hook. - * - * @param {Object} shortcut - * @param {Function} callback - * @param {Object} [config] - */ -export default function useKeyboardShortcut(shortcut, callback, config = {}) { - 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 KeyboardShortcut.subscribe( - shortcut.shortcutKey, - callback, - shortcut.descriptionKey, - shortcut.modifiers, - captureOnInputs, - shouldBubble, - priority, - shouldPreventDefault, - excludedNodes, - ); - } - return () => {}; - }, [isActive, callback, captureOnInputs, excludedNodes, priority, shortcut.descriptionKey, shortcut.modifiers, shortcut.shortcutKey, shouldBubble, shouldPreventDefault]); -} diff --git a/src/hooks/useKeyboardShortcut.ts b/src/hooks/useKeyboardShortcut.ts new file mode 100644 index 00000000000..b15fc44cbe1 --- /dev/null +++ b/src/hooks/useKeyboardShortcut.ts @@ -0,0 +1,56 @@ +import {useEffect} from 'react'; +import {ValueOf} from 'type-fest'; +import KeyboardShortcut from '../libs/KeyboardShortcut'; +import CONST from '../CONST'; + +type Shortcut = ValueOf; +type KeyboardShortcutConfig = { + captureOnInputs?: boolean; + shouldBubble?: boolean; + priority?: number; + shouldPreventDefault?: boolean; + excludedNodes?: Node[]; + 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 = {}) { + 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, + ); + + return () => { + unsubscribe(); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isActive, callback, captureOnInputs, excludedNodes, priority, shortcut.descriptionKey, shortcut.modifiers.join(), shortcut.shortcutKey, shouldBubble, shouldPreventDefault]); +} From 52ffc1e2d2280bd595de49c3ae44d102509fb798 Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Mon, 30 Oct 2023 13:53:21 +0100 Subject: [PATCH 2/7] fix: type issues --- src/hooks/useKeyboardShortcut.ts | 2 +- src/libs/KeyboardShortcut/index.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/hooks/useKeyboardShortcut.ts b/src/hooks/useKeyboardShortcut.ts index 4fce062170f..4d4f7a2cfb7 100644 --- a/src/hooks/useKeyboardShortcut.ts +++ b/src/hooks/useKeyboardShortcut.ts @@ -9,7 +9,7 @@ type KeyboardShortcutConfig = { shouldBubble?: boolean; priority?: number; shouldPreventDefault?: boolean; - excludedNodes?: Node[]; + excludedNodes?: string[]; isActive?: boolean; }; diff --git a/src/libs/KeyboardShortcut/index.ts b/src/libs/KeyboardShortcut/index.ts index 249311f92cd..0087ecb6dde 100644 --- a/src/libs/KeyboardShortcut/index.ts +++ b/src/libs/KeyboardShortcut/index.ts @@ -18,11 +18,13 @@ type EventHandler = { // Handlers for the various keyboard listeners we set up const eventHandlers: Record = {}; +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 @@ -101,13 +103,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]; return platformModifiers?.[operatingSystem as keyof typeof platformModifiers] ?? platformModifiers.DEFAULT ?? key; }); } @@ -129,12 +131,12 @@ function subscribe( key: string, callback: () => void, descriptionKey: string, - modifiers: string[] = ['shift'], + modifiers: ShortcutModifiers = ['CTRL'], captureOnInputs = false, shouldBubble = false, priority = 0, shouldPreventDefault = true, - excludedNodes = [], + excludedNodes: string[] | readonly never[] = [], ) { const platformAdjustedModifiers = getPlatformEquivalentForKeys(modifiers); const displayName = getDisplayName(key, platformAdjustedModifiers); From e065a1dbd58754bc89c49be93a3485b3044d62d9 Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Mon, 30 Oct 2023 14:45:11 +0100 Subject: [PATCH 3/7] fix: types issues --- src/libs/KeyboardShortcut/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/KeyboardShortcut/index.ts b/src/libs/KeyboardShortcut/index.ts index 0087ecb6dde..365ce779dac 100644 --- a/src/libs/KeyboardShortcut/index.ts +++ b/src/libs/KeyboardShortcut/index.ts @@ -151,7 +151,7 @@ function subscribe( captureOnInputs, shouldPreventDefault, shouldBubble, - excludedNodes, + excludedNodes: [...excludedNodes], }); if (descriptionKey) { From 01a14f4c018902f03450cbf8a5ccf5efc7c49c5f Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Tue, 31 Oct 2023 12:40:00 +0100 Subject: [PATCH 4/7] fix: add props comments --- src/hooks/useKeyboardShortcut.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/hooks/useKeyboardShortcut.ts b/src/hooks/useKeyboardShortcut.ts index 4d4f7a2cfb7..2d272002823 100644 --- a/src/hooks/useKeyboardShortcut.ts +++ b/src/hooks/useKeyboardShortcut.ts @@ -5,11 +5,17 @@ import CONST from '@src/CONST'; type Shortcut = ValueOf; 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; }; From ff032c684dcb46b84f9a4da22b652b6aa15d2af2 Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Tue, 7 Nov 2023 16:30:37 +0100 Subject: [PATCH 5/7] fix: resolve comments --- src/hooks/useKeyboardShortcut.ts | 2 +- src/libs/KeyboardShortcut/index.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hooks/useKeyboardShortcut.ts b/src/hooks/useKeyboardShortcut.ts index 2d272002823..3a4647ccff5 100644 --- a/src/hooks/useKeyboardShortcut.ts +++ b/src/hooks/useKeyboardShortcut.ts @@ -51,7 +51,7 @@ export default function useKeyboardShortcut(shortcut: Shortcut, callback: () => shouldBubble, priority, shouldPreventDefault, - excludedNodes, + excludedNodes as string[], ); return () => { diff --git a/src/libs/KeyboardShortcut/index.ts b/src/libs/KeyboardShortcut/index.ts index 8cdb73ea807..781ee6fcbf2 100644 --- a/src/libs/KeyboardShortcut/index.ts +++ b/src/libs/KeyboardShortcut/index.ts @@ -137,7 +137,7 @@ function subscribe( shouldBubble = false, priority = 0, shouldPreventDefault = true, - excludedNodes: string[] | readonly never[] = [], + excludedNodes: string[] = [], shouldStopPropagation = false, ) { const platformAdjustedModifiers = getPlatformEquivalentForKeys(modifiers); @@ -153,7 +153,7 @@ function subscribe( captureOnInputs, shouldPreventDefault, shouldBubble, - excludedNodes: [...excludedNodes], + excludedNodes, shouldStopPropagation, }); From f45c0a5fef41cf132886df9aafe09b781e244ef8 Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Tue, 7 Nov 2023 16:32:11 +0100 Subject: [PATCH 6/7] fix: fixed typo --- src/hooks/useKeyboardShortcut.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hooks/useKeyboardShortcut.ts b/src/hooks/useKeyboardShortcut.ts index 3a4647ccff5..e4a7a16f4cf 100644 --- a/src/hooks/useKeyboardShortcut.ts +++ b/src/hooks/useKeyboardShortcut.ts @@ -22,7 +22,6 @@ type KeyboardShortcutConfig = { /** * 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 = {}) { const { From 9c1200ba3a91dded7c075a9bfa7e0a954ecc64d8 Mon Sep 17 00:00:00 2001 From: Jakub Butkiewicz Date: Tue, 7 Nov 2023 19:18:18 +0100 Subject: [PATCH 7/7] fix: types --- src/components/Pressable/GenericPressable/types.ts | 8 +------- src/libs/KeyboardShortcut/index.ts | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/components/Pressable/GenericPressable/types.ts b/src/components/Pressable/GenericPressable/types.ts index 35616cb600a..cfe35641bff 100644 --- a/src/components/Pressable/GenericPressable/types.ts +++ b/src/components/Pressable/GenericPressable/types.ts @@ -1,17 +1,11 @@ import {ElementRef, RefObject} from 'react'; import {GestureResponderEvent, HostComponent, PressableStateCallbackType, PressableProps as RNPressableProps, StyleProp, ViewStyle} from 'react-native'; import {ValueOf} from 'type-fest'; +import {Shortcut} from '@libs/KeyboardShortcut'; import CONST from '@src/CONST'; type StylePropWithFunction = StyleProp | ((state: PressableStateCallbackType) => StyleProp); -type Shortcut = { - displayName: string; - shortcutKey: string; - descriptionKey: string; - modifiers: string[]; -}; - type RequiredAccessibilityLabel = | { /** diff --git a/src/libs/KeyboardShortcut/index.ts b/src/libs/KeyboardShortcut/index.ts index 861e49f3179..44ba54953c4 100644 --- a/src/libs/KeyboardShortcut/index.ts +++ b/src/libs/KeyboardShortcut/index.ts @@ -192,4 +192,4 @@ const KeyboardShortcut = { }; export default KeyboardShortcut; -export type {EventHandler}; +export type {EventHandler, Shortcut};