chore: add more click logic and comments

coder · Parkreiner · Sep 25, 2023 · Sep 22, 2023 · Sep 22, 2023 · Sep 22, 2023
commit 04c1b5dfa10c1e5526b53b9b4d1f9b171653d81b
diff --git a/site/src/hooks/useClickable.ts b/site/src/hooks/useClickable.ts
@@ -1,6 +1,6 @@
 import {
+  type KeyboardEventHandler,
   type MouseEventHandler,
-  type KeyboardEvent,
   type RefObject,
   useRef,
 } from "react";
@@ -17,14 +17,21 @@ export interface UseClickableResult<
   tabIndex: 0;
   role: "button";
   onClick: MouseEventHandler<T>;
-  onKeyDown: (event: KeyboardEvent) => void;
+  onKeyDown: KeyboardEventHandler<T>;
 }
 
+/**
+ * Exposes props to add basic click/interactive behavior to HTML elements that
+ * don't traditionally have support for them.
+ */
 export const useClickable = <
   // T doesn't have a default type to make it more obvious that the hook expects
   // a type argument in order to work at all
   T extends ClickableElement,
 >(
+  // Even though onClick isn't used in any of the internal calculations, it's
+  // still a required argument, just to make sure that useClickable can't
+  // accidentally be called in a component without also defining click behavior
   onClick: MouseEventHandler<T>,
 ): UseClickableResult<T> => {
   const ref = useRef<T>(null);
@@ -34,12 +41,16 @@ export const useClickable = <
     tabIndex: 0,
     role: "button",
     onClick,
-    onKeyDown: (event: KeyboardEvent) => {
-      if (event.key === "Enter") {
+
+    // Most interactive elements already have this logic baked in automatically,
+    // but you explicitly have to add it for non-interactive elements
+    onKeyDown: (event) => {
+      if (event.key === "Enter" || event.key === "Space") {
         // Can't call onClick directly because onClick needs to work with an
         // event, and mouse events + keyboard events aren't compatible; wouldn't
         // have a value to pass in. Have to use a ref to simulate a click
         ref.current?.click();
+        event.stopPropagation();
       }
     },
   };