From 90ed6a5366b3ca0d18c389a3e3b7489baf0d8bb9 Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Sat, 13 Apr 2024 18:31:40 +0530 Subject: [PATCH] Fixes hover preview going out of window bounds (#6776) * fix: hover preview out of bounds * fix: pop * fix: check for both element and data * fix: show loading indicator * fix: width --- app/components/HoverPreview/Components.tsx | 3 +- app/components/HoverPreview/HoverPreview.tsx | 209 ++++++++----------- app/editor/extensions/HoverPreviews.tsx | 36 +++- server/routes/api/urls/schema.ts | 13 +- 4 files changed, 137 insertions(+), 124 deletions(-) diff --git a/app/components/HoverPreview/Components.tsx b/app/components/HoverPreview/Components.tsx index 91d4669a4..8f2b1dffc 100644 --- a/app/components/HoverPreview/Components.tsx +++ b/app/components/HoverPreview/Components.tsx @@ -25,8 +25,7 @@ export const Preview = styled(Link)` 0 0 1px 1px rgba(0, 0, 0, 0.05); overflow: hidden; position: absolute; - min-width: 350px; - max-width: 375px; + width: 375px; `; export const Title = styled(Text).attrs({ as: "h2", size: "large" })` diff --git a/app/components/HoverPreview/HoverPreview.tsx b/app/components/HoverPreview/HoverPreview.tsx index 4d96d0fd1..4d1c6ec0b 100644 --- a/app/components/HoverPreview/HoverPreview.tsx +++ b/app/components/HoverPreview/HoverPreview.tsx @@ -4,16 +4,11 @@ import { Portal } from "react-portal"; import styled from "styled-components"; import { depths } from "@shared/styles"; import { UnfurlResourceType } from "@shared/types"; -import LoadingIndicator from "~/components/LoadingIndicator"; -import env from "~/env"; 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"; +import LoadingIndicator from "../LoadingIndicator"; import { CARD_MARGIN } from "./Components"; import HoverPreviewDocument from "./HoverPreviewDocument"; import HoverPreviewIssue from "./HoverPreviewIssue"; @@ -21,13 +16,17 @@ import HoverPreviewLink from "./HoverPreviewLink"; import HoverPreviewMention from "./HoverPreviewMention"; import HoverPreviewPullRequest from "./HoverPreviewPullRequest"; -const DELAY_CLOSE = 600; +const DELAY_CLOSE = 500; const POINTER_HEIGHT = 22; const POINTER_WIDTH = 22; type Props = { /** The HTML element that is being hovered over, or null if none. */ element: HTMLElement | null; + /** Data to be previewed */ + data: Record | null; + /** Whether the preview data is being loaded */ + dataLoading: boolean; /** A callback on close of the hover preview. */ onClose: () => void; }; @@ -37,12 +36,10 @@ enum Direction { DOWN, } -function HoverPreviewDesktop({ element, onClose }: Props) { - const url = element?.getAttribute("href") || element?.dataset.url; - const previousUrl = usePrevious(url, true); +function HoverPreviewDesktop({ element, data, dataLoading, onClose }: Props) { const [isVisible, setVisible] = React.useState(false); const timerClose = React.useRef>(); - const cardRef = React.useRef(null); + const cardRef = React.useRef(null); const { cardLeft, cardTop, pointerLeft, pointerTop, pointerDir } = useHoverPosition({ cardRef, @@ -68,12 +65,12 @@ function HoverPreviewDesktop({ element, onClose }: Props) { // Open and close the preview when the element changes. React.useEffect(() => { - if (element) { + if (element && data && !dataLoading) { setVisible(true); } else { startCloseTimer(); } - }, [startCloseTimer, element]); + }, [startCloseTimer, element, data, dataLoading]); // Close the preview on Escape, scroll, or click outside. useOnClickOutside(cardRef, closePreview); @@ -101,108 +98,7 @@ function HoverPreviewDesktop({ element, onClose }: Props) { }; }, [element, startCloseTimer, isVisible, stopCloseTimer]); - const displayUrl = url ?? previousUrl; - - if (!isVisible || !displayUrl) { - return null; - } - - return ( - - - - {(data) => ( - - {data.type === UnfurlResourceType.Mention ? ( - - ) : data.type === UnfurlResourceType.Document ? ( - - ) : data.type === UnfurlResourceType.Issue ? ( - - ) : data.type === UnfurlResourceType.PR ? ( - - ) : ( - - )} - - - )} - - - - ); -} - -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: url.startsWith("/") ? env.URL + url : url, - documentId: ui.activeDocumentId, - }), - [url, ui.activeDocumentId] - ) - ); - - React.useEffect(() => { - if (url) { - void request(); - } - }, [url, request]); - - if (loading) { + if (dataLoading) { return ; } @@ -210,16 +106,93 @@ function DataLoader({ return null; } - return <>{children(data)}; + return ( + + + {isVisible ? ( + + {data.type === UnfurlResourceType.Mention ? ( + + ) : data.type === UnfurlResourceType.Document ? ( + + ) : data.type === UnfurlResourceType.Issue ? ( + + ) : data.type === UnfurlResourceType.PR ? ( + + ) : ( + + )} + + + ) : null} + + + ); } -function HoverPreview({ element, ...rest }: Props) { +function HoverPreview({ element, data, dataLoading, ...rest }: Props) { const isMobile = useMobile(); if (isMobile) { return null; } - return ; + return ( + + ); } function useHoverPosition({ diff --git a/app/editor/extensions/HoverPreviews.tsx b/app/editor/extensions/HoverPreviews.tsx index 6ddca9607..c91f7f3ff 100644 --- a/app/editor/extensions/HoverPreviews.tsx +++ b/app/editor/extensions/HoverPreviews.tsx @@ -4,6 +4,8 @@ import { EditorView } from "prosemirror-view"; import * as React from "react"; import Extension from "@shared/editor/lib/Extension"; import HoverPreview from "~/components/HoverPreview"; +import env from "~/env"; +import { client } from "~/utils/ApiClient"; interface HoverPreviewsOptions { /** Delay before the target is considered "hovered" and callback is triggered. */ @@ -13,13 +15,17 @@ interface HoverPreviewsOptions { export default class HoverPreviews extends Extension { state: { activeLinkElement: HTMLElement | null; + data: Record | null; + dataLoading: boolean; } = observable({ activeLinkElement: null, + data: null, + dataLoading: false, }); get defaultOptions(): HoverPreviewsOptions { return { - delay: 500, + delay: 600, }; } @@ -45,8 +51,30 @@ export default class HoverPreviews extends Extension { ); if (isHoverTarget(target, view)) { hoveringTimeout = setTimeout( - action(() => { - this.state.activeLinkElement = target as HTMLElement; + action(async () => { + const element = target as HTMLElement; + + const url = + element?.getAttribute("href") || element?.dataset.url; + const documentId = window.location.pathname + .split("/") + .pop(); + + if (url) { + this.state.dataLoading = true; + try { + const data = await client.post("/urls.unfurl", { + url: url.startsWith("/") ? env.URL + url : url, + documentId, + }); + this.state.activeLinkElement = element; + this.state.data = data; + } catch (err) { + this.state.activeLinkElement = null; + } finally { + this.state.dataLoading = false; + } + } }), this.options.delay ); @@ -72,6 +100,8 @@ export default class HoverPreviews extends Extension { widget = () => ( { this.state.activeLinkElement = null; })} diff --git a/server/routes/api/urls/schema.ts b/server/routes/api/urls/schema.ts index ef2b30266..719e6a2ef 100644 --- a/server/routes/api/urls/schema.ts +++ b/server/routes/api/urls/schema.ts @@ -1,5 +1,7 @@ import isNil from "lodash/isNil"; +import isUUID from "validator/lib/isUUID"; import { z } from "zod"; +import { UrlHelper } from "@shared/utils/UrlHelper"; import { isUrl } from "@shared/utils/urls"; import { ValidateURL } from "@server/validation"; import { BaseSchema } from "../schema"; @@ -24,7 +26,16 @@ export const UrlsUnfurlSchema = BaseSchema.extend({ }, { message: ValidateURL.message } ), - documentId: z.string().uuid().optional(), + documentId: z + .string() + .optional() + .refine( + (val) => + val ? isUUID(val) || UrlHelper.SLUG_URL_REGEX.test(val) : true, + { + message: "must be uuid or url slug", + } + ), }) .refine( (val) =>