From 60c0e928993c257e84e6078343f34495b6738301 Mon Sep 17 00:00:00 2001 From: Saad Najmi Date: Wed, 4 Jan 2023 10:27:50 -0600 Subject: [PATCH] More useKeyProps followup (#2477) * More useKeyProps followup * Change files * Revert windows change, add comment --- ...-1a2b6bba-f91d-4116-9de7-9a46ac1d9b9b.json | 7 ++ .../__snapshots__/useKeyProps.test.tsx.snap | 30 ++++++++ .../src/__tests__/useKeyProps.test.tsx | 37 +++++++--- .../interactive-hooks/src/useKeyProps.ts | 72 ++++++++++--------- .../src/useKeyProps.types.macos.ts | 6 +- .../src/useKeyProps.types.ts | 6 +- 6 files changed, 108 insertions(+), 50 deletions(-) create mode 100644 change/@fluentui-react-native-interactive-hooks-1a2b6bba-f91d-4116-9de7-9a46ac1d9b9b.json diff --git a/change/@fluentui-react-native-interactive-hooks-1a2b6bba-f91d-4116-9de7-9a46ac1d9b9b.json b/change/@fluentui-react-native-interactive-hooks-1a2b6bba-f91d-4116-9de7-9a46ac1d9b9b.json new file mode 100644 index 0000000000..bd2fc58aec --- /dev/null +++ b/change/@fluentui-react-native-interactive-hooks-1a2b6bba-f91d-4116-9de7-9a46ac1d9b9b.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fix useKeyProps for Windows + more followup", + "packageName": "@fluentui-react-native/interactive-hooks", + "email": "sanajmi@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/utils/interactive-hooks/src/__tests__/__snapshots__/useKeyProps.test.tsx.snap b/packages/utils/interactive-hooks/src/__tests__/__snapshots__/useKeyProps.test.tsx.snap index 37505ad221..78aed2b536 100644 --- a/packages/utils/interactive-hooks/src/__tests__/__snapshots__/useKeyProps.test.tsx.snap +++ b/packages/utils/interactive-hooks/src/__tests__/__snapshots__/useKeyProps.test.tsx.snap @@ -29,3 +29,33 @@ exports[`Pressable with useKeyProps 1`] = ` onStartShouldSetResponder={[Function]} /> `; + +exports[`useKeyProps called twice 1`] = ` + +`; diff --git a/packages/utils/interactive-hooks/src/__tests__/useKeyProps.test.tsx b/packages/utils/interactive-hooks/src/__tests__/useKeyProps.test.tsx index 7b31ea1942..3b5838f56c 100644 --- a/packages/utils/interactive-hooks/src/__tests__/useKeyProps.test.tsx +++ b/packages/utils/interactive-hooks/src/__tests__/useKeyProps.test.tsx @@ -15,23 +15,40 @@ const PressableWithDesktopProps = (props: PressablePropsExtended) => { }; it('Pressable with useKeyProps', () => { - const keyboardProps = useKeyProps(dummyFunction, ' ', 'Enter'); - const tree = renderer.create().toJSON(); + const TestComponent = () => { + const keyboardProps = useKeyProps(dummyFunction, ' ', 'Enter'); + return ; + }; + + const tree = renderer.create().toJSON(); expect(tree).toMatchSnapshot(); }); it('useKeyProps called twice', () => { - const keyboardProps1 = useKeyProps(dummyFunction, ' ', 'Enter'); - const keyboardProps2 = useKeyProps(dummyFunction, ' ', 'Enter'); - expect(keyboardProps1).toBe(keyboardProps2); + const TestComponent = () => { + const keyboardProps1 = useKeyProps(dummyFunction, ' ', 'Enter'); + const keyboardProps2 = useKeyProps(dummyFunction, ' ', 'Enter'); + expect(keyboardProps1).toBe(keyboardProps2); + return ; + }; + + const tree = renderer.create().toJSON(); + expect(tree).toMatchSnapshot(); }); -it('Simple Pressable with useKeyProps rendering does not invalidate styling', () => { - const keyboardProps = useKeyProps(dummyFunction, ' ', 'Enter'); - checkRenderConsistency(() => , 2); +it('Pressable with useKeyProps simple rendering does not invalidate styling', () => { + const TestComponent = () => { + const keyboardProps = useKeyProps(dummyFunction, ' ', 'Enter'); + return ; + }; + + checkRenderConsistency(() => , 2); }); it('Pressable with useKeyProps re-renders correctly', () => { - const keyboardProps = useKeyProps(dummyFunction, ' ', 'Enter'); - checkReRender(() => , 2); + const TestComponent = () => { + const keyboardProps = useKeyProps(dummyFunction, ' ', 'Enter'); + return ; + }; + checkReRender(() => , 2); }); diff --git a/packages/utils/interactive-hooks/src/useKeyProps.ts b/packages/utils/interactive-hooks/src/useKeyProps.ts index f08743df59..2409a183fd 100644 --- a/packages/utils/interactive-hooks/src/useKeyProps.ts +++ b/packages/utils/interactive-hooks/src/useKeyProps.ts @@ -22,28 +22,7 @@ export const isModifierKey = (nativeEvent: any): boolean => { ); }; -/** - * Re-usable hook for an onKeyDown event. - * @param userCallback The function you want to be called once the key has been activated on key up - * @param keys A string of the key you want to perform some action on. If undefined, always invokes userCallback - * @returns onKeyEvent() - Callback to determine if key was pressed, if so, call userCallback - * @deprecated use the hook `useKeyProps` instead - */ -export function useKeyCallback(userCallback?: KeyCallback, ...keys: string[]) { - const onKeyEvent = React.useCallback( - (e: KeyPressEvent) => { - if (userCallback !== undefined && (keys === undefined || keys.includes(e.nativeEvent.key))) { - userCallback(e); - e.stopPropagation(); - } - }, - [keys, userCallback], - ); - - return onKeyEvent; -} - -function getKeyCallbackWorker(userCallback?: KeyCallback, ...keys: string[]) { +function keyPressCallback(userCallback?: KeyCallback, ...keys: string[]) { const onKeyEvent = (e: KeyPressEvent) => { if (userCallback !== undefined && !isModifierKey(e.nativeEvent) && (keys === undefined || keys.includes(e.nativeEvent.key))) { userCallback(e); @@ -58,18 +37,20 @@ function getKeyUpPropsWorker(userCallback: KeyCallback, ...keys: string[]): KeyP ios: undefined, android: undefined, macos: { - onKeyUp: getKeyCallbackWorker(userCallback, ...keys), + onKeyUp: keyPressCallback(userCallback, ...keys), validKeysUp: keys, }, windows: { - onKeyUp: getKeyCallbackWorker(userCallback, ...keys), - keyUpEvents: keys.map((keyCode) => { - return { key: keyCode }; - }), + /** + * https://github.com/microsoft/react-native-windows/issues/11049 + * Windows doesn't filter on `key` but on `code`, which is quite different ('A' vs 'KeyA' or 'GamepadA'). + * While this discrepancy is present, let's not specify `keyUpEvents`. + */ + onKeyUp: keyPressCallback(userCallback, ...keys), }, // win32 default: { - onKeyUp: getKeyCallbackWorker(userCallback, ...keys), + onKeyUp: keyPressCallback(userCallback, ...keys), keyUpEvents: keys.map((keyCode) => { return { key: keyCode }; }), @@ -83,18 +64,20 @@ function getKeyDownPropsWorker(userCallback: KeyCallback, ...keys: string[]): Ke ios: undefined, android: undefined, macos: { - onKeyDown: getKeyCallbackWorker(userCallback, ...keys), + onKeyDown: keyPressCallback(userCallback, ...keys), validKeysDown: keys, }, windows: { - onKeyDown: getKeyCallbackWorker(userCallback, ...keys), - keyDownEvents: keys.map((keyCode) => { - return { key: keyCode }; - }), + /** + * https://github.com/microsoft/react-native-windows/issues/11049 + * Windows doesn't filter on `key` but on `code`, which is quite different ('A' vs 'KeyA' or 'GamepadA'). + * While this discrepancy is present, let's not specify `keyDownEvents`. + */ + onKeyDown: keyPressCallback(userCallback, ...keys), }, // win32 default: { - onKeyDown: getKeyCallbackWorker(userCallback, ...keys), + onKeyDown: keyPressCallback(userCallback, ...keys), keyDownEvents: keys.map((keyCode) => { return { key: keyCode }; }), @@ -132,3 +115,24 @@ export const preferKeyDownForKeyEvents = Platform.select({ * @returns KeyPressProps: An object containing the correct platform specific props to handle key press */ export const useKeyProps = preferKeyDownForKeyEvents ? useKeyDownProps : useKeyUpProps; + +/** + * Re-usable hook for an onKeyDown event. + * @param userCallback The function you want to be called once the key has been activated on key up + * @param keys A string of the key you want to perform some action on. If undefined, always invokes userCallback + * @returns onKeyEvent() - Callback to determine if key was pressed, if so, call userCallback + * @deprecated use the hook `useKeyProps` instead + */ +export function useKeyCallback(userCallback?: KeyCallback, ...keys: string[]) { + const onKeyEvent = React.useCallback( + (e: KeyPressEvent) => { + if (userCallback !== undefined && (keys === undefined || keys.includes(e.nativeEvent.key))) { + userCallback(e); + e.stopPropagation(); + } + }, + [keys, userCallback], + ); + + return onKeyEvent; +} diff --git a/packages/utils/interactive-hooks/src/useKeyProps.types.macos.ts b/packages/utils/interactive-hooks/src/useKeyProps.types.macos.ts index 0e08ea17fd..b3593d1835 100644 --- a/packages/utils/interactive-hooks/src/useKeyProps.types.macos.ts +++ b/packages/utils/interactive-hooks/src/useKeyProps.types.macos.ts @@ -25,9 +25,7 @@ export type KeyCallback = (e?: KeyPressEvent) => void; export type KeyPressProps = { onKeyDown?: KeyCallback; - validKeysDown?: string[]; // macOS - keyDownEvents?: any[]; // windows + validKeysDown?: string[]; onKeyUp?: KeyCallback; - validKeysUp?: string[]; // macOS - keyUpEvents?: any[]; // windows + validKeysUp?: string[]; }; diff --git a/packages/utils/interactive-hooks/src/useKeyProps.types.ts b/packages/utils/interactive-hooks/src/useKeyProps.types.ts index 8e80ac429c..92911c1715 100644 --- a/packages/utils/interactive-hooks/src/useKeyProps.types.ts +++ b/packages/utils/interactive-hooks/src/useKeyProps.types.ts @@ -6,7 +6,9 @@ export type KeyCallback = (e?: KeyPressEvent) => void; export type KeyPressProps = { onKeyDown?: KeyCallback; - validKeysDown?: string[]; + validKeysDown?: string[]; // macOS + keyDownEvents?: any[]; // windows onKeyUp?: KeyCallback; - validKeysUp?: string[]; + validKeysUp?: string[]; // macOS + keyUpEvents?: any[]; // windows };