From ab68c62c7fc417062389a8e0e9be6a28e7286d3a Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Mon, 17 Oct 2022 14:15:34 +0000 Subject: [PATCH 1/3] site: cleanup code around WorkspaceBuildProgress --- .../WorkspaceBuildProgress.tsx | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index f9685b8951da4..6931ce47940e4 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -6,6 +6,7 @@ import { FC, useEffect, useState } from "react" import { MONOSPACE_FONT_FAMILY } from "theme/constants" import duration from "dayjs/plugin/duration" +import { ChooseOne, Cond } from "components/Conditionals/ChooseOne" dayjs.extend(duration) @@ -18,10 +19,9 @@ const estimateFinish = ( } const realPercentage = dayjs().diff(startedAt) / templateAverage - // Showing a full bar is frustrating. - const maxPercentage = 0.99 + const maxPercentage = 1 if (realPercentage > maxPercentage) { - return [maxPercentage, "Any moment now..."] + return [1, "Any moment now..."] } return [ @@ -62,7 +62,9 @@ export const WorkspaceBuildProgress: FC = ({ }) => { const styles = useStyles() const job = workspace.latest_build.job - const [progressValue, setProgressValue] = useState(0) + const [progressValue, setProgressValue] = useState( + undefined, + ) // By default workspace is updated every second, which can cause visual stutter // when the build estimate is a few seconds. The timer ensures no observable @@ -70,41 +72,43 @@ export const WorkspaceBuildProgress: FC = ({ useEffect(() => { const updateProgress = () => { if (job.status !== "running") { - setProgressValue(0) + setProgressValue(undefined) return } - setProgressValue( - estimateFinish(dayjs(job.started_at), buildEstimate)[0] * 100, - ) + const est = estimateFinish(dayjs(job.started_at), buildEstimate)[0] * 100 + setProgressValue(est) } + // Perform initial update setTimeout(updateProgress, 100) }, [progressValue, job, buildEstimate]) - // buildEstimate may be undefined if the template is new or coderd hasn't - // finished initial metrics collection. - if (buildEstimate === undefined) { - return ( -
- -
-
{`Build ${job.status}`}
-
Unknown ETA
-
-
- ) - } - return (
{`Build ${job.status}`}
- {job.status === "running" && - estimateFinish(dayjs(job.started_at), buildEstimate)[1]} + + + {estimateFinish(dayjs(job.started_at), buildEstimate)[1]} + + Unknown ETA +
@@ -116,6 +120,9 @@ const useStyles = makeStyles((theme) => ({ paddingLeft: theme.spacing(0.2), paddingRight: theme.spacing(0.2), }, + noTransition: { + transition: "none", + }, barHelpers: { display: "flex", justifyContent: "space-between", From b070943e33a3fd65c539c348cd39c85a9715339d Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Mon, 17 Oct 2022 14:41:42 +0000 Subject: [PATCH 2/3] fixup! site: cleanup code around WorkspaceBuildProgress --- .../WorkspaceBuildProgress.tsx | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index 6931ce47940e4..af7b4010bba64 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -6,28 +6,24 @@ import { FC, useEffect, useState } from "react" import { MONOSPACE_FONT_FAMILY } from "theme/constants" import duration from "dayjs/plugin/duration" -import { ChooseOne, Cond } from "components/Conditionals/ChooseOne" dayjs.extend(duration) const estimateFinish = ( startedAt: Dayjs, - templateAverage?: number, -): [number, string] => { - if (templateAverage === undefined) { - return [0, "Unknown"] - } - const realPercentage = dayjs().diff(startedAt) / templateAverage + buildEstimate: number, +): [number | undefined, string] => { + const realPercentage = dayjs().diff(startedAt) / buildEstimate const maxPercentage = 1 if (realPercentage > maxPercentage) { - return [1, "Any moment now..."] + return [undefined, "Any moment now..."] } return [ - realPercentage, + realPercentage * 100, `~${Math.ceil( - dayjs.duration((1 - realPercentage) * templateAverage).asSeconds(), + dayjs.duration((1 - realPercentage) * buildEstimate).asSeconds(), )} seconds remaining...`, ] } @@ -71,14 +67,13 @@ export const WorkspaceBuildProgress: FC = ({ // stutter in all cases. useEffect(() => { const updateProgress = () => { - if (job.status !== "running") { + if (job.status !== "running" || buildEstimate === undefined) { setProgressValue(undefined) return } - const est = estimateFinish(dayjs(job.started_at), buildEstimate)[0] * 100 + const est = estimateFinish(dayjs(job.started_at), buildEstimate)[0] setProgressValue(est) } - // Perform initial update setTimeout(updateProgress, 100) }, [progressValue, job, buildEstimate]) @@ -91,7 +86,7 @@ export const WorkspaceBuildProgress: FC = ({ // (e.g. the build isn't yet running). If we flicker from the // indeterminate bar to the determinate bar, the vigilant user // perceives the bar jumping from 100% to 0%. - progressValue !== undefined || dayjs(job.started_at).diff() < 500 + progressValue !== undefined || dayjs(job.started_at).diff() > 500 ? "determinate" : "indeterminate" } @@ -103,12 +98,15 @@ export const WorkspaceBuildProgress: FC = ({
{`Build ${job.status}`}
- - - {estimateFinish(dayjs(job.started_at), buildEstimate)[1]} - - Unknown ETA - + {(() => { + if (job.status !== "running") { + return "" + } else if (buildEstimate !== undefined) { + return estimateFinish(dayjs(job.started_at), buildEstimate)[1] + } else { + return "Unknown ETA" + } + })()}
From eea9dc1c8bab0a12a1c310f9ca49bffd2b0b9d98 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Mon, 17 Oct 2022 15:04:55 +0000 Subject: [PATCH 3/3] fixup! site: cleanup code around WorkspaceBuildProgress --- .../WorkspaceBuildProgress.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index af7b4010bba64..f1d61ee26ea15 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -12,12 +12,12 @@ dayjs.extend(duration) const estimateFinish = ( startedAt: Dayjs, buildEstimate: number, -): [number | undefined, string] => { +): [number, string] => { const realPercentage = dayjs().diff(startedAt) / buildEstimate const maxPercentage = 1 if (realPercentage > maxPercentage) { - return [undefined, "Any moment now..."] + return [maxPercentage * 100, "Any moment now..."] } return [ @@ -58,9 +58,7 @@ export const WorkspaceBuildProgress: FC = ({ }) => { const styles = useStyles() const job = workspace.latest_build.job - const [progressValue, setProgressValue] = useState( - undefined, - ) + const [progressValue, setProgressValue] = useState(0) // By default workspace is updated every second, which can cause visual stutter // when the build estimate is a few seconds. The timer ensures no observable @@ -74,7 +72,7 @@ export const WorkspaceBuildProgress: FC = ({ const est = estimateFinish(dayjs(job.started_at), buildEstimate)[0] setProgressValue(est) } - setTimeout(updateProgress, 100) + setTimeout(updateProgress, 5) }, [progressValue, job, buildEstimate]) return ( @@ -86,14 +84,16 @@ export const WorkspaceBuildProgress: FC = ({ // (e.g. the build isn't yet running). If we flicker from the // indeterminate bar to the determinate bar, the vigilant user // perceives the bar jumping from 100% to 0%. - progressValue !== undefined || dayjs(job.started_at).diff() > 500 + progressValue !== undefined && + progressValue < 100 && + buildEstimate !== undefined ? "determinate" : "indeterminate" } // If a transition is set, there is a moment on new load where the // bar accelerates to progressValue and then rapidly decelerates, which // is not indicative of true progress. - className={styles.noTransition} + classes={{ bar: styles.noTransition }} />
{`Build ${job.status}`}