Skip to content

Commit

Permalink
fix: Improve run messages (#510)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
qgerome authored Dec 26, 2023
1 parent fc090e6 commit 1998ff9
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 70 deletions.
1 change: 1 addition & 0 deletions public/locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
1 change: 1 addition & 0 deletions public/locales/fr/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "",
Expand Down
24 changes: 18 additions & 6 deletions src/core/hooks/useAutoScroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@ const useAutoScroll = (
scrollableContentRef: React.RefObject<HTMLElement>,
scrollBehavior: "auto" | "smooth" = "auto",
) => {
const isScrolledToBottomRef = useRef<boolean>(true);
const isAutoScrollActive = useRef<boolean>(true);
const lastScrollTop = useRef<number>(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,
});
}
Expand All @@ -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);
Expand All @@ -41,6 +48,11 @@ const useAutoScroll = (
};
}
}, [scrollableContentRef, scrollBehavior]);

useEffect(() => {
isAutoScrollActive.current = true;
lastScrollTop.current = 0;
}, [scrollableContentRef]);
};

export default useAutoScroll;
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -205,7 +205,7 @@ const WorkspacePipelineRunPage: NextPageWithLayout = (props: Props) => {
title={run.executionDate}
suppressHydrationWarning={true}
>
<PipelineRunStatusBadge run={run} />
<PipelineRunStatusBadge run={run} polling={false} />
</div>
</div>
</div>
Expand Down Expand Up @@ -278,7 +278,8 @@ const WorkspacePipelineRunPage: NextPageWithLayout = (props: Props) => {
</Block.Section>
)}
<Block.Section title={t("Messages")}>
<RunMessages run={run} />
{/* Set a ref to the component to recreate it completely when the run id changes. */}
<RunMessages key={run.id} run={run} />
</Block.Section>
<Block.Section title={t("Logs")} collapsible defaultOpen={false}>
<RunLogs run={run} />
Expand Down
5 changes: 3 additions & 2 deletions src/pipelines/features/PipelineRunStatusBadge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
115 changes: 58 additions & 57 deletions src/pipelines/features/RunMessages/RunMessages.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<HTMLDivElement>(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 (
<Overflow vertical style={{ height: maxHeight }} forwardedRef={ref}>
{run.messages.length === 0 ? (
<p className="text-sm italic text-gray-600">{t("No messages")}</p>
) : (
<table className="table-fixed">
<tbody>
{run.messages.map((message, index) => (
<tr key={index}>
<td className="p-1.5">
<Badge className={getBadgeClassName(message.priority)}>
{message.priority}
</Badge>
</td>
<td className="p-1.5">
<Time
className="text-sm text-gray-400"
datetime={message.timestamp}
format={DateTime.DATETIME_SHORT_WITH_SECONDS}
/>
</td>
<td className="p-1.5 text-sm">
<Linkify
as="span"
options={{
render: ({
attributes,
content,
}: {
attributes: any;
content: any;
}) => <Link {...attributes}>{content}</Link>,
}}
>
{message.message}
</Linkify>
</td>
</tr>
))}
</tbody>
</table>
<>
<div className="max-h-96 overflow-y-auto">
<div ref={ref}>
{run.messages.length === 0 &&
[PipelineRunStatus.Failed, PipelineRunStatus.Success].includes(
run.status as PipelineRunStatus,
) ? (
<p className="text-sm italic text-gray-600">{t("No messages")}</p>
) : (
<table className="table-fixed">
<tbody>
{run.messages.map((message, index) => (
<tr key={index}>
<td className="p-1">
<Badge className={getBadgeClassName(message.priority)}>
{message.priority}
</Badge>
</td>
<td className="p-1">
<Time
className="text-sm text-gray-400"
datetime={message.timestamp}
format={DateTime.DATETIME_SHORT_WITH_SECONDS}
/>
</td>
<td className="p-1 text-sm">
<Linkify
as="span"
options={{
render: ({
attributes,
content,
}: {
attributes: any;
content: any;
}) => <Link {...attributes}>{content}</Link>,
}}
>
{message.message}
</Linkify>
</td>
</tr>
))}
</tbody>
</table>
)}
</div>
</div>
{run.status === PipelineRunStatus.Running && (
<div className="flex items-center gap-1.5 text-gray-400 text-sm px-2 py-2">
<Spinner size="xs" />
{t("Waiting for messages...")}
</div>
)}
</Overflow>
</>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 1998ff9

Please sign in to comment.