fix(util): Use a much simpler hotkeys implementation (#35554)

static/app/utils/useHotkeys.tsx

@@ -1,76 +1,59 @@
-import {useCallback, useEffect, useMemo, useRef} from 'react';
+import {useCallback, useEffect, useMemo} from 'react';
 
 import {getKeyCode} from './getKeyCode';
 
-const modifierKeys = [getKeyCode('shift'), getKeyCode('alt'), getKeyCode('ctrl')];
+const isKeyPressed = (key: string, evt: KeyboardEvent): boolean => {
+  const keyCode = getKeyCode(key);
+  switch (keyCode) {
+    case getKeyCode('command'):
+      return evt.metaKey;
+    case getKeyCode('shift'):
+      return evt.shiftKey;
+    case getKeyCode('ctrl'):
+      return evt.ctrlKey;
+    case getKeyCode('alt'):
+      return evt.altKey;
+    default:
+      return keyCode === evt.keyCode;
+  }
+};
 
 /**
  * Pass in the hotkey combinations under match and the corresponding callback function to be called.
  * Separate key names with +. For example, 'command+alt+shift+x'
  * Alternate matchings as an array: ['command+alt+backspace', 'ctrl+alt+delete']
+ *
+ * Note: you can only use one non-modifier (keys other than shift, ctrl, alt, command) key at a time.
  */
 export function useHotkeys(
   hotkeys: {callback: (e: KeyboardEvent) => void; match: string[] | string}[],
   deps: React.DependencyList
 ): void {
-  const keysPressedRef = useRef<Set<number>>(new Set());
-
   // eslint-disable-next-line react-hooks/exhaustive-deps
   const memoizedHotkeys = useMemo(() => hotkeys, deps);
 
   const onKeyDown = useCallback(
-    (e: KeyboardEvent) => {
-      const removeKey = keyCode => {
-        keysPressedRef.current.delete(keyCode);
-      };
-
-      if (e.type === 'keydown') {
-        keysPressedRef.current.add(e.keyCode);
-
-        if (e.metaKey && !modifierKeys.includes(e.keyCode)) {
-          /*
-          If command/metaKey is held, keyup does not get called for non-modifier keys. See:
-          https://web.archive.org/web/20160304022453/http://bitspushedaround.com/on-a-few-things-you-may-not-know-about-the-hellish-command-key-and-javascript-events/
-
-          So, if the metaKey is held, we just have it remove the key after a set timeout, this allows you to hold the command key down
-          and press the other key again after the timeout removes the key.
-        */
-          setTimeout(() => {
-            removeKey(e.keyCode);
-          }, 500);
-        }
-
-        for (const hotkey of memoizedHotkeys) {
-          const matches = (
-            Array.isArray(hotkey.match) ? hotkey.match : [hotkey.match]
-          ).map(o => o.trim().split('+'));
-
-          for (const keys of matches) {
-            if (
-              keys.length === keysPressedRef.current.size &&
-              keys.every(key => keysPressedRef.current.has(getKeyCode(key)))
-            ) {
-              hotkey.callback(e);
-              break;
-            }
+    (evt: KeyboardEvent) => {
+      for (const set of memoizedHotkeys) {
+        const keysets = Array.isArray(set.match) ? set.match : [set.match];
+        for (const keyset of keysets) {
+          const keys = keyset.split('+');
+
+          if (keys.every(key => isKeyPressed(key, evt))) {
+            set.callback(evt);
+            return;
           }
         }
       }
-
-      if (e.type === 'keyup') {
-        removeKey(e.keyCode);
-      }
     },
     [memoizedHotkeys]
   );
 
   useEffect(() => {
     document.addEventListener('keydown', onKeyDown);
-    document.addEventListener('keyup', onKeyDown);
 
     return () => {
       document.removeEventListener('keydown', onKeyDown);
-      document.removeEventListener('keyup', onKeyDown);
     };
   }, [onKeyDown]);
 }

tests/js/spec/utils/useHotkeys.spec.jsx

@@ -0,0 +1,145 @@
+import {reactHooks} from 'sentry-test/reactTestingLibrary';
+
+import {getKeyCode} from 'sentry/utils/getKeyCode';
+import {useHotkeys} from 'sentry/utils/useHotkeys';
+
+describe('useHotkeys', function () {
+  let events = {};
+
+  beforeEach(() => {
+    // Empty our events before each test case
+    events = {};
+
+    // Define the addEventListener method with a Jest mock function
+    document.addEventListener = jest.fn((event, callback) => {
+      events[event] = callback;
+    });
+
+    document.removeEventListener = jest.fn(event => {
+      delete events[event];
+    });
+  });
+
+  it('simple match', function () {
+    let didGetCalled = false;
+
+    expect(events.keydown).toBeUndefined();
+
+    reactHooks.renderHook(() =>
+      useHotkeys([
+        {
+          match: 'ctrl+s',
+          callback: () => {
+            didGetCalled = true;
+          },
+        },
+      ])
+    );
+
+    expect(events.keydown).toBeDefined();
+    expect(didGetCalled).toEqual(false);
+
+    events.keydown({keyCode: getKeyCode('s'), ctrlKey: true});
+
+    expect(didGetCalled).toEqual(true);
+  });
+
+  it('multi match', function () {
+    let didGetCalled = false;
+
+    expect(events.keydown).toBeUndefined();
+
+    reactHooks.renderHook(() =>
+      useHotkeys([
+        {
+          match: ['ctrl+s', 'cmd+m'],
+          callback: () => {
+            didGetCalled = true;
+          },
+        },
+      ])
+    );
+
+    expect(events.keydown).toBeDefined();
+    expect(didGetCalled).toEqual(false);
+
+    events.keydown({keyCode: getKeyCode('s'), ctrlKey: true});
+
+    expect(didGetCalled).toEqual(true);
+
+    didGetCalled = false;
+
+    events.keydown({keyCode: getKeyCode('m'), metaKey: true});
+
+    expect(didGetCalled).toEqual(true);
+  });
+
+  it('complex match', function () {
+    let didGetCalled = false;
+
+    expect(events.keydown).toBeUndefined();
+
+    reactHooks.renderHook(() =>
+      useHotkeys([
+        {
+          match: ['cmd+control+option+shift+x'],
+          callback: () => {
+            didGetCalled = true;
+          },
+        },
+      ])
+    );
+
+    expect(events.keydown).toBeDefined();
+    expect(didGetCalled).toEqual(false);
+
+    events.keydown({
+      keyCode: getKeyCode('x'),
+      altKey: true,
+      metaKey: true,
+      shiftKey: true,
+      ctrlKey: true,
+    });
+
+    expect(didGetCalled).toEqual(true);
+  });
+
+  it('rerender', function () {
+    let didGetCalled = false;
+
+    expect(events.keydown).toBeUndefined();
+
+    const {rerender} = reactHooks.renderHook(
+      p =>
+        useHotkeys([
+          {
+            match: p.match,
+            callback: () => {
+              didGetCalled = true;
+            },
+          },
+        ]),
+      {
+        initialProps: {
+          match: 'ctrl+s',
+        },
+      }
+    );
+
+    expect(events.keydown).toBeDefined();
+    expect(didGetCalled).toEqual(false);
+
+    events.keydown({keyCode: getKeyCode('s'), ctrlKey: true});
+
+    expect(didGetCalled).toEqual(true);
+    didGetCalled = false;
+
+    rerender({match: 'cmd+m'});
+
+    events.keydown({keyCode: getKeyCode('s'), ctrlKey: true});
+    expect(didGetCalled).toEqual(false);
+
+    events.keydown({keyCode: getKeyCode('m'), metaKey: true});
+    expect(didGetCalled).toEqual(true);
+  });
+});