Merge pull request #875 from JohannesKlauss/fix/853

Fix rerender when using HotkeysProvider and a bug in removing bound h…

src/HotkeysProvider.tsx

@@ -1,6 +1,7 @@
 import { Hotkey } from './types'
-import { createContext, ReactNode, useMemo, useState, useContext } from 'react'
+import { createContext, ReactNode, useState, useContext, useCallback } from 'react'
 import BoundHotkeysProxyProviderProvider from './BoundHotkeysProxyProvider'
+import deepEqual from './deepEqual'
 
 export type HotkeysContextType = {
   hotkeys: ReadonlyArray<Hotkey>
@@ -32,41 +33,51 @@ export const HotkeysProvider = ({initiallyActiveScopes = ['*'], children}: Props
   const [internalActiveScopes, setInternalActiveScopes] = useState(initiallyActiveScopes?.length > 0 ? initiallyActiveScopes : ['*'])
   const [boundHotkeys, setBoundHotkeys] = useState<Hotkey[]>([]);
 
-  const isAllActive = useMemo(() => internalActiveScopes.includes('*'), [internalActiveScopes])
+  const enableScope = useCallback((scope: string) => {
+    setInternalActiveScopes((prev) => {
+      if (prev.includes('*')) {
+        return [scope]
+      }
 
-  const enableScope = (scope: string) => {
-    if (isAllActive) {
-      setInternalActiveScopes([scope])
-    } else {
-      setInternalActiveScopes(Array.from(new Set([...internalActiveScopes, scope])))
-    }
-  }
+      return Array.from(new Set([...prev, scope]))
+    })
+  }, [])
 
-  const disableScope = (scope: string) => {
-    const scopes = internalActiveScopes.filter(s => s !== scope)
+  const disableScope = useCallback((scope: string) => {
+    setInternalActiveScopes((prev) => {
+      if (prev.filter(s => s !== scope).length === 0) {
+        return ['*']
+      } else {
+        return prev.filter(s => s !== scope)
+      }
+    })
+  }, [])
 
-    if (scopes.length === 0) {
-      setInternalActiveScopes(['*'])
-    } else {
-      setInternalActiveScopes(scopes)
-    }
-  }
+  const toggleScope = useCallback((scope: string) => {
+    setInternalActiveScopes((prev) => {
+      if (prev.includes(scope)) {
+        if (prev.filter(s => s !== scope).length === 0) {
+          return ['*']
+        } else {
+          return prev.filter(s => s !== scope)
+        }
+      } else {
+        if (prev.includes('*')) {
+          return [scope]
+        }
 
-  const toggleScope = (scope: string) => {
-    if (internalActiveScopes.includes(scope)) {
-      disableScope(scope)
-    } else {
-      enableScope(scope)
-    }
-  }
+        return Array.from(new Set([...prev, scope]))
+      }
+    })
+  }, [])
 
-  const addBoundHotkey = (hotkey: Hotkey) => {
-    setBoundHotkeys([...boundHotkeys, hotkey])
-  }
+  const addBoundHotkey = useCallback((hotkey: Hotkey) => {
+    setBoundHotkeys((prev) => [...prev, hotkey])
+  }, [])
 
-  const removeBoundHotkey = (hotkey: Hotkey) => {
-    setBoundHotkeys(boundHotkeys.filter(h => h.keys !== hotkey.keys))
-  }
+  const removeBoundHotkey = useCallback((hotkey: Hotkey) => {
+    setBoundHotkeys((prev) => prev.filter(h => !deepEqual(h, hotkey)))
+  }, [])
 
   return (
     <HotkeysContext.Provider value={{enabledScopes: internalActiveScopes, hotkeys: boundHotkeys, enableScope, disableScope, toggleScope}}>

src/useHotkeys.ts

@@ -51,7 +51,7 @@ export default function useHotkeys<T extends HTMLElement>(
       }
 
       // TODO: SINCE THE EVENT IS NOW ATTACHED TO THE REF, THE ACTIVE ELEMENT CAN NEVER BE INSIDE THE REF. THE HOTKEY ONLY TRIGGERS IF THE
-      // REF IS THE ACTIVE ELEMENT. THIS IS A PROBLEM SINCE FOCUSED SUB COMPONENTS WONT TRIGGER THE HOTKEY.
+      // REF IS THE ACTIVE ELEMENT. THIS IS A PROBLEM SINCE FOCUSED SUB COMPONENTS WON'T TRIGGER THE HOTKEY.
       if (ref.current !== null && document.activeElement !== ref.current && !ref.current.contains(document.activeElement)) {
         stopPropagation(e)
 

tests/HotkeysProvider.test.tsx

@@ -182,8 +182,14 @@ test('should update bound hotkeys when useHotkeys changes its scopes', () => {
     return useHotkeysContext()
   }
 
-  const wrapper = ({ children }: { children: ReactNode }) => <HotkeysProvider
-    initiallyActiveScopes={['foo']}>{children}</HotkeysProvider>
+  const wrapper = ({ children }: { children: ReactNode }) => {
+    return (
+      <HotkeysProvider initiallyActiveScopes={['foo']}>
+        {children}
+      </HotkeysProvider>
+    )
+  }
+
   const { result, rerender } = renderHook<{ scopes: string[] }, HotkeysContextType>(({ scopes }) => useIntegratedHotkeys(scopes), {
     // @ts-ignore
     wrapper,