From 4d84bdb8828f0fe03a9d51f699c723376bda64df Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 5 Aug 2022 16:36:35 +0200 Subject: [PATCH 1/6] Fix copy link to logs functionality --- .../src/components/JobItem/JobItem.tsx | 25 +++++++------------ .../JobItem/components/MainInfo.tsx | 12 ++++----- .../components/StatusIcon/StatusIcon.test.tsx | 1 + .../src/components/StatusIcon/StatusIcon.tsx | 24 ++++++++++-------- 4 files changed, 28 insertions(+), 34 deletions(-) diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx index df7d6006ebd3..708f8481e2bf 100644 --- a/airbyte-webapp/src/components/JobItem/JobItem.tsx +++ b/airbyte-webapp/src/components/JobItem/JobItem.tsx @@ -7,7 +7,7 @@ import { Spinner } from "components"; import { SynchronousJobReadWithStatus } from "core/request/LogsRequestError"; import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/components/JobsList"; -import { AttemptRead, JobStatus } from "../../core/request/AirbyteClient"; +import { AttemptRead, CheckConnectionReadStatus, JobStatus } from "../../core/request/AirbyteClient"; import { useAttemptLink } from "./attemptLinkUtils"; import ContentWrapper from "./components/ContentWrapper"; import ErrorDetails from "./components/ErrorDetails"; @@ -32,7 +32,6 @@ const LoadLogs = styled.div` `; interface JobItemProps { - shortInfo?: boolean; job: SynchronousJobReadWithStatus | JobsWithJobs; } @@ -40,20 +39,21 @@ const didJobSucceed = (job: SynchronousJobReadWithStatus | JobsWithJobs) => { return getJobStatus(job) !== "failed"; }; -export const getJobStatus: (job: SynchronousJobReadWithStatus | JobsWithJobs) => JobStatus = (job) => { - return (job as JobsWithJobs).job?.status ?? (job as SynchronousJobReadWithStatus).status; +export const getJobStatus: ( + job: SynchronousJobReadWithStatus | JobsWithJobs +) => JobStatus | CheckConnectionReadStatus = (job) => { + return "status" in job ? job.status : job.job.status; }; export const getJobAttemps: (job: SynchronousJobReadWithStatus | JobsWithJobs) => AttemptRead[] | undefined = (job) => { return "attempts" in job ? job.attempts : undefined; }; -export const getJobId = (job: SynchronousJobReadWithStatus | JobsWithJobs) => - (job as SynchronousJobReadWithStatus).id ?? (job as JobsWithJobs).job.id; +export const getJobId = (job: SynchronousJobReadWithStatus | JobsWithJobs) => ("id" in job ? job.id : job.job.id); -export const JobItem: React.FC = ({ shortInfo, job }) => { +export const JobItem: React.FC = ({ job }) => { const { jobId: linkedJobId } = useAttemptLink(); - const [isOpen, setIsOpen] = useState(linkedJobId === getJobId(job)); + const [isOpen, setIsOpen] = useState(linkedJobId === String(getJobId(job))); const scrollAnchor = useRef(null); const didSucceed = didJobSucceed(job); @@ -73,14 +73,7 @@ export const JobItem: React.FC = ({ shortInfo, job }) => { return ( - +
void; isFailed?: boolean; - shortInfo?: boolean; } -const MainInfo: React.FC = ({ job, attempts = [], isOpen, onExpand, isFailed, shortInfo }) => { +const MainInfo: React.FC = ({ job, attempts = [], isOpen, onExpand, isFailed }) => { const jobStatus = getJobStatus(job); const isPartialSuccess = partialSuccessCheck(attempts); const statusIcon = () => { switch (true) { case jobStatus === JobStatus.cancelled: - return ; + return ; case jobStatus === JobStatus.running: return ; case jobStatus === JobStatus.succeeded: return ; case isPartialSuccess: return ; - case !isPartialSuccess && isFailed && !shortInfo: - return ; + case !isPartialSuccess && isFailed: + return ; default: return null; } @@ -71,8 +70,7 @@ const MainInfo: React.FC = ({ job, attempts = [], isOpen, onExpan ) : ( )} - {shortInfo && } - {attempts.length && !shortInfo && ( + {attempts.length && ( <> {attempts.length > 1 && (
diff --git a/airbyte-webapp/src/components/StatusIcon/StatusIcon.test.tsx b/airbyte-webapp/src/components/StatusIcon/StatusIcon.test.tsx index 0003a0705873..d35d77f7fda8 100644 --- a/airbyte-webapp/src/components/StatusIcon/StatusIcon.test.tsx +++ b/airbyte-webapp/src/components/StatusIcon/StatusIcon.test.tsx @@ -20,6 +20,7 @@ describe("", () => { { status: "sleep", icon: "moon" }, { status: "warning", icon: "triangle-exclamation" }, { status: "loading", icon: "circle-loader" }, + { status: "error", icon: "xmark" }, ]; test.each(statusCases)("renders $status status", ({ status, icon }) => { diff --git a/airbyte-webapp/src/components/StatusIcon/StatusIcon.tsx b/airbyte-webapp/src/components/StatusIcon/StatusIcon.tsx index c746a72f147c..2da76601b8bd 100644 --- a/airbyte-webapp/src/components/StatusIcon/StatusIcon.tsx +++ b/airbyte-webapp/src/components/StatusIcon/StatusIcon.tsx @@ -1,4 +1,4 @@ -import { faBan, faCheck, faExclamationTriangle, faTimes, IconDefinition } from "@fortawesome/free-solid-svg-icons"; +import { faBan, faCheck, faExclamationTriangle, faTimes } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import React from "react"; import styled from "styled-components"; @@ -8,7 +8,7 @@ import { MoonIcon } from "components/icons/MoonIcon"; import PauseIcon from "../icons/PauseIcon"; import CircleLoader from "./CircleLoader"; -export type StatusIconStatus = "sleep" | "inactive" | "success" | "warning" | "loading"; +export type StatusIconStatus = "sleep" | "inactive" | "success" | "warning" | "loading" | "error"; interface StatusIconProps { className?: string; @@ -20,20 +20,22 @@ interface StatusIconProps { const getBadgeWidth = (props: StatusIconProps) => (props.big ? (props.value ? 57 : 40) : props.value ? 37 : 20); -const _iconByStatus: Partial> = { +const _iconByStatus = { sleep: faBan, success: faCheck, warning: faExclamationTriangle, -}; + error: faTimes, +} as const; -const _themeByStatus: Partial> = { +const _themeByStatus = { sleep: "lightTextColor", inactive: "lightTextColor", success: "successColor", warning: "warningColor", -}; + error: "dangerColor", +} as const; -const Container = styled.div` +const Container = styled.div>` width: ${(props) => getBadgeWidth(props)}px; height: ${({ big }) => (big ? 40 : 20)}px; margin-right: 10px; @@ -44,8 +46,8 @@ const Container = styled.div` vertical-align: middle; `; -const Badge = styled(Container)` - background: ${(props) => props.theme[(props.status && _themeByStatus[props.status]) || "dangerColor"]}; +const Badge = styled(Container)<{ status: Exclude }>` + background: ${({ theme, status }) => theme[_themeByStatus[status]]}; border-radius: ${({ value }) => (value ? "15px" : "50%")}; color: ${({ theme }) => theme.whiteColor}; padding-top: ${({ status }) => (status === "warning" || status === "inactive" ? 3 : 4)}px; @@ -66,7 +68,7 @@ const Value = styled.span` padding-left: 3px; `; -const StatusIcon: React.FC = ({ title, status, ...props }) => { +const StatusIcon: React.FC = ({ title, status = "error", ...props }) => { const valueElement = props.value ? {props.value} : null; if (status === "loading") { @@ -85,7 +87,7 @@ const StatusIcon: React.FC = ({ title, status, ...props }) => { ) : status === "sleep" ? ( ) : ( - + )} {valueElement} From 7aefd6fb4b92d072e047a977d68cdf351f3bf465 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 5 Aug 2022 21:01:00 +0200 Subject: [PATCH 2/6] Update airbyte-webapp/src/components/JobItem/JobItem.tsx Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com> --- airbyte-webapp/src/components/JobItem/JobItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx index 708f8481e2bf..67444bfd54de 100644 --- a/airbyte-webapp/src/components/JobItem/JobItem.tsx +++ b/airbyte-webapp/src/components/JobItem/JobItem.tsx @@ -53,7 +53,7 @@ export const getJobId = (job: SynchronousJobReadWithStatus | JobsWithJobs) => (" export const JobItem: React.FC = ({ job }) => { const { jobId: linkedJobId } = useAttemptLink(); - const [isOpen, setIsOpen] = useState(linkedJobId === String(getJobId(job))); + const [isOpen, setIsOpen] = useState(() => linkedJobId === String(getJobId(job))); const scrollAnchor = useRef(null); const didSucceed = didJobSucceed(job); From 4d393d29f31b443003880018033d63af72f18134 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 8 Aug 2022 16:27:29 +0200 Subject: [PATCH 3/6] Fix scrolling --- .../src/components/JobItem/JobItem.tsx | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx index 67444bfd54de..df50fa6f2f34 100644 --- a/airbyte-webapp/src/components/JobItem/JobItem.tsx +++ b/airbyte-webapp/src/components/JobItem/JobItem.tsx @@ -63,12 +63,20 @@ export const JobItem: React.FC = ({ job }) => { }; useEffectOnce(() => { - if (linkedJobId) { - scrollAnchor.current?.scrollIntoView({ - behavior: "smooth", - block: "start", - }); + if (linkedJobId === String(getJobId(job))) { + // We need to wait here a bit, so the page has a chance to finish rendering, before starting to scroll + // since otherwise this scroll won't really do anything. + const timeout = window.setTimeout(() => { + scrollAnchor.current?.scrollIntoView({ + behavior: "smooth", + block: "start", + }); + }, 1000); + return () => { + window.clearTimeout(timeout); + }; } + return undefined; }); return ( From 1527d19356414e45a98f6306a96093b3eff791a7 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 8 Aug 2022 17:51:56 +0200 Subject: [PATCH 4/6] Remove smooth scrolling --- airbyte-webapp/src/components/JobItem/JobItem.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx index df50fa6f2f34..a7c657467d46 100644 --- a/airbyte-webapp/src/components/JobItem/JobItem.tsx +++ b/airbyte-webapp/src/components/JobItem/JobItem.tsx @@ -68,7 +68,6 @@ export const JobItem: React.FC = ({ job }) => { // since otherwise this scroll won't really do anything. const timeout = window.setTimeout(() => { scrollAnchor.current?.scrollIntoView({ - behavior: "smooth", block: "start", }); }, 1000); From 00f717a7266d499514edeacdc1d1ff1be96a667e Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 8 Aug 2022 17:52:44 +0200 Subject: [PATCH 5/6] Improve effect for better return statements --- .../src/components/JobItem/JobItem.tsx | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx index a7c657467d46..fe0297304fd2 100644 --- a/airbyte-webapp/src/components/JobItem/JobItem.tsx +++ b/airbyte-webapp/src/components/JobItem/JobItem.tsx @@ -63,19 +63,19 @@ export const JobItem: React.FC = ({ job }) => { }; useEffectOnce(() => { - if (linkedJobId === String(getJobId(job))) { - // We need to wait here a bit, so the page has a chance to finish rendering, before starting to scroll - // since otherwise this scroll won't really do anything. - const timeout = window.setTimeout(() => { - scrollAnchor.current?.scrollIntoView({ - block: "start", - }); - }, 1000); - return () => { - window.clearTimeout(timeout); - }; + if (linkedJobId !== String(getJobId(job))) { + return; } - return undefined; + // We need to wait here a bit, so the page has a chance to finish rendering, before starting to scroll + // since otherwise this scroll won't really do anything. + const timeout = window.setTimeout(() => { + scrollAnchor.current?.scrollIntoView({ + block: "start", + }); + }, 1000); + return () => { + window.clearTimeout(timeout); + }; }); return ( From 60b0082c656f4716ea596a8b09c91e07b31b1024 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 9 Aug 2022 18:52:23 +0200 Subject: [PATCH 6/6] Better scroll --- .../src/components/JobItem/JobItem.tsx | 26 +++++++------------ .../JobItem/components/ContentWrapper.tsx | 4 ++- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx index fe0297304fd2..2eb6a02f4098 100644 --- a/airbyte-webapp/src/components/JobItem/JobItem.tsx +++ b/airbyte-webapp/src/components/JobItem/JobItem.tsx @@ -1,5 +1,4 @@ -import React, { Suspense, useRef, useState } from "react"; -import { useEffectOnce } from "react-use"; +import React, { Suspense, useCallback, useRef, useState } from "react"; import styled from "styled-components"; import { Spinner } from "components"; @@ -53,6 +52,7 @@ export const getJobId = (job: SynchronousJobReadWithStatus | JobsWithJobs) => (" export const JobItem: React.FC = ({ job }) => { const { jobId: linkedJobId } = useAttemptLink(); + const alreadyScrolled = useRef(false); const [isOpen, setIsOpen] = useState(() => linkedJobId === String(getJobId(job))); const scrollAnchor = useRef(null); @@ -62,26 +62,20 @@ export const JobItem: React.FC = ({ job }) => { setIsOpen(!isOpen); }; - useEffectOnce(() => { - if (linkedJobId !== String(getJobId(job))) { + const onDetailsToggled = useCallback(() => { + if (alreadyScrolled.current || linkedJobId !== String(getJobId(job))) { return; } - // We need to wait here a bit, so the page has a chance to finish rendering, before starting to scroll - // since otherwise this scroll won't really do anything. - const timeout = window.setTimeout(() => { - scrollAnchor.current?.scrollIntoView({ - block: "start", - }); - }, 1000); - return () => { - window.clearTimeout(timeout); - }; - }); + scrollAnchor.current?.scrollIntoView({ + block: "start", + }); + alreadyScrolled.current = true; + }, [job, linkedJobId]); return ( - +
void; } -const ContentWrapper: React.FC = ({ children, isOpen }) => { +const ContentWrapper: React.FC = ({ children, isOpen, onToggled }) => { return (