fix: Refactor hover previews to reduce false positives (#6091)

app/components/Editor.tsx

@@ -77,10 +77,13 @@ function Editor(props: Props, ref: React.RefObject<SharedEditor> | null) {
     React.useState<HTMLAnchorElement | null>(null);
   const previousCommentIds = React.useRef<string[]>();
 
-  const handleLinkActive = React.useCallback((element: HTMLAnchorElement) => {
-    setActiveLink(element);
-    return false;
-  }, []);
+  const handleLinkActive = React.useCallback(
+    (element: HTMLAnchorElement | null) => {
+      setActiveLink(element);
+      return false;
+    },
+    []
+  );
 
   const handleLinkInactive = React.useCallback(() => {
     setActiveLink(null);
@@ -351,7 +354,7 @@ function Editor(props: Props, ref: React.RefObject<SharedEditor> | null) {
             minHeight={props.editorStyle.paddingBottom}
           />
         )}
-        {activeLinkElement && !shareId && (
+        {!shareId && (
           <HoverPreview
             element={activeLinkElement}
             onClose={handleLinkInactive}

app/components/HoverPreview/HoverPreview.tsx

@@ -9,6 +9,7 @@ import useEventListener from "~/hooks/useEventListener";
 import useKeyDown from "~/hooks/useKeyDown";
 import useMobile from "~/hooks/useMobile";
 import useOnClickOutside from "~/hooks/useOnClickOutside";
+import usePrevious from "~/hooks/usePrevious";
 import useRequest from "~/hooks/useRequest";
 import useStores from "~/hooks/useStores";
 import { client } from "~/utils/ApiClient";
@@ -17,13 +18,14 @@ import HoverPreviewDocument from "./HoverPreviewDocument";
 import HoverPreviewLink from "./HoverPreviewLink";
 import HoverPreviewMention from "./HoverPreviewMention";
 
-const DELAY_OPEN = 500;
 const DELAY_CLOSE = 600;
+const POINTER_HEIGHT = 22;
+const POINTER_WIDTH = 22;
 
 type Props = {
-  /** The HTML element that is being hovered over */
-  element: HTMLAnchorElement;
-  /** A callback on close of the hover preview */
+  /** The HTML element that is being hovered over, or null if none. */
+  element: HTMLAnchorElement | null;
+  /** A callback on close of the hover preview. */
   onClose: () => void;
 };
 
@@ -32,16 +34,175 @@ enum Direction {
   DOWN,
 }
 
-const POINTER_HEIGHT = 22;
-const POINTER_WIDTH = 22;
-
-function HoverPreviewInternal({ element, onClose }: Props) {
-  const url = element.href || element.dataset.url;
+function HoverPreviewDesktop({ element, onClose }: Props) {
+  const url = element?.href || element?.dataset.url;
+  const previousUrl = usePrevious(url, true);
   const [isVisible, setVisible] = React.useState(false);
   const timerClose = React.useRef<ReturnType<typeof setTimeout>>();
-  const timerOpen = React.useRef<ReturnType<typeof setTimeout>>();
   const cardRef = React.useRef<HTMLDivElement>(null);
-  const stores = useStores();
+  const { cardLeft, cardTop, pointerLeft, pointerTop, pointerDir } =
+    useHoverPosition({
+      cardRef,
+      element,
+      isVisible,
+    });
+
+  const closePreview = React.useCallback(() => {
+    setVisible(false);
+    onClose();
+  }, [onClose]);
+
+  const stopCloseTimer = React.useCallback(() => {
+    if (timerClose.current) {
+      clearTimeout(timerClose.current);
+      timerClose.current = undefined;
+    }
+  }, []);
+
+  const startCloseTimer = React.useCallback(() => {
+    timerClose.current = setTimeout(closePreview, DELAY_CLOSE);
+  }, [closePreview]);
+
+  // Open and close the preview when the element changes.
+  React.useEffect(() => {
+    if (element) {
+      setVisible(true);
+    } else {
+      startCloseTimer();
+    }
+  }, [startCloseTimer, element]);
+
+  // Close the preview on Escape, scroll, or click outside.
+  useOnClickOutside(cardRef, closePreview);
+  useKeyDown("Escape", closePreview);
+  useEventListener("scroll", closePreview, window, { capture: true });
+
+  // Ensure that the preview stays open while the user is hovering over the card.
+  React.useEffect(() => {
+    const card = cardRef.current;
+
+    if (isVisible) {
+      if (card) {
+        card.addEventListener("mouseenter", stopCloseTimer);
+        card.addEventListener("mouseleave", startCloseTimer);
+      }
+    }
+
+    return () => {
+      if (card) {
+        card.removeEventListener("mouseenter", stopCloseTimer);
+        card.removeEventListener("mouseleave", startCloseTimer);
+      }
+
+      stopCloseTimer();
+    };
+  }, [element, startCloseTimer, isVisible, stopCloseTimer]);
+
+  const displayUrl = url ?? previousUrl;
+
+  if (!isVisible || !displayUrl) {
+    return null;
+  }
+
+  return (
+    <Portal>
+      <Position top={cardTop} left={cardLeft} ref={cardRef} aria-hidden>
+        <DataLoader url={displayUrl}>
+          {(data) => (
+            <Animate
+              initial={{ opacity: 0, y: -20, pointerEvents: "none" }}
+              animate={{ opacity: 1, y: 0, pointerEvents: "auto" }}
+            >
+              {data.type === UnfurlType.Mention ? (
+                <HoverPreviewMention
+                  url={data.thumbnailUrl}
+                  title={data.title}
+                  info={data.meta.info}
+                  color={data.meta.color}
+                />
+              ) : data.type === UnfurlType.Document ? (
+                <HoverPreviewDocument
+                  id={data.meta.id}
+                  url={data.url}
+                  title={data.title}
+                  description={data.description}
+                  info={data.meta.info}
+                />
+              ) : (
+                <HoverPreviewLink
+                  url={data.url}
+                  thumbnailUrl={data.thumbnailUrl}
+                  title={data.title}
+                  description={data.description}
+                />
+              )}
+              <Pointer
+                top={pointerTop}
+                left={pointerLeft}
+                direction={pointerDir}
+              />
+            </Animate>
+          )}
+        </DataLoader>
+      </Position>
+    </Portal>
+  );
+}
+
+function DataLoader({
+  url,
+  children,
+}: {
+  url: string;
+  children: (data: any) => React.ReactNode;
+}) {
+  const { ui } = useStores();
+  const { data, request, loading } = useRequest(
+    React.useCallback(
+      () =>
+        client.post("/urls.unfurl", {
+          url,
+          documentId: ui.activeDocumentId,
+        }),
+      [url, ui.activeDocumentId]
+    )
+  );
+
+  React.useEffect(() => {
+    if (url) {
+      void request();
+    }
+  }, [url, request]);
+
+  if (loading) {
+    return <LoadingIndicator />;
+  }
+
+  if (!data) {
+    return null;
+  }
+
+  return <>{children(data)}</>;
+}
+
+function HoverPreview({ element, ...rest }: Props) {
+  const isMobile = useMobile();
+  if (isMobile) {
+    return null;
+  }
+
+  return <HoverPreviewDesktop {...rest} element={element} />;
+}
+
+function useHoverPosition({
+  cardRef,
+  element,
+  isVisible,
+}: {
+  cardRef: React.RefObject<HTMLDivElement>;
+  element: HTMLAnchorElement | null;
+  isVisible: boolean;
+}) {
   const [cardLeft, setCardLeft] = React.useState(0);
   const [cardTop, setCardTop] = React.useState(0);
   const [pointerLeft, setPointerLeft] = React.useState(0);
@@ -49,7 +210,7 @@ function HoverPreviewInternal({ element, onClose }: Props) {
   const [pointerDir, setPointerDir] = React.useState(Direction.UP);
 
   React.useLayoutEffect(() => {
-    if (isVisible && cardRef.current) {
+    if (isVisible && element && cardRef.current) {
       const elem = element.getBoundingClientRect();
       const card = cardRef.current.getBoundingClientRect();
 
@@ -85,156 +246,9 @@ function HoverPreviewInternal({ element, onClose }: Props) {
       setCardLeft(cLeft);
       setPointerLeft(pLeft);
     }
-  }, [isVisible, element]);
+  }, [isVisible, cardRef, element]);
 
-  const { data, request, loading } = useRequest(
-    React.useCallback(
-      () =>
-        client.post("/urls.unfurl", {
-          url,
-          documentId: stores.ui.activeDocumentId,
-        }),
-      [url, stores.ui.activeDocumentId]
-    )
-  );
-
-  React.useEffect(() => {
-    if (url) {
-      stopOpenTimer();
-      setVisible(false);
-
-      void request();
-    }
-  }, [url, request]);
-
-  const stopOpenTimer = () => {
-    if (timerOpen.current) {
-      clearTimeout(timerOpen.current);
-      timerOpen.current = undefined;
-    }
-  };
-
-  const closePreview = React.useCallback(() => {
-    if (isVisible) {
-      stopOpenTimer();
-      setVisible(false);
-      onClose();
-    }
-  }, [isVisible, onClose]);
-
-  useOnClickOutside(cardRef, closePreview);
-  useKeyDown("Escape", closePreview);
-  useEventListener("scroll", closePreview, window, { capture: true });
-
-  const stopCloseTimer = React.useCallback(() => {
-    if (timerClose.current) {
-      clearTimeout(timerClose.current);
-      timerClose.current = undefined;
-    }
-  }, []);
-
-  const startOpenTimer = React.useCallback(() => {
-    if (!timerOpen.current) {
-      timerOpen.current = setTimeout(() => setVisible(true), DELAY_OPEN);
-    }
-  }, []);
-
-  const startCloseTimer = React.useCallback(() => {
-    stopOpenTimer();
-    timerClose.current = setTimeout(closePreview, DELAY_CLOSE);
-  }, [closePreview]);
-
-  React.useEffect(() => {
-    const card = cardRef.current;
-
-    if (data) {
-      startOpenTimer();
-
-      if (card) {
-        card.addEventListener("mouseenter", stopCloseTimer);
-        card.addEventListener("mouseleave", startCloseTimer);
-      }
-
-      element.addEventListener("mouseout", startCloseTimer);
-      element.addEventListener("mouseover", stopCloseTimer);
-      element.addEventListener("mouseover", startOpenTimer);
-    }
-
-    return () => {
-      element.removeEventListener("mouseout", startCloseTimer);
-      element.removeEventListener("mouseover", stopCloseTimer);
-      element.removeEventListener("mouseover", startOpenTimer);
-
-      if (card) {
-        card.removeEventListener("mouseenter", stopCloseTimer);
-        card.removeEventListener("mouseleave", startCloseTimer);
-      }
-
-      stopCloseTimer();
-    };
-  }, [element, startCloseTimer, data, startOpenTimer, stopCloseTimer]);
-
-  if (loading) {
-    return <LoadingIndicator />;
-  }
-
-  if (!data) {
-    return null;
-  }
-
-  return (
-    <Portal>
-      <Position top={cardTop} left={cardLeft} aria-hidden>
-        {isVisible ? (
-          <Animate
-            initial={{ opacity: 0, y: -20, pointerEvents: "none" }}
-            animate={{ opacity: 1, y: 0, pointerEvents: "auto" }}
-          >
-            {data.type === UnfurlType.Mention ? (
-              <HoverPreviewMention
-                ref={cardRef}
-                url={data.thumbnailUrl}
-                title={data.title}
-                info={data.meta.info}
-                color={data.meta.color}
-              />
-            ) : data.type === UnfurlType.Document ? (
-              <HoverPreviewDocument
-                ref={cardRef}
-                id={data.meta.id}
-                url={data.url}
-                title={data.title}
-                description={data.description}
-                info={data.meta.info}
-              />
-            ) : (
-              <HoverPreviewLink
-                ref={cardRef}
-                url={data.url}
-                thumbnailUrl={data.thumbnailUrl}
-                title={data.title}
-                description={data.description}
-              />
-            )}
-            <Pointer
-              top={pointerTop}
-              left={pointerLeft}
-              direction={pointerDir}
-            />
-          </Animate>
-        ) : null}
-      </Position>
-    </Portal>
-  );
-}
-
-function HoverPreview({ element, ...rest }: Props) {
-  const isMobile = useMobile();
-  if (isMobile) {
-    return null;
-  }
-
-  return <HoverPreviewInternal {...rest} element={element} />;
+  return { cardLeft, cardTop, pointerLeft, pointerTop, pointerDir };
 }
 
 const Animate = styled(m.div)`

app/editor/index.tsx

@@ -124,7 +124,7 @@ export type Props = {
     event: MouseEvent | React.MouseEvent<HTMLButtonElement>
   ) => void;
   /** Callback when user hovers on any link in the document */
-  onHoverLink?: (element: HTMLAnchorElement) => boolean;
+  onHoverLink?: (element: HTMLAnchorElement | null) => boolean;
   /** Callback when user presses any key with document focused */
   onKeyDown?: (event: React.KeyboardEvent<HTMLDivElement>) => void;
   /** Collection of embed types to render in the document */

app/hooks/usePrevious.ts

@@ -1,9 +1,19 @@
 import * as React from "react";
 
-export default function usePrevious<T>(value: T): T | void {
+/**
+ * A hook to get the previous value of a variable.
+ *
+ * @param value The value to track.
+ * @param onlyTruthy Whether to include only truthy values.
+ * @returns The previous value of the variable.
+ */
+export default function usePrevious<T>(value: T, onlyTruthy = false): T | void {
   const ref = React.useRef<T>();
 
   React.useEffect(() => {
+    if (onlyTruthy && !value) {
+      return;
+    }
     ref.current = value;
   });
 

app/hooks/useRequest.ts

@@ -16,7 +16,7 @@ type RequestResponse<T> = {
  * A hook to make an API request and track its state within a component.
  *
  * @param requestFn The function to call to make the request, it should return a promise.
- * @returns
+ * @returns An object containing the request state and a function to start the request.
  */
 export default function useRequest<T = unknown>(
   requestFn: () => Promise<T>