From 58499cefb33fe2e44ba7cec3eb0782e26be86658 Mon Sep 17 00:00:00 2001
From: Andrew Bone <AndrewB05@gmail.com>
Date: Wed, 24 Mar 2021 14:03:45 +0000
Subject: [PATCH] Update shortcut code with best practices (#13026)

* Update with best practices

* Remove useCallback

* Update app/javascript/shared/components/useKeyboardShortcuts.js

Co-authored-by: Suzanne Aitchison <suzanne@forem.com>

* Fill out JSDoc

* Follow Prettier rules

* Resolve conflict

Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
---
 .../shared/components/useKeyboardShortcuts.js | 97 ++++++++++---------
 1 file changed, 49 insertions(+), 48 deletions(-)

diff --git a/app/javascript/shared/components/useKeyboardShortcuts.js b/app/javascript/shared/components/useKeyboardShortcuts.js
index 8ac2c32f0..28e40a626 100644
--- a/app/javascript/shared/components/useKeyboardShortcuts.js
+++ b/app/javascript/shared/components/useKeyboardShortcuts.js
@@ -1,4 +1,4 @@
-import { useState, useEffect, useCallback } from 'preact/hooks';
+import { useState, useEffect } from 'preact/hooks';
 import PropTypes from 'prop-types';
 
 /**
@@ -25,6 +25,37 @@ function isFormField(element) {
   );
 }
 
+/**
+ * Function to handle converting key presses to callback functions
+ * 
+ * @param {KeyboardEvent} e Keyboard event
+ * @param {String} keys special keys formatted in a string
+ * @param {Array} chain array of past keys
+ * @param {Object} shortcuts object containing callback functions
+ *
+ * @returns {Array} New chain
+ */
+const callShortcut = (e, keys, chain, shortcuts) => {
+  const shortcut =
+    chain && chain.length > 0
+      ? shortcuts[`${chain.join('~')}~${e.code}`]
+      : shortcuts[`${keys}${e.code}`] ||
+        shortcuts[`${keys}${e.key.toLowerCase()}`];
+
+  // if a valid shortcut is found call it and reset the chain
+  if (shortcut) {
+    shortcut(e);
+    return [];
+  }
+
+  // if we have keys don't add to the chain
+  if (keys || e.key === 'Shift') {
+    return [];
+  }
+
+  return [...chain, e.code];
+};
+
 // Default options to be used if null
 const defaultOptions = {
   timeout: 0, // The default is zero as we want no delays between keystrokes by default.
@@ -36,15 +67,15 @@ const defaultOptions = {
  *
  * @example
  * const shortcuts = {
- *   "ctrl+alt+KeyG": (e) => {
+ *   'ctrl+alt+KeyG': (e) => {
  *     e.preventDefault();
- *     alert("Control Alt G has been pressed");
+ *     alert('Control Alt G has been pressed');
  *   },
- *   "KeyG~KeyH": (e) => {
+ *   'KeyG~KeyH': (e) => {
  *     e.preventDefault();
- *     alert("G has been pressed quickly followed by H");
+ *     alert('G has been pressed quickly followed by H');
  *   },
- *   "?": (e) => {
+ *   '?': (e) => {
  *     setIsHelpVisible(true);
  *   }
  * }
@@ -61,34 +92,13 @@ export function useKeyboardShortcuts(
   eventTarget = window,
   options = {},
 ) {
+  const [storedShortcuts] = useState(shortcuts);
   const [keyChain, setKeyChain] = useState([]);
-  const [keyChainQueue, setKeyChainQueue] = useState(null);
   const [mergedOptions, setMergedOptions] = useState({
     ...defaultOptions,
-    ...options,
+    ...options
   });
 
-  // Work out the correct shortcut for the key press
-  const callShortcut = useCallback(
-    (e, keys) => {
-      let shortcut;
-      if (keyChain.length > 0) {
-        shortcut = shortcuts[`${keyChain.join('~')}~${e.code}`];
-      } else {
-        shortcut =
-          shortcuts[`${keys}${e.code}`] ||
-          shortcuts[`${keys}${e.key.toLowerCase()}`];
-      }
-
-      // if a valid shortcut is found call it and reset the chain
-      if (shortcut) {
-        shortcut(e);
-        setKeyChain([]);
-      }
-    },
-    [shortcuts, keyChain],
-  );
-
   // update mergedOptions if options prop changes
   useEffect(() => {
     const newOptions = {};
@@ -97,26 +107,21 @@ export function useKeyboardShortcuts(
     setMergedOptions({ ...defaultOptions, ...newOptions });
   }, [options.timeout]);
 
-  // Set up key chains
+  // clear key chain after timeout is reached
   useEffect(() => {
-    if (!keyChainQueue && keyChain.length === 0) return;
+    if (keyChain.length <= 0) return;
 
     const timeout = window.setTimeout(() => {
       clearTimeout(timeout);
       setKeyChain([]);
     }, mergedOptions.timeout);
 
-    if (keyChainQueue) {
-      setKeyChain([...keyChain, keyChainQueue]);
-      setKeyChainQueue(null);
-    }
-
     return () => clearTimeout(timeout);
-  }, [keyChain, keyChainQueue, mergedOptions.timeout]);
+  }, [keyChain.length, mergedOptions.timeout]);
 
   // set up event listeners
   useEffect(() => {
-    if (!shortcuts || Object.keys(shortcuts).length === 0) return;
+    if (!storedShortcuts || Object.keys(storedShortcuts).length === 0) return;
 
     const keyEvent = (e) => {
       if (e.defaultPrevented) return;
@@ -129,20 +134,16 @@ export function useKeyboardShortcuts(
       // If no special keys, except shift, are pressed and focus is inside a field return
       if (e.target instanceof Node && isFormField(e.target) && !keys) return;
 
-      // If a special key is pressed reset the key chain else add to the chain
-      if (keys) {
-        setKeyChain([]);
-      } else {
-        setKeyChainQueue(e.code);
-      }
+      const newChain = callShortcut(e, keys, keyChain, storedShortcuts);
 
-      callShortcut(e, keys);
+      // update keychain with latest chain
+      setKeyChain(newChain);
     };
 
     eventTarget.addEventListener('keydown', keyEvent);
 
     return () => eventTarget.removeEventListener('keydown', keyEvent);
-  }, [shortcuts, eventTarget, callShortcut]);
+  }, [keyChain, storedShortcuts, eventTarget]);
 }
 
 /**
@@ -151,9 +152,9 @@ export function useKeyboardShortcuts(
  *
  * @example
  * const shortcuts = {
- *   "ctrl+alt+KeyG": (e) => {
+ *   'ctrl+alt+KeyG': (e) => {
  *     e.preventDefault();
- *     alert("Control Alt G has been pressed")
+ *     alert('Control Alt G has been pressed')
  *   }
  * }
  *