From fc95b12310796e3b100a0a5ccc6d12ecb3bbbb0e Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 12 Sep 2023 12:19:50 +1000 Subject: [PATCH] Don't show eyebrow unnecessarily If the `StoryInfo` is already going to prompt you to switch, there's no point in redundantly showing you also in the eyebrow: https://www.chromatic.com/review?appId=6480e1b0042842f149cfd74c&number=87&activeElementId=comment-thread-64ff2a5781851ff6d5e44492 --- src/screens/VisualTests/BuildResults.tsx | 28 ++++++++++++------------ src/screens/VisualTests/VisualTests.tsx | 8 +++---- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/screens/VisualTests/BuildResults.tsx b/src/screens/VisualTests/BuildResults.tsx index b3823d84..513da28f 100644 --- a/src/screens/VisualTests/BuildResults.tsx +++ b/src/screens/VisualTests/BuildResults.tsx @@ -31,7 +31,6 @@ interface BuildResultsProps { nextBuild: NextBuildFieldsFragment; switchToNextBuild?: () => void; startDevBuild: () => void; - isOutdated: boolean; isReviewing: boolean; onAccept: (testId: string, batch: ReviewTestBatch) => Promise; onUnaccept: (testId: string) => Promise; @@ -43,7 +42,6 @@ export const BuildResults = ({ nextBuild, switchToNextBuild, startDevBuild, - isOutdated, isReviewing, onAccept, onUnaccept, @@ -57,18 +55,6 @@ export const BuildResults = ({ const isRunningBuildInProgress = runningBuild && runningBuild.step !== "complete"; const isReviewable = nextBuild.id === storyBuild.id; - const showBuildStatus = - // We always want to show the status of the running build (until it is done) - isRunningBuildInProgress || - // Even if there's no build running, we want to show the next build if it hasn't been selected. - !isReviewable; - const runningBuildIsNextBuild = runningBuild && runningBuild?.buildId === nextBuild.id; - const buildStatus = showBuildStatus && ( - - ); const storyTests = [ ...getFragment( @@ -85,6 +71,20 @@ export const BuildResults = ({ const isStorySuperseded = !isReviewable && nextStoryTests.every(({ status }) => status !== TestStatus.InProgress); + const showBuildStatus = + // We always want to show the status of the running build (until it is done) + isRunningBuildInProgress || + // Even if there's no build running, we want to show the next build if it hasn't been selected, + // unless the story info itself is going to tell us to switch already + (!isReviewable && !(isStorySuperseded && switchToNextBuild)); + const runningBuildIsNextBuild = runningBuild && runningBuild?.buildId === nextBuild.id; + const buildStatus = showBuildStatus && ( + + ); + // It shouldn't be possible for one test to be skipped but not all of them const isSkipped = !!storyTests?.find((t) => t.result === TestResult.Skipped); if (isSkipped) { diff --git a/src/screens/VisualTests/VisualTests.tsx b/src/screens/VisualTests/VisualTests.tsx index f580deb3..1204d90f 100644 --- a/src/screens/VisualTests/VisualTests.tsx +++ b/src/screens/VisualTests/VisualTests.tsx @@ -139,6 +139,10 @@ export const VisualTests = ({ data?.storyBuild ?? data?.project?.nextBuild ); + // Currently only used by the sidebar button to show a blue dot ("build outdated") + const isOutdated = storyBuild?.uncommittedHash !== gitInfo.uncommittedHash; + useEffect(() => setOutdated(isOutdated), [isOutdated, setOutdated]); + // If the next build is *newer* than the current commit, we don't want to switch to the build const nextBuildNewer = nextBuild && nextBuild.committedAt > gitInfo.committedAt; const canSwitchToNextBuild = nextBuild && !nextBuildNewer; @@ -178,9 +182,6 @@ export const VisualTests = ({ ); const isRunningBuildStarting = runningBuild && !["success", "error"].includes(runningBuild.step); - const isOutdated = storyBuild?.uncommittedHash !== gitInfo.uncommittedHash; - - useEffect(() => setOutdated(isOutdated), [isOutdated, setOutdated]); return !nextBuild || error ? (