From 1998ff956c6dd1c272e007a0ce5478c480c85930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20G=C3=A9r=C3=B4me?= Date: Tue, 26 Dec 2023 14:34:12 +0100 Subject: [PATCH] fix: Improve run messages (#510) * chore(PipelineRunStatusBadge): Add a way to disable the polling * fix(useAutoScroll): Better detection of scroll position * chore(RunPipelineDialog): Do not re-render the dialog so often * chore(Runs): Increase polling rate when running * chore(Runs): Change display of run messages * chore(i18n): update translations --- public/locales/en/messages.json | 1 + public/locales/fr/messages.json | 1 + src/core/hooks/useAutoScroll.ts | 24 +++- .../pipelines/[pipelineCode]/runs/[runId].tsx | 9 +- .../features/PipelineRunStatusBadge.tsx | 5 +- .../features/RunMessages/RunMessages.tsx | 115 +++++++++--------- .../RunPipelineDialog/RunPipelineDialog.tsx | 2 +- 7 files changed, 87 insertions(+), 70 deletions(-) diff --git a/public/locales/en/messages.json b/public/locales/en/messages.json index dd14e170..9e20422f 100644 --- a/public/locales/en/messages.json +++ b/public/locales/en/messages.json @@ -415,6 +415,7 @@ "View all": "", "Viewer": "", "Visit the OpenHexa documentation homepage": "", + "Waiting for messages...": "", "We were not able to add this run to your favorites": "", "We were not able to generate a download url for this file": "", "We were not able to remove it from your favorites": "", diff --git a/public/locales/fr/messages.json b/public/locales/fr/messages.json index c543eddb..ad84d4b6 100644 --- a/public/locales/fr/messages.json +++ b/public/locales/fr/messages.json @@ -417,6 +417,7 @@ "View all": "", "Viewer": "", "Visit the OpenHexa documentation homepage": "", + "Waiting for messages...": "", "We were not able to add this run to your favorites": "", "We were not able to generate a download url for this file": "", "We were not able to remove it from your favorites": "", diff --git a/src/core/hooks/useAutoScroll.ts b/src/core/hooks/useAutoScroll.ts index 5e64c9a7..14526b55 100644 --- a/src/core/hooks/useAutoScroll.ts +++ b/src/core/hooks/useAutoScroll.ts @@ -4,16 +4,17 @@ const useAutoScroll = ( scrollableContentRef: React.RefObject, scrollBehavior: "auto" | "smooth" = "auto", ) => { - const isScrolledToBottomRef = useRef(true); + const isAutoScrollActive = useRef(true); + const lastScrollTop = useRef(0); useEffect(() => { const child = scrollableContentRef.current; const parent = child?.parentElement; if (child && parent) { const resizeObserver = new ResizeObserver(() => { - if (isScrolledToBottomRef.current) { + if (isAutoScrollActive.current) { parent.scrollTo({ - top: parent.scrollHeight + 10, + top: child.scrollHeight, behavior: scrollBehavior, }); } @@ -22,9 +23,15 @@ const useAutoScroll = ( resizeObserver.observe(child); const onScroll = (event: any) => { - const { scrollTop, scrollHeight, clientHeight } = event.target; - const isScrolledToBottom = scrollHeight - scrollTop === clientHeight; - isScrolledToBottomRef.current = isScrolledToBottom; + const { scrollTop, scrollHeight } = event.target; + const isScrolledToBottom = scrollHeight === child.scrollHeight; + if (scrollTop < lastScrollTop.current && lastScrollTop.current !== 0) { + isAutoScrollActive.current = false; + } else if (isScrolledToBottom) { + isAutoScrollActive.current = true; + } + + lastScrollTop.current = scrollTop; }; // Observe scroll position parent.addEventListener("scroll", onScroll); @@ -41,6 +48,11 @@ const useAutoScroll = ( }; } }, [scrollableContentRef, scrollBehavior]); + + useEffect(() => { + isAutoScrollActive.current = true; + lastScrollTop.current = 0; + }, [scrollableContentRef]); }; export default useAutoScroll; diff --git a/src/pages/workspaces/[workspaceSlug]/pipelines/[pipelineCode]/runs/[runId].tsx b/src/pages/workspaces/[workspaceSlug]/pipelines/[pipelineCode]/runs/[runId].tsx index 98594c0d..d7052026 100644 --- a/src/pages/workspaces/[workspaceSlug]/pipelines/[pipelineCode]/runs/[runId].tsx +++ b/src/pages/workspaces/[workspaceSlug]/pipelines/[pipelineCode]/runs/[runId].tsx @@ -60,9 +60,9 @@ const WorkspacePipelineRunPage: NextPageWithLayout = (props: Props) => { const refreshInterval = useMemo(() => { switch (data?.pipelineRun?.status) { case PipelineRunStatus.Queued: - return 1 * 1000; + return 0.5 * 1000; // 2 times per second case PipelineRunStatus.Running: - return 2 * 1000; + return 0.25 * 1000; // 4 times per second default: return null; } @@ -205,7 +205,7 @@ const WorkspacePipelineRunPage: NextPageWithLayout = (props: Props) => { title={run.executionDate} suppressHydrationWarning={true} > - + @@ -278,7 +278,8 @@ const WorkspacePipelineRunPage: NextPageWithLayout = (props: Props) => { )} - + {/* Set a ref to the component to recreate it completely when the run id changes. */} + diff --git a/src/pipelines/features/PipelineRunStatusBadge.tsx b/src/pipelines/features/PipelineRunStatusBadge.tsx index e94b2881..fbd04cbf 100644 --- a/src/pipelines/features/PipelineRunStatusBadge.tsx +++ b/src/pipelines/features/PipelineRunStatusBadge.tsx @@ -15,11 +15,12 @@ type PipelineRunStatusBadgeProps = { run: | PipelineRunStatusBadge_DagRunFragment | PipelineRunStatusBadge_RunFragment; + polling?: boolean; }; const PipelineRunStatusBadge = (props: PipelineRunStatusBadgeProps) => { - const { run } = props; - usePipelineRunPoller((run as any).id); + const { run, polling = true } = props; + usePipelineRunPoller(polling ? (run as any).id : null); let className = useMemo(() => { switch (run.status) { case DagRunStatus.Failed: diff --git a/src/pipelines/features/RunMessages/RunMessages.tsx b/src/pipelines/features/RunMessages/RunMessages.tsx index 475cbb0f..2f72c2a1 100644 --- a/src/pipelines/features/RunMessages/RunMessages.tsx +++ b/src/pipelines/features/RunMessages/RunMessages.tsx @@ -1,18 +1,18 @@ import { gql } from "@apollo/client"; import Badge from "core/components/Badge"; -import Overflow from "core/components/Overflow"; +import Link from "core/components/Link"; +import Spinner from "core/components/Spinner"; import Time from "core/components/Time"; import useAutoScroll from "core/hooks/useAutoScroll"; import { PipelineRunStatus } from "graphql-types"; +import Linkify from "linkify-react"; import { DateTime } from "luxon"; import { useTranslation } from "next-i18next"; -import { useMemo, useRef } from "react"; -import Linkify from "linkify-react"; +import { useRef } from "react"; import { RunMessages_DagRunFragment, RunMessages_RunFragment, } from "./RunMessages.generated"; -import Link from "core/components/Link"; type RunMessagesProps = { run: RunMessages_DagRunFragment | RunMessages_RunFragment; @@ -33,67 +33,68 @@ function getBadgeClassName(priority: string) { } } -// Approximate height of a single message row -const MESSAGE_HEIGHT = 40; - const RunMessages = (props: RunMessagesProps) => { const { t } = useTranslation(); const { run } = props; const ref = useRef(null); useAutoScroll(ref, "smooth"); - const maxHeight = useMemo(() => { - if (run.status === PipelineRunStatus.Running) { - return 400; - } - return Math.min(400, MESSAGE_HEIGHT * (run.messages.length + 1)); - }, [run.messages.length, run.status]); - - // Scroll to bottom the container when the height changes - return ( - - {run.messages.length === 0 ? ( -

{t("No messages")}

- ) : ( - - - {run.messages.map((message, index) => ( - - - - - - ))} - -
- - {message.priority} - - - - {content}, - }} - > - {message.message} - -
+ <> +
+
+ {run.messages.length === 0 && + [PipelineRunStatus.Failed, PipelineRunStatus.Success].includes( + run.status as PipelineRunStatus, + ) ? ( +

{t("No messages")}

+ ) : ( + + + {run.messages.map((message, index) => ( + + + + + + ))} + +
+ + {message.priority} + + + + {content}, + }} + > + {message.message} + +
+ )} +
+
+ {run.status === PipelineRunStatus.Running && ( +
+ + {t("Waiting for messages...")} +
)} - + ); }; diff --git a/src/workspaces/features/RunPipelineDialog/RunPipelineDialog.tsx b/src/workspaces/features/RunPipelineDialog/RunPipelineDialog.tsx index a948cce4..50dc3b99 100644 --- a/src/workspaces/features/RunPipelineDialog/RunPipelineDialog.tsx +++ b/src/workspaces/features/RunPipelineDialog/RunPipelineDialog.tsx @@ -148,7 +148,7 @@ const RunPipelineDialog = (props: RunPipelineDialogProps) => { }, }); } - }, [open, form, fetch, props, pipeline]); + }, [open, form, fetch, props, pipeline.code, pipeline.workspace]); useEffect(() => { if (!form.formData.version && open) {