From 6b13a322342f91ffa9a7af8f9b5c4d60d352bece Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 29 Oct 2023 18:31:12 -0400 Subject: [PATCH] fix: Refactor hover previews to reduce false positives (#6091) --- app/components/Editor.tsx | 13 +- app/components/HoverPreview/HoverPreview.tsx | 336 ++++++++++--------- app/editor/index.tsx | 2 +- app/hooks/usePrevious.ts | 12 +- app/hooks/useRequest.ts | 2 +- shared/editor/extensions/HoverPreviews.ts | 64 ++++ shared/editor/marks/Link.tsx | 16 +- shared/editor/nodes/Mention.ts | 30 +- shared/editor/nodes/index.ts | 2 + 9 files changed, 265 insertions(+), 212 deletions(-) create mode 100644 shared/editor/extensions/HoverPreviews.ts diff --git a/app/components/Editor.tsx b/app/components/Editor.tsx index 4c0dafa58..c5f3e41bd 100644 --- a/app/components/Editor.tsx +++ b/app/components/Editor.tsx @@ -77,10 +77,13 @@ function Editor(props: Props, ref: React.RefObject | null) { React.useState(null); const previousCommentIds = React.useRef(); - 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 | null) { minHeight={props.editorStyle.paddingBottom} /> )} - {activeLinkElement && !shareId && ( + {!shareId && ( 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>(); - const timerOpen = React.useRef>(); const cardRef = React.useRef(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 ( + + + + {(data) => ( + + {data.type === UnfurlType.Mention ? ( + + ) : data.type === UnfurlType.Document ? ( + + ) : ( + + )} + + + )} + + + + ); +} + +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 ; + } + + if (!data) { + return null; + } + + return <>{children(data)}; +} + +function HoverPreview({ element, ...rest }: Props) { + const isMobile = useMobile(); + if (isMobile) { + return null; + } + + return ; +} + +function useHoverPosition({ + cardRef, + element, + isVisible, +}: { + cardRef: React.RefObject; + 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 ; - } - - if (!data) { - return null; - } - - return ( - - - {isVisible ? ( - - {data.type === UnfurlType.Mention ? ( - - ) : data.type === UnfurlType.Document ? ( - - ) : ( - - )} - - - ) : null} - - - ); -} - -function HoverPreview({ element, ...rest }: Props) { - const isMobile = useMobile(); - if (isMobile) { - return null; - } - - return ; + return { cardLeft, cardTop, pointerLeft, pointerTop, pointerDir }; } const Animate = styled(m.div)` diff --git a/app/editor/index.tsx b/app/editor/index.tsx index 08bc8dcca..9bd1218a9 100644 --- a/app/editor/index.tsx +++ b/app/editor/index.tsx @@ -124,7 +124,7 @@ export type Props = { event: MouseEvent | React.MouseEvent ) => 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) => void; /** Collection of embed types to render in the document */ diff --git a/app/hooks/usePrevious.ts b/app/hooks/usePrevious.ts index 4dbc6a474..6e395ab3c 100644 --- a/app/hooks/usePrevious.ts +++ b/app/hooks/usePrevious.ts @@ -1,9 +1,19 @@ import * as React from "react"; -export default function usePrevious(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(value: T, onlyTruthy = false): T | void { const ref = React.useRef(); React.useEffect(() => { + if (onlyTruthy && !value) { + return; + } ref.current = value; }); diff --git a/app/hooks/useRequest.ts b/app/hooks/useRequest.ts index e8af4b1cd..603cd4493 100644 --- a/app/hooks/useRequest.ts +++ b/app/hooks/useRequest.ts @@ -16,7 +16,7 @@ type RequestResponse = { * 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( requestFn: () => Promise diff --git a/shared/editor/extensions/HoverPreviews.ts b/shared/editor/extensions/HoverPreviews.ts new file mode 100644 index 000000000..c3ba34ed2 --- /dev/null +++ b/shared/editor/extensions/HoverPreviews.ts @@ -0,0 +1,64 @@ +import { Plugin } from "prosemirror-state"; +import { EditorView } from "prosemirror-view"; +import Extension from "../lib/Extension"; + +interface HoverPreviewsOptions { + /** Callback when a hover target is found or lost. */ + onHoverLink?: (target: Element | null) => void; + + /** Delay before the target is considered "hovered" and callback is triggered. */ + delay: number; +} + +export default class HoverPreviews extends Extension { + get defaultOptions(): HoverPreviewsOptions { + return { + delay: 500, + }; + } + + get name() { + return "hover-previews"; + } + + get plugins() { + const isHoverTarget = (target: Element | null, view: EditorView) => + target instanceof HTMLElement && + this.editor.elementRef.current?.contains(target) && + (!view.editable || (view.editable && !view.hasFocus())); + + let hoveringTimeout: ReturnType; + + return [ + new Plugin({ + props: { + handleDOMEvents: { + mouseover: (view: EditorView, event: MouseEvent) => { + const target = (event.target as HTMLElement)?.closest( + ".use-hover-preview" + ); + if (isHoverTarget(target, view)) { + if (this.options.onHoverLink) { + hoveringTimeout = setTimeout(() => { + this.options.onHoverLink?.(target); + }, this.options.delay); + } + } + return false; + }, + mouseout: (view: EditorView, event: MouseEvent) => { + const target = (event.target as HTMLElement)?.closest( + ".use-hover-preview" + ); + if (isHoverTarget(target, view)) { + clearTimeout(hoveringTimeout); + this.options.onHoverLink?.(null); + } + return false; + }, + }, + }, + }), + ]; + } +} diff --git a/shared/editor/marks/Link.tsx b/shared/editor/marks/Link.tsx index ecfdfdd0a..59bbbfa9b 100644 --- a/shared/editor/marks/Link.tsx +++ b/shared/editor/marks/Link.tsx @@ -88,7 +88,7 @@ export default class Link extends Mark { { title: node.attrs.title, href: sanitizeUrl(node.attrs.href), - class: "text-link", + class: "use-hover-preview", rel: "noopener noreferrer nofollow", }, 0, @@ -203,20 +203,6 @@ export default class Link extends Mark { props: { decorations: (state: EditorState) => plugin.getState(state), handleDOMEvents: { - mouseover: (view: EditorView, event: MouseEvent) => { - const target = (event.target as HTMLElement)?.closest("a"); - if ( - target instanceof HTMLAnchorElement && - target.className.includes("text-link") && - this.editor.elementRef.current?.contains(target) && - (!view.editable || (view.editable && !view.hasFocus())) - ) { - if (this.options.onHoverLink) { - return this.options.onHoverLink(target); - } - } - return false; - }, mousedown: (view: EditorView, event: MouseEvent) => { const target = (event.target as HTMLElement)?.closest("a"); if (!(target instanceof HTMLAnchorElement) || event.button !== 0) { diff --git a/shared/editor/nodes/Mention.ts b/shared/editor/nodes/Mention.ts index 948c7783d..18fef134f 100644 --- a/shared/editor/nodes/Mention.ts +++ b/shared/editor/nodes/Mention.ts @@ -5,8 +5,7 @@ import { NodeType, Schema, } from "prosemirror-model"; -import { Command, Plugin, TextSelection } from "prosemirror-state"; -import { EditorView } from "prosemirror-view"; +import { Command, TextSelection } from "prosemirror-state"; import { Primitive } from "utility-types"; import Suggestion from "../extensions/Suggestion"; import { MarkdownSerializerState } from "../lib/markdown/serializer"; @@ -64,7 +63,7 @@ export default class Mention extends Suggestion { toDOM: (node) => [ "span", { - class: `${node.type.name}`, + class: `${node.type.name} use-hover-preview`, id: node.attrs.id, "data-type": node.attrs.type, "data-id": node.attrs.modelId, @@ -81,31 +80,6 @@ export default class Mention extends Suggestion { return [mentionRule]; } - get plugins(): Plugin[] { - return [ - new Plugin({ - props: { - handleDOMEvents: { - mouseover: (view: EditorView, event: MouseEvent) => { - const target = (event.target as HTMLElement)?.closest("span"); - if ( - target instanceof HTMLSpanElement && - this.editor.elementRef.current?.contains(target) && - target.className.includes("mention") && - (!view.editable || (view.editable && !view.hasFocus())) - ) { - if (this.options.onHoverLink) { - return this.options.onHoverLink(target); - } - } - return false; - }, - }, - }, - }), - ]; - } - commands({ type }: { type: NodeType; schema: Schema }) { return (attrs: Record): Command => (state, dispatch) => { diff --git a/shared/editor/nodes/index.ts b/shared/editor/nodes/index.ts index 6ef1263b1..917aa4deb 100644 --- a/shared/editor/nodes/index.ts +++ b/shared/editor/nodes/index.ts @@ -3,6 +3,7 @@ import ClipboardTextSerializer from "../extensions/ClipboardTextSerializer"; import DateTime from "../extensions/DateTime"; import FindAndReplace from "../extensions/FindAndReplace"; import History from "../extensions/History"; +import HoverPreviews from "../extensions/HoverPreviews"; import Keys from "../extensions/Keys"; import MaxLength from "../extensions/MaxLength"; import PasteHandler from "../extensions/PasteHandler"; @@ -113,6 +114,7 @@ export const richExtensions: Nodes = [ MathBlock, PreventTab, FindAndReplace, + HoverPreviews, ]; /**