diff --git a/src/types.ts b/src/types.ts index a5181c29..9938d8bd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -43,3 +43,5 @@ export type Options = { } export type OptionsOrDependencyArray = Options | DependencyList + +export type KeyboardEventHandler = (event: KeyboardEvent) => any diff --git a/src/useHotkeys.ts b/src/useHotkeys.ts index 84197d56..fc69566d 100644 --- a/src/useHotkeys.ts +++ b/src/useHotkeys.ts @@ -1,5 +1,5 @@ -import { HotkeyCallback, Keys, Options, OptionsOrDependencyArray, RefType } from './types' -import { DependencyList, useCallback, useEffect, useLayoutEffect, useRef } from 'react' +import { HotkeyCallback, KeyboardEventHandler, Keys, Options, OptionsOrDependencyArray, RefType } from './types' +import { DependencyList, RefCallback, useCallback, useEffect, useLayoutEffect, useRef } from 'react' import { mapKey, parseHotkey, parseKeysHookInput } from './parseHotkeys' import { isHotkeyEnabled, @@ -31,6 +31,11 @@ export default function useHotkeys( const ref = useRef>(null) const hasTriggeredRef = useRef(false) + const handleKeyUpRef = useRef() + const handleKeyDownRef = useRef() + const stableHandleKeyUpRef = useRef((...args) => handleKeyUpRef.current?.(...args)) + const stableHandleKeyDownRef = useRef((...args) => handleKeyDownRef.current?.(...args)) + const _options: Options | undefined = !(options instanceof Array) ? (options as Options) : !(dependencies instanceof Array) @@ -140,12 +145,8 @@ export default function useHotkeys( } } - const domNode = ref.current || _options?.document || document - - // @ts-ignore - domNode.addEventListener('keyup', handleKeyUp) - // @ts-ignore - domNode.addEventListener('keydown', handleKeyDown) + handleKeyUpRef.current = handleKeyUp + handleKeyDownRef.current = handleKeyDown if (proxy) { parseKeysHookInput(_keys, memoisedOptions?.splitKey).forEach((key) => @@ -154,10 +155,8 @@ export default function useHotkeys( } return () => { - // @ts-ignore - domNode.removeEventListener('keyup', handleKeyUp) - // @ts-ignore - domNode.removeEventListener('keydown', handleKeyDown) + handleKeyUpRef.current = undefined + handleKeyDownRef.current = undefined if (proxy) { parseKeysHookInput(_keys, memoisedOptions?.splitKey).forEach((key) => @@ -167,5 +166,34 @@ export default function useHotkeys( } }, [_keys, memoisedOptions, enabledScopes]) - return ref + useEffect(() => { + // This effect is needed to attach event listeners to something when the returned ref is not used. + // and to clean up when the component using the hook unmounts. + const domNode = (ref.current || memoisedOptions?.document || document) as T + + domNode?.addEventListener('keyup', stableHandleKeyUpRef.current) + domNode?.addEventListener('keydown', stableHandleKeyDownRef.current) + + return () => { + ref.current?.removeEventListener('keyup', stableHandleKeyUpRef.current) + ref.current?.removeEventListener('keydown', stableHandleKeyDownRef.current) + } + }, [memoisedOptions?.document]) + + const refCallback: RefCallback = useCallback((element) => { + const domNode = (element || memoisedOptions?.document || document) as T + + // Cleanup old event handlers + ref.current?.removeEventListener('keyup', stableHandleKeyUpRef.current) + ref.current?.removeEventListener('keydown', stableHandleKeyDownRef.current) + + // Update refObject + ref.current = domNode + + // Re-attach handlers to the new element + domNode?.addEventListener('keyup', stableHandleKeyUpRef.current) + domNode?.addEventListener('keydown', stableHandleKeyDownRef.current) + }, []) + + return refCallback } diff --git a/tests/useHotkeys.test.tsx b/tests/useHotkeys.test.tsx index f3fdf115..b9920747 100644 --- a/tests/useHotkeys.test.tsx +++ b/tests/useHotkeys.test.tsx @@ -4,9 +4,9 @@ import { FormTags, HotkeyCallback, Keys, Options } from '../src/types' import { DependencyList, JSXElementConstructor, - MutableRefObject, ReactElement, ReactNode, + Ref, useCallback, useState, } from 'react' @@ -372,7 +372,7 @@ test('should reflect set splitKey character', async () => { const user = userEvent.setup() const callback = jest.fn() - const { rerender } = renderHook, HookParameters>( + const { rerender } = renderHook, HookParameters>( ({ keys, options }) => useHotkeys(keys, callback, options), { initialProps: { keys: 'a, b', options: undefined }, @@ -774,6 +774,44 @@ test('should only trigger when the element is focused if a ref is set', async () expect(callback).toHaveBeenCalledTimes(1) }) +test('should trigger when the ref is re-attached to another element', async () => { + const user = userEvent.setup() + const callback = jest.fn() + + const Component = ({ cb }: { cb: HotkeyCallback }) => { + const ref = useHotkeys('a', cb) + const [toggle, setToggle] = useState(false); + + if(toggle) { + return ( + + + + + ) + } + + return ( +
+ + +
+ ) + } + + const { getByTestId } = render() + + await user.keyboard('A') + + expect(callback).not.toHaveBeenCalled() + + await user.click(getByTestId('toggle')) + await user.click(getByTestId('div')) + await user.keyboard('A') + + expect(callback).toHaveBeenCalledTimes(1) +}) + test.skip('should preventDefault and stop propagation when ref is not focused', async () => { const callback = jest.fn()