diff --git a/src/Panel.tsx b/src/Panel.tsx index c84cebaf..73f0f51a 100644 --- a/src/Panel.tsx +++ b/src/Panel.tsx @@ -19,6 +19,7 @@ import { GitNotFound } from "./screens/GitNotFound/GitNotFound"; import { LinkedProject } from "./screens/LinkProject/LinkedProject"; import { LinkingProjectFailed } from "./screens/LinkProject/LinkingProjectFailed"; import { LinkProject } from "./screens/LinkProject/LinkProject"; +import { ControlsProvider } from "./screens/VisualTests/ControlsContext"; import { VisualTests } from "./screens/VisualTests/VisualTests"; import { GitInfoPayload, LocalBuildProgress, UpdateStatusFunction } from "./types"; import { client, Provider, useAccessToken } from "./utils/graphQLClient"; @@ -143,17 +144,19 @@ export const Panel = ({ active, api }: PanelProps) => { return ( ); diff --git a/src/screens/Authentication/Authentication.stories.tsx b/src/screens/Authentication/Authentication.stories.tsx index 8ed8b273..b0630755 100644 --- a/src/screens/Authentication/Authentication.stories.tsx +++ b/src/screens/Authentication/Authentication.stories.tsx @@ -4,14 +4,15 @@ import { findByRole, userEvent } from "@storybook/testing-library"; import { http, HttpResponse } from "msw"; import { panelModes } from "../../modes"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; import { playAll } from "../../utils/playAll"; +import { storyWrapper } from "../../utils/storyWrapper"; import { withFigmaDesign } from "../../utils/withFigmaDesign"; import { Authentication } from "./Authentication"; const meta = { component: Authentication, - decorators: [storyWrapper], + decorators: [storyWrapper(GraphQLClientProvider)], args: { setAccessToken: action("setAccessToken"), hasProjectId: false, diff --git a/src/screens/GitNotFound/GitNotFound.stories.tsx b/src/screens/GitNotFound/GitNotFound.stories.tsx index 0b9371bc..d4445275 100644 --- a/src/screens/GitNotFound/GitNotFound.stories.tsx +++ b/src/screens/GitNotFound/GitNotFound.stories.tsx @@ -1,12 +1,12 @@ import type { Meta, StoryObj } from "@storybook/react"; -import React from "react"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; +import { storyWrapper } from "../../utils/storyWrapper"; import { GitNotFound } from "./GitNotFound"; const meta = { component: GitNotFound, - decorators: [storyWrapper], + decorators: [storyWrapper(GraphQLClientProvider)], args: { gitInfoError: new Error("Git info not found"), }, diff --git a/src/screens/LinkProject/LinkProject.stories.ts b/src/screens/LinkProject/LinkProject.stories.ts index 8418f636..d97dd524 100644 --- a/src/screens/LinkProject/LinkProject.stories.ts +++ b/src/screens/LinkProject/LinkProject.stories.ts @@ -5,14 +5,15 @@ import { delay, graphql, HttpResponse } from "msw"; import { SelectProjectsQueryQuery } from "../../gql/graphql"; import { panelModes } from "../../modes"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; import { playAll } from "../../utils/playAll"; +import { storyWrapper } from "../../utils/storyWrapper"; import { withFigmaDesign } from "../../utils/withFigmaDesign"; import { LinkProject } from "./LinkProject"; const meta = { component: LinkProject, - decorators: [storyWrapper], + decorators: [storyWrapper(GraphQLClientProvider)], args: { createdProjectId: undefined, onUpdateProject: action("updateProject"), diff --git a/src/screens/LinkProject/LinkedProject.stories.ts b/src/screens/LinkProject/LinkedProject.stories.ts index 8cf4f1a5..3d01c803 100644 --- a/src/screens/LinkProject/LinkedProject.stories.ts +++ b/src/screens/LinkProject/LinkedProject.stories.ts @@ -4,7 +4,8 @@ import { graphql, HttpResponse } from "msw"; import { ProjectQueryQuery } from "../../gql/graphql"; import { panelModes } from "../../modes"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; +import { storyWrapper } from "../../utils/storyWrapper"; import { withFigmaDesign } from "../../utils/withFigmaDesign"; import { LinkedProject } from "./LinkedProject"; @@ -22,7 +23,7 @@ const meta = { goToNext: action("goToNext"), setAccessToken: action("setAccessToken"), }, - decorators: [storyWrapper], + decorators: [storyWrapper(GraphQLClientProvider)], parameters: { chromatic: { modes: panelModes, diff --git a/src/screens/Onboarding/Onboarding.stories.tsx b/src/screens/Onboarding/Onboarding.stories.tsx index 2b17a136..fa18bab7 100644 --- a/src/screens/Onboarding/Onboarding.stories.tsx +++ b/src/screens/Onboarding/Onboarding.stories.tsx @@ -2,13 +2,14 @@ import type { Meta, StoryObj } from "@storybook/react"; import { graphql, HttpResponse } from "msw"; import { panelModes } from "../../modes"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; +import { storyWrapper } from "../../utils/storyWrapper"; import { withFigmaDesign } from "../../utils/withFigmaDesign"; import { Onboarding } from "./Onboarding"; const meta = { component: Onboarding, - decorators: [storyWrapper], + decorators: [storyWrapper(GraphQLClientProvider)], parameters: { chromatic: { modes: panelModes, diff --git a/src/screens/VisualTests/BuildContext.tsx b/src/screens/VisualTests/BuildContext.tsx new file mode 100644 index 00000000..42887962 --- /dev/null +++ b/src/screens/VisualTests/BuildContext.tsx @@ -0,0 +1,154 @@ +import React, { createContext, useEffect, useMemo } from "react"; +import { useQuery } from "urql"; + +import { getFragment } from "../../gql"; +import { StoryTestFieldsFragment, TestStatus } from "../../gql/graphql"; +import { GitInfoPayload } from "../../types"; +import { summarizeTests } from "../../utils/summarizeTests"; +import { statusMap } from "../../utils/testsToStatusUpdate"; +import { SelectedBuildInfo } from "../../utils/updateSelectedBuildInfo"; +import { useRequiredContext } from "../../utils/useRequiredContext"; +import { useTests } from "../../utils/useTests"; +import { useControlsDispatch } from "./ControlsContext"; +import { + FragmentLastBuildOnBranchBuildFields, + FragmentLastBuildOnBranchTestFields, + FragmentSelectedBuildFields, + FragmentStoryTestFields, + QueryBuild, +} from "./graphql"; + +export const useBuild = ({ + projectId, + storyId, + gitInfo, + selectedBuildInfo, +}: { + projectId: string; + storyId: string; + gitInfo: Pick< + GitInfoPayload, + "branch" | "slug" | "userEmailHash" | "commit" | "committedAt" | "uncommittedHash" + >; + selectedBuildInfo?: SelectedBuildInfo; +}) => { + const [{ data, error: queryError, operation }, rerunQuery] = useQuery({ + query: QueryBuild, + variables: { + projectId, + storyId, + testStatuses: Object.keys(statusMap) as any as TestStatus[], + branch: gitInfo.branch || "", + ...(gitInfo.slug ? { repositoryOwnerName: gitInfo.slug.split("/", 1)[0] } : {}), + gitUserEmailHash: gitInfo.userEmailHash, + selectedBuildId: selectedBuildInfo?.buildId || "", + hasSelectedBuildId: !!selectedBuildInfo, + }, + }); + + // Poll for updates + useEffect(() => { + const interval = setInterval(rerunQuery, 5000); + return () => clearInterval(interval); + }, [rerunQuery]); + + // When you change story, for a period the query will return the previous set of data, and indicate + // that with the operation being for the previous query. + const storyDataIsStale = operation && storyId && operation.variables.storyId !== storyId; + + const lastBuildOnBranch = getFragment( + FragmentLastBuildOnBranchBuildFields, + data?.project?.lastBuildOnBranch + ); + + const lastBuildOnBranchStoryTests = [ + ...getFragment( + FragmentLastBuildOnBranchTestFields, + lastBuildOnBranch && "testsForStory" in lastBuildOnBranch && lastBuildOnBranch.testsForStory + ? lastBuildOnBranch.testsForStory.nodes + : [] + ), + ]; + + // If the last build is *newer* than the current head commit, we don't want to select it + // as our local code wouldn't yet have the changes made in that build. + const lastBuildOnBranchIsNewer = lastBuildOnBranch?.committedAt > gitInfo.committedAt; + const lastBuildOnBranchIsSelectable = !!lastBuildOnBranch && !lastBuildOnBranchIsNewer; + + // If any tests for the current story are still in progress, we aren't ready to select the build + const lastBuildOnBranchIsReady = + !!lastBuildOnBranch && + lastBuildOnBranchStoryTests.every((t) => t.status !== TestStatus.InProgress); + + // If we didn't explicitly select a build, select the last build on the branch (if any) + const selectedBuild = getFragment( + FragmentSelectedBuildFields, + data?.selectedBuild ?? (lastBuildOnBranchIsReady ? data?.project?.lastBuildOnBranch : undefined) + ); + + return { + hasData: !!data && !storyDataIsStale, + hasProject: !!data?.project, + hasSelectedBuild: selectedBuild?.branch === gitInfo.branch, + lastBuildOnBranch, + lastBuildOnBranchIsNewer, + lastBuildOnBranchIsReady, + lastBuildOnBranchIsSelectable, + selectedBuild, + selectedBuildMatchesGit: + selectedBuild?.branch === gitInfo.branch && + selectedBuild?.commit === gitInfo.commit && + selectedBuild?.uncommittedHash === gitInfo.uncommittedHash, + rerunQuery, + queryError, + userCanReview: !!data?.viewer?.projectMembership?.userCanReview, + }; +}; + +type BuildInfo = ReturnType | null; +type SelectedStory = + | ({ + hasTests: boolean; + tests: StoryTestFieldsFragment[]; + summary: ReturnType; + } & ReturnType) + | null; + +export const BuildContext = createContext(null); +export const StoryContext = createContext(null); + +export const useBuildState = () => useRequiredContext(BuildContext, "Build"); +export const useSelectedBuildState = () => { + const { selectedBuild } = useRequiredContext(BuildContext, "Build"); + if (!selectedBuild) throw new Error("No selectedBuild on Build context"); + return selectedBuild; +}; +export const useSelectedStoryState = () => useRequiredContext(StoryContext, "Story"); + +export const BuildProvider = ({ + children, + watchState = null, +}: { + children: React.ReactNode; + watchState?: BuildInfo; +}) => { + const hasTests = !!watchState?.selectedBuild && "testsForStory" in watchState.selectedBuild; + const testsForStory = + watchState?.selectedBuild && + "testsForStory" in watchState.selectedBuild && + watchState.selectedBuild.testsForStory?.nodes; + const tests = [...getFragment(FragmentStoryTestFields, testsForStory || [])]; + const summary = summarizeTests(tests); + + const { toggleDiff } = useControlsDispatch(); + useEffect(() => toggleDiff(summary.changeCount > 0), [toggleDiff, summary.changeCount]); + + return ( + // eslint-disable-next-line react-hooks/exhaustive-deps + watchState, [JSON.stringify(watchState)])}> + + {children} + + + ); +}; diff --git a/src/screens/VisualTests/BuildResults.tsx b/src/screens/VisualTests/BuildResults.tsx index 3b6e3d97..36ae18ba 100644 --- a/src/screens/VisualTests/BuildResults.tsx +++ b/src/screens/VisualTests/BuildResults.tsx @@ -1,6 +1,6 @@ -import { Icons, Link, TooltipNote, WithTooltip } from "@storybook/components"; +import { Icons, Link } from "@storybook/components"; import { styled } from "@storybook/theming"; -import React, { useState } from "react"; +import React from "react"; import { BuildProgressInline } from "../../components/BuildProgressBarInline"; import { Button } from "../../components/Button"; @@ -9,19 +9,13 @@ import { Eyebrow } from "../../components/Eyebrow"; import { Heading } from "../../components/Heading"; import { Section, Sections } from "../../components/layout"; import { Text as CenterText } from "../../components/Text"; -import { getFragment } from "../../gql"; -import { - BuildStatus, - LastBuildOnBranchBuildFieldsFragment, - ReviewTestBatch, - SelectedBuildFieldsFragment, - StoryTestFieldsFragment, - TestResult, -} from "../../gql/graphql"; +import { BuildStatus, TestResult } from "../../gql/graphql"; import { LocalBuildProgress } from "../../types"; +import { useBuildState, useSelectedBuildState, useSelectedStoryState } from "./BuildContext"; import { BuildEyebrow } from "./BuildEyebrow"; -import { FragmentStoryTestFields } from "./graphql"; +import { useControlsDispatch, useControlsState } from "./ControlsContext"; import { RenderSettings } from "./RenderSettings"; +import { useReviewTestState } from "./ReviewTestContext"; import { SnapshotComparison } from "./SnapshotComparison"; import { Warnings } from "./Warnings"; @@ -29,16 +23,9 @@ interface BuildResultsProps { branch: string; dismissBuildError: () => void; localBuildProgress?: LocalBuildProgress; - selectedBuild: SelectedBuildFieldsFragment; - storyId: string; - lastBuildOnBranch?: LastBuildOnBranchBuildFieldsFragment; - lastBuildOnBranchCompletedStory: boolean; switchToLastBuildOnBranch?: () => void; + storyId: string; startDevBuild: () => void; - userCanReview: boolean; - isReviewing: boolean; - onAccept: (testId: StoryTestFieldsFragment["id"], batch?: ReviewTestBatch) => void; - onUnaccept: (testId: string) => Promise; setAccessToken: (accessToken: string | null) => void; } @@ -54,39 +41,31 @@ export const BuildResults = ({ branch, dismissBuildError, localBuildProgress, - lastBuildOnBranch, - lastBuildOnBranchCompletedStory, switchToLastBuildOnBranch, startDevBuild, - userCanReview, - isReviewing, - onAccept, - onUnaccept, - selectedBuild, storyId, setAccessToken, }: BuildResultsProps) => { - const [settingsVisible, setSettingsVisible] = useState(false); - const [warningsVisible, setWarningsVisible] = useState(false); - const [baselineImageVisible, setBaselineImageVisible] = useState(false); - const toggleBaselineImage = () => setBaselineImageVisible(!baselineImageVisible); + const { settingsVisible, warningsVisible } = useControlsState(); + const { toggleSettings, toggleWarnings } = useControlsDispatch(); + + const { lastBuildOnBranch, lastBuildOnBranchIsReady, lastBuildOnBranchIsSelectable } = + useBuildState(); + + const selectedBuild = useSelectedBuildState(); + const selectedStory = useSelectedStoryState(); + + const { buildIsReviewable, userCanReview } = useReviewTestState(); const isLocalBuildInProgress = - localBuildProgress && localBuildProgress.currentStep !== "complete"; - - const storyTests = [ - ...getFragment( - FragmentStoryTestFields, - selectedBuild && "testsForStory" in selectedBuild && selectedBuild.testsForStory - ? selectedBuild.testsForStory.nodes - : [] - ), - ]; - - const isReviewable = lastBuildOnBranch?.id === selectedBuild?.id; - const isStorySuperseded = !isReviewable && lastBuildOnBranchCompletedStory; + !!localBuildProgress && localBuildProgress.currentStep !== "complete"; + // Do we want to encourage them to switch to the next build? - const shouldSwitchToLastBuildOnBranch = isStorySuperseded && !!switchToLastBuildOnBranch; + const shouldSwitchToLastBuildOnBranch = + !buildIsReviewable && + lastBuildOnBranchIsReady && + lastBuildOnBranchIsSelectable && + !!switchToLastBuildOnBranch; const lastBuildOnBranchInProgress = lastBuildOnBranch?.status === BuildStatus.InProgress; const showBuildStatus = @@ -94,7 +73,7 @@ export const BuildResults = ({ isLocalBuildInProgress || // Even if there's no build running, we need to tell them why they can't review, unless // the story is superseded and the UI is already telling them - (!isReviewable && !shouldSwitchToLastBuildOnBranch); + (!buildIsReviewable && !shouldSwitchToLastBuildOnBranch); const localBuildProgressIsLastBuildOnBranch = localBuildProgress && localBuildProgress?.buildId === lastBuildOnBranch?.id; @@ -112,13 +91,12 @@ export const BuildResults = ({ /> ); - // If there are no tests yet, there is no baseline for this story. User needs to create one. - const isCompletelyNewStory = "testsForStory" in selectedBuild && storyTests.length === 0; + const isNewStory = selectedStory?.hasTests && selectedStory?.tests.length === 0; const isLocalBuildProgressOnSelectedBuild = selectedBuild.id !== `Build:${localBuildProgress?.buildId}`; - if (isCompletelyNewStory) { + if (isNewStory) { return (
@@ -151,7 +129,7 @@ export const BuildResults = ({ ); } // 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); + const isSkipped = !!selectedStory?.tests?.find((t) => t.result === TestResult.Skipped); if (isSkipped) { return ( @@ -183,14 +161,13 @@ export const BuildResults = ({ } const { status } = selectedBuild; - const startedAt = "startedAt" in selectedBuild && selectedBuild.startedAt; const isSelectedBuildStarting = [ BuildStatus.Announced, BuildStatus.Published, BuildStatus.Prepared, ].includes(status); const isBuildFailed = status === BuildStatus.Failed; - const isReviewLocked = status === BuildStatus.Pending && (!userCanReview || !isReviewable); + const isReviewLocked = status === BuildStatus.Pending && (!userCanReview || !buildIsReviewable); return ( @@ -219,25 +196,12 @@ export const BuildResults = ({ ); diff --git a/src/screens/VisualTests/BuildResultsFooter.tsx b/src/screens/VisualTests/BuildResultsFooter.tsx index 888f8aac..c74f28f8 100644 --- a/src/screens/VisualTests/BuildResultsFooter.tsx +++ b/src/screens/VisualTests/BuildResultsFooter.tsx @@ -3,57 +3,50 @@ import React from "react"; import { FooterMenu } from "../../components/FooterMenu"; import { IconButton } from "../../components/IconButton"; -import { Bar, Col, Section, Sections, Text } from "../../components/layout"; -import { SelectedBuildFieldsFragment } from "../../gql/graphql"; +import { Bar, Col, Section, Text } from "../../components/layout"; +import { useSelectedBuildState, useSelectedStoryState } from "./BuildContext"; +import { useControlsDispatch, useControlsState } from "./ControlsContext"; export const BuildResultsFooter = ({ - hasBaselineSnapshot, setAccessToken, - baselineImageVisible, - toggleBaselineImage, - selectedBuild, - setWarningsVisible, - warningsVisible, - setSettingsVisible, - settingsVisible, }: { - hasBaselineSnapshot: boolean; setAccessToken: (token: string | null) => void; - baselineImageVisible: boolean; - toggleBaselineImage: () => void; - selectedBuild: SelectedBuildFieldsFragment; - setWarningsVisible: (visible: boolean) => void; - warningsVisible: boolean; - setSettingsVisible: (visible: boolean) => void; - settingsVisible: boolean; -}) => ( -
- - {hasBaselineSnapshot && ( - - } - trigger="hover" - hasChrome={false} - > - toggleBaselineImage()}> - - - - - )} - - {baselineImageVisible ? ( - - Baseline Build {selectedBuild.number} on {selectedBuild.branch} - - ) : ( - - Latest Build {selectedBuild.number} on {selectedBuild.branch} - +}) => { + const { baselineImageVisible } = useControlsState(); + const { toggleBaselineImage } = useControlsDispatch(); + const selectedBuild = useSelectedBuildState(); + const storyState = useSelectedStoryState(); + + const hasBaselineSnapshot = !!storyState.selectedComparison?.baseCapture?.captureImage; + + return ( +
+ + {hasBaselineSnapshot && ( + + } + trigger="hover" + hasChrome={false} + > + toggleBaselineImage()}> + + + + )} - - {/* + + {baselineImageVisible ? ( + + Baseline Build {selectedBuild.number} on {selectedBuild.branch} + + ) : ( + + Latest Build {selectedBuild.number} on {selectedBuild.branch} + + )} + + {/* } trigger="hover" @@ -71,7 +64,7 @@ export const BuildResultsFooter = ({ */} - {/* + {/* } trigger="hover" @@ -90,9 +83,10 @@ export const BuildResultsFooter = ({ */} - - - - -
-); + + + + +
+ ); +}; diff --git a/src/screens/VisualTests/ControlsContext.tsx b/src/screens/VisualTests/ControlsContext.tsx new file mode 100644 index 00000000..d73fa940 --- /dev/null +++ b/src/screens/VisualTests/ControlsContext.tsx @@ -0,0 +1,70 @@ +import React, { createContext, useMemo, useReducer } from "react"; + +import { useRequiredContext } from "../../utils/useRequiredContext"; + +const initialControls = { + settingsVisible: false, + warningsVisible: false, + baselineImageVisible: false, + focusVisible: false, + diffVisible: false, +}; + +type State = typeof initialControls; + +const toggle = + (key: keyof State) => + (state: State, visible?: boolean): State => ({ + ...state, + [key]: typeof visible === "boolean" ? visible : !state[key], + }); + +const handlers = { + toggleDiff: toggle("diffVisible"), + toggleFocus: toggle("focusVisible"), + toggleSettings: toggle("settingsVisible"), + toggleWarnings: toggle("warningsVisible"), + toggleBaselineImage: toggle("baselineImageVisible"), +} as const; + +type Action = { type: keyof typeof handlers; payload?: any }; + +const controlsReducer = (state: State, action: Action) => + handlers[action.type](state, action.payload); + +export const ControlsContext = createContext(initialControls); +export const ControlsDispatchContext = createContext>(() => {}); + +export const useControlsState = () => useRequiredContext(ControlsContext, "Controls"); +export const useControlsDispatch = () => { + const dispatch = useRequiredContext(ControlsDispatchContext, "ControlsDispatch"); + return useMemo( + () => ({ + toggleDiff: (visible?: boolean) => dispatch({ type: "toggleDiff", payload: visible }), + toggleFocus: (visible?: boolean) => dispatch({ type: "toggleFocus", payload: visible }), + toggleSettings: (visible?: boolean) => dispatch({ type: "toggleSettings", payload: visible }), + toggleWarnings: (visible?: boolean) => dispatch({ type: "toggleWarnings", payload: visible }), + toggleBaselineImage: (visible?: boolean) => + dispatch({ type: "toggleBaselineImage", payload: visible }), + }), + [dispatch] + ); +}; + +export const ControlsProvider = ({ + children, + initialState = initialControls, +}: { + children: React.ReactNode; + initialState?: State; +}) => { + const [state, dispatch] = useReducer(controlsReducer, initialState); + + return ( + + + {children} + + + ); +}; diff --git a/src/screens/VisualTests/RenderSettings.stories.tsx b/src/screens/VisualTests/RenderSettings.stories.tsx index cb94e911..0b765e9a 100644 --- a/src/screens/VisualTests/RenderSettings.stories.tsx +++ b/src/screens/VisualTests/RenderSettings.stories.tsx @@ -3,13 +3,14 @@ import { graphql, HttpResponse } from "msw"; import { ProjectQueryQuery } from "../../gql/graphql"; import { panelModes } from "../../modes"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; +import { storyWrapper } from "../../utils/storyWrapper"; import { withFigmaDesign } from "../../utils/withFigmaDesign"; import { RenderSettings } from "./RenderSettings"; const meta = { component: RenderSettings, - decorators: [storyWrapper], + decorators: [storyWrapper(GraphQLClientProvider)], parameters: { chromatic: { modes: panelModes, diff --git a/src/screens/VisualTests/ReviewTestContext.tsx b/src/screens/VisualTests/ReviewTestContext.tsx new file mode 100644 index 00000000..49910e9d --- /dev/null +++ b/src/screens/VisualTests/ReviewTestContext.tsx @@ -0,0 +1,29 @@ +import React, { createContext } from "react"; + +import { ReviewTestBatch } from "../../gql/graphql"; +import { useRequiredContext } from "../../utils/useRequiredContext"; + +const initialState = { + isReviewing: false, + userCanReview: false, + buildIsReviewable: false, + acceptTest: (testId: string, batch: ReviewTestBatch = ReviewTestBatch.Spec) => Promise.resolve(), + unacceptTest: (testId: string, batch: ReviewTestBatch = ReviewTestBatch.Spec) => + Promise.resolve(), +}; + +type State = typeof initialState; + +export const ReviewTestContext = createContext(initialState); + +export const useReviewTestState = () => useRequiredContext(ReviewTestContext, "ReviewTest"); + +export const ReviewTestProvider = ({ + children, + watchState = initialState, +}: { + children: React.ReactNode; + watchState?: State; +}) => { + return {children}; +}; diff --git a/src/screens/VisualTests/SnapshotComparison.stories.tsx b/src/screens/VisualTests/SnapshotComparison.stories.tsx index 60111994..9c6f4b7f 100644 --- a/src/screens/VisualTests/SnapshotComparison.stories.tsx +++ b/src/screens/VisualTests/SnapshotComparison.stories.tsx @@ -1,58 +1,71 @@ import { action } from "@storybook/addon-actions"; import type { Meta, StoryObj } from "@storybook/react"; -import { screen, userEvent, within } from "@storybook/testing-library"; +import { findByRole, fireEvent, screen, userEvent, within } from "@storybook/testing-library"; +import type { StoryContext } from "@storybook/types"; import React, { ComponentProps } from "react"; import { Browser, ComparisonResult, - StoryTestFieldsFragment, + SelectedBuildFieldsFragment, TestResult, TestStatus, } from "../../gql/graphql"; import { panelModes } from "../../modes"; import { playAll } from "../../utils/playAll"; import { makeComparison, makeTest, makeTests } from "../../utils/storyData"; -import { interactionFailureTests, pendingBuild } from "./mocks"; +import { storyWrapper } from "../../utils/storyWrapper"; +import { BuildProvider } from "./BuildContext"; +import { ControlsProvider } from "./ControlsContext"; +import { interactionFailureTests, pendingBuild, pendingTests, withTests } from "./mocks"; +import { ReviewTestProvider } from "./ReviewTestContext"; import { SnapshotComparison } from "./SnapshotComparison"; +const build = { ...pendingBuild, startedAt: new Date() }; + +const buildInfo = (selectedBuild?: SelectedBuildFieldsFragment) => ({ + hasData: true, + hasProject: true, + hasSelectedBuild: !!selectedBuild, + lastBuildOnBranch: undefined, + lastBuildOnBranchIsNewer: false, + lastBuildOnBranchIsReady: false, + lastBuildOnBranchIsSelectable: false, + selectedBuild, + selectedBuildMatchesGit: true, + rerunQuery: () => {}, + queryError: undefined, + userCanReview: true, +}); + const meta = { component: SnapshotComparison, + decorators: [ + storyWrapper(ReviewTestProvider, (ctx) => ({ + watchState: { + isReviewing: false, + userCanReview: true, + buildIsReviewable: true, + acceptTest: action("acceptTest"), + unacceptTest: action("unacceptTest"), + ...ctx.parameters.reviewTest, + }, + })), + storyWrapper(BuildProvider, (ctx) => ({ watchState: buildInfo(ctx.parameters.selectedBuild) })), + storyWrapper(ControlsProvider), + ], args: { storyId: "button--primary", - tests: makeTests({ - browsers: [Browser.Chrome, Browser.Safari], - viewports: [ - { - status: TestStatus.Pending, - viewport: 480, - comparisonResults: [ComparisonResult.Changed, ComparisonResult.Equal], - }, - { status: TestStatus.Passed, viewport: 800 }, - { status: TestStatus.Passed, viewport: 1200 }, - ], - }), - startedAt: new Date(), - userCanReview: true, isStarting: false, isBuildFailed: false, - isReviewable: true, - isReviewing: false, - onAccept: action("onAccept"), - onUnaccept: action("onUnaccept"), - baselineImageVisible: false, shouldSwitchToLastBuildOnBranch: false, - selectedBuild: pendingBuild, - setSettingsVisible: action("setSettingsVisible"), - settingsVisible: false, - setWarningsVisible: action("setWarningsVisible"), - warningsVisible: false, setAccessToken: action("setAccessToken"), }, parameters: { chromatic: { modes: panelModes, }, + selectedBuild: withTests(build, pendingTests), }, } satisfies Meta; @@ -60,19 +73,22 @@ export default meta; type Story = StoryObj; export const InProgress = { - args: { - tests: makeTests({ - browsers: [Browser.Chrome, Browser.Safari], - viewports: [ - { - status: TestStatus.Pending, - viewport: 480, - comparisonResults: [ComparisonResult.Changed, ComparisonResult.Equal], - }, - { status: TestStatus.Passed, viewport: 800 }, - { status: TestStatus.InProgress, viewport: 1200 }, - ], - }), + parameters: { + selectedBuild: withTests( + build, + makeTests({ + browsers: [Browser.Chrome, Browser.Safari], + viewports: [ + { + status: TestStatus.Pending, + viewport: 480, + comparisonResults: [ComparisonResult.Changed, ComparisonResult.Equal], + }, + { status: TestStatus.Passed, viewport: 800 }, + { status: TestStatus.InProgress, viewport: 1200 }, + ], + }) + ), }, } satisfies Story; @@ -84,49 +100,55 @@ export const Default = {} satisfies Story; * test, which does not have a visual diff. */ export const FirstPassed: Story = { - args: { - tests: makeTests({ - browsers: [Browser.Chrome, Browser.Safari], - viewports: [ - { status: TestStatus.Passed, viewport: 800 }, - { - status: TestStatus.Pending, - viewport: 1200, - comparisonResults: [ComparisonResult.Equal, ComparisonResult.Changed], - }, - ], - }), + parameters: { + selectedBuild: withTests( + build, + makeTests({ + browsers: [Browser.Chrome, Browser.Safari], + viewports: [ + { status: TestStatus.Passed, viewport: 800 }, + { + status: TestStatus.Pending, + viewport: 1200, + comparisonResults: [ComparisonResult.Equal, ComparisonResult.Changed], + }, + ], + }) + ), }, } satisfies Story; export const StoryAdded: Story = { - args: { - tests: makeTests({ - browsers: [Browser.Chrome, Browser.Safari], - viewports: [ - { - status: TestStatus.Pending, - result: TestResult.Added, - viewport: 800, - comparisons: [ - makeComparison({ result: ComparisonResult.Added, baseCapture: null }), - makeComparison({ result: ComparisonResult.Added, baseCapture: null }), - ], - }, - ], - }), + parameters: { + selectedBuild: withTests( + build, + makeTests({ + browsers: [Browser.Chrome, Browser.Safari], + viewports: [ + { + status: TestStatus.Pending, + result: TestResult.Added, + viewport: 800, + comparisons: [ + makeComparison({ result: ComparisonResult.Added, baseCapture: null }), + makeComparison({ result: ComparisonResult.Added, baseCapture: null }), + ], + }, + ], + }) + ), }, }; export const ShowingBaseline: Story = { - args: { - baselineImageVisible: true, - }, + play: playAll(async ({ canvasElement }) => { + fireEvent.click(await findByRole(canvasElement, "button", { name: "Switch snapshot" })); + }), } satisfies Story; export const NoBaseline: Story = { - args: { - tests: [ + parameters: { + selectedBuild: withTests(build, [ makeTest({ status: TestStatus.Pending, comparisons: [ @@ -136,32 +158,38 @@ export const NoBaseline: Story = { }, ], }), - ], + ]), }, }; export const SwitchingMode = { - args: { - tests: makeTests({ - browsers: [Browser.Chrome, Browser.Safari], - viewports: [ - { status: TestStatus.Passed, viewport: 320 }, - { status: TestStatus.Passed, viewport: 600 }, - { status: TestStatus.Passed, viewport: 1200 }, - ], - }).map((test) => ({ - ...test, - comparisons: test.comparisons.map((comparison) => ({ - ...comparison, - headCapture: { - ...comparison.headCapture, - captureImage: { - imageUrl: `/ProjectItem-${comparison.browser.name}-${parseInt(test.mode.name, 10)}.png`, - imageWidth: parseInt(test.mode.name, 10), + parameters: { + selectedBuild: withTests( + build, + makeTests({ + browsers: [Browser.Chrome, Browser.Safari], + viewports: [ + { status: TestStatus.Passed, viewport: 320 }, + { status: TestStatus.Passed, viewport: 600 }, + { status: TestStatus.Passed, viewport: 1200 }, + ], + }).map((test) => ({ + ...test, + comparisons: test.comparisons.map((comparison) => ({ + ...comparison, + headCapture: { + ...comparison.headCapture, + captureImage: { + imageUrl: `/ProjectItem-${comparison.browser.name}-${parseInt( + test.mode.name, + 10 + )}.png`, + imageWidth: parseInt(test.mode.name, 10), + }, }, - }, - })), - })), + })), + })) + ), }, play: playAll(async ({ canvasElement, canvasIndex }) => { const canvas = within(canvasElement); @@ -173,7 +201,7 @@ export const SwitchingMode = { } satisfies Story; export const SwitchingBrowser = { - args: SwitchingMode.args, + parameters: SwitchingMode.parameters, play: playAll(async ({ canvasElement, canvasIndex }) => { const canvas = within(canvasElement); const menu = await canvas.findByRole("button", { name: "Chrome" }); @@ -184,16 +212,23 @@ export const SwitchingBrowser = { } satisfies Story; export const SwitchingTests = { - args: SwitchingMode.args, - render: function RenderSwitchingTests({ ...props }: ComponentProps) { - const [tests, setTests] = React.useState(); - if (!tests) setTimeout(() => setTests([makeTest({})]), 0); - return ; + parameters: SwitchingMode.parameters, + render: function RenderSwitchingTests( + { ...props }: ComponentProps, + { parameters }: StoryContext + ) { + const [activeBuild, setBuild] = React.useState(); + if (!activeBuild) setTimeout(() => setBuild(withTests(build, [makeTest({})])), 0); + return ( + + + + ); }, } satisfies Story; export const InteractionFailure = { - args: { - tests: interactionFailureTests, + parameters: { + selectedBuild: withTests(build, interactionFailureTests), }, }; diff --git a/src/screens/VisualTests/SnapshotComparison.tsx b/src/screens/VisualTests/SnapshotComparison.tsx index 35d26b3e..5f725a2c 100644 --- a/src/screens/VisualTests/SnapshotComparison.tsx +++ b/src/screens/VisualTests/SnapshotComparison.tsx @@ -1,21 +1,15 @@ import { Loader } from "@storybook/components"; import { Link } from "@storybook/design-system"; import { styled } from "@storybook/theming"; -import React, { useEffect, useState } from "react"; +import React from "react"; import { Text } from "../../components/layout"; import { SnapshotImage } from "../../components/SnapshotImage"; -import { - ComparisonResult, - ReviewTestBatch, - SelectedBuildFieldsFragment, - StoryTestFieldsFragment, - TestResult, - TestStatus, -} from "../../gql/graphql"; +import { ComparisonResult, TestResult, TestStatus } from "../../gql/graphql"; import { summarizeTests } from "../../utils/summarizeTests"; -import { useTests } from "../../utils/useTests"; +import { useSelectedBuildState, useSelectedStoryState } from "./BuildContext"; import { BuildResultsFooter } from "./BuildResultsFooter"; +import { useControlsDispatch, useControlsState } from "./ControlsContext"; import { SnapshotControls } from "./SnapshotControls"; import { StoryInfo } from "./StoryInfo"; @@ -109,60 +103,37 @@ const WarningText = styled(Text)(({ theme }) => ({ })); interface SnapshotSectionProps { - tests?: StoryTestFieldsFragment[]; - startedAt: Date; isStarting: boolean; startDevBuild: () => void; isBuildFailed: boolean; shouldSwitchToLastBuildOnBranch: boolean; switchToLastBuildOnBranch?: () => void; - userCanReview: boolean; - isReviewable: boolean; - isReviewing: boolean; - baselineImageVisible: boolean; - toggleBaselineImage: () => void; - onAccept: (testId: StoryTestFieldsFragment["id"], batch?: ReviewTestBatch) => void; - onUnaccept: (testId: StoryTestFieldsFragment["id"]) => void; setAccessToken: (accessToken: string | null) => void; - selectedBuild: SelectedBuildFieldsFragment; - setSettingsVisible: (visible: boolean) => void; - setWarningsVisible: (visible: boolean) => void; - settingsVisible: boolean; - warningsVisible: boolean; hidden?: boolean; storyId: string; } export const SnapshotComparison = ({ - tests = [], - startedAt, isStarting, startDevBuild, isBuildFailed, shouldSwitchToLastBuildOnBranch, switchToLastBuildOnBranch, - userCanReview, - isReviewable, - isReviewing, - onAccept, - onUnaccept, - baselineImageVisible, - toggleBaselineImage, setAccessToken, - selectedBuild, - setSettingsVisible, - setWarningsVisible, - settingsVisible, - warningsVisible, hidden, storyId, }: SnapshotSectionProps) => { - const [diffVisible, setDiffVisible] = useState(true); - const [focusVisible] = useState(false); - const testControls = useTests(tests); + const { baselineImageVisible, diffVisible, focusVisible } = useControlsState(); + const { toggleBaselineImage, toggleSettings, toggleWarnings } = useControlsDispatch(); + + const selectedBuild = useSelectedBuildState(); + const startedAt: Date = "startedAt" in selectedBuild && selectedBuild.startedAt; + + const selectedStory = useSelectedStoryState(); + const { tests } = selectedStory; const prevStoryIdRef = React.useRef(storyId); - const prevSelectedComparisonIdRef = React.useRef(testControls.selectedComparison?.id); + const prevSelectedComparisonIdRef = React.useRef(selectedStory.selectedComparison?.id); const prevSelectedBuildIdRef = React.useRef(selectedBuild.id); React.useEffect(() => { @@ -170,24 +141,23 @@ export const SnapshotComparison = ({ // This is most important for the baseline image toggle because baseline can not exist for a different story. if ( prevStoryIdRef.current !== storyId || - prevSelectedComparisonIdRef.current !== testControls.selectedComparison?.id || + prevSelectedComparisonIdRef.current !== selectedStory.selectedComparison?.id || prevSelectedBuildIdRef.current !== selectedBuild.id ) { - if (baselineImageVisible) toggleBaselineImage(); - setSettingsVisible(false); - setWarningsVisible(false); + toggleBaselineImage(false); + toggleSettings(false); + toggleWarnings(false); } - prevSelectedComparisonIdRef.current = testControls.selectedComparison?.id; + prevSelectedComparisonIdRef.current = selectedStory.selectedComparison?.id; prevStoryIdRef.current = storyId; prevSelectedBuildIdRef.current = selectedBuild.id; }, [ - baselineImageVisible, selectedBuild.id, - setSettingsVisible, - setWarningsVisible, storyId, - testControls, + selectedStory, toggleBaselineImage, + toggleSettings, + toggleWarnings, ]); const storyInfo = ( @@ -210,7 +180,7 @@ export const SnapshotComparison = ({ const testSummary = summarizeTests(tests); const { isInProgress } = testSummary; - const { selectedTest, selectedComparison } = testControls; + const { selectedTest, selectedComparison } = selectedStory; // isNewStory is when the story itself is added and all tests should also be added const isNewStory = tests.every( @@ -240,13 +210,7 @@ export const SnapshotComparison = ({ {storyInfo} - - + @@ -309,17 +273,7 @@ export const SnapshotComparison = ({ )} - + ); diff --git a/src/screens/VisualTests/SnapshotControls.stories.tsx b/src/screens/VisualTests/SnapshotControls.stories.tsx index 418ba8a2..9def328a 100644 --- a/src/screens/VisualTests/SnapshotControls.stories.tsx +++ b/src/screens/VisualTests/SnapshotControls.stories.tsx @@ -1,14 +1,16 @@ -import { action } from "@storybook/addon-actions"; import { expect } from "@storybook/jest"; import type { Meta, StoryObj } from "@storybook/react"; import { screen, userEvent, within } from "@storybook/testing-library"; -import React from "react"; import { Browser, ComparisonResult, TestStatus } from "../../gql/graphql"; import { panelModes } from "../../modes"; +import { action } from "../../utils/action"; import { playAll } from "../../utils/playAll"; import { makeTest, makeTests } from "../../utils/storyData"; +import { storyWrapper } from "../../utils/storyWrapper"; import { summarizeTests } from "../../utils/summarizeTests"; +import { ControlsProvider } from "./ControlsContext"; +import { ReviewTestProvider } from "./ReviewTestContext"; import { Grid } from "./SnapshotComparison"; import { SnapshotControls } from "./SnapshotControls"; @@ -20,6 +22,11 @@ const withTests = (tests: ReturnType) => ({ const meta = { component: SnapshotControls, + decorators: [ + storyWrapper(ReviewTestProvider, (ctx) => ({ watchState: ctx.parameters.reviewTest })), + storyWrapper(ControlsProvider, () => ({ initialState: { diffVisible: true } })), + storyWrapper(Grid), + ], args: { ...withTests( makeTests({ @@ -37,25 +44,18 @@ const meta = { ), onSelectMode: action("onSelectMode"), onSelectBrowser: action("onSelectBrowser"), - userCanReview: true, - isReviewable: true, - isReviewing: false, - onAccept: action("onAccept"), - onUnaccept: action("onUnaccept"), - diffVisible: true, - setDiffVisible: action("setDiffVisible"), }, - decorators: [ - (Story) => ( - - - - ), - ], parameters: { chromatic: { modes: panelModes, }, + reviewTest: { + isReviewing: false, + userCanReview: true, + buildIsReviewable: true, + acceptTest: action("acceptTest"), + unacceptTest: action("unacceptTest"), + }, }, } satisfies Meta; @@ -82,9 +82,12 @@ export const WithMultipleTestsInProgress = { } satisfies Story; export const WithSingleTestAccepting = { - args: { - ...WithSingleTest.args, - isReviewing: true, + args: WithSingleTest.args, + parameters: { + reviewTest: { + ...meta.parameters.reviewTest, + isReviewing: true, + }, }, } satisfies Story; @@ -93,9 +96,12 @@ export const WithSingleTestAccepted = { } satisfies Story; export const WithSingleTestUnreviewable = { - args: { - ...WithSingleTest.args, - isReviewable: false, + args: WithSingleTest.args, + parameters: { + reviewTest: { + ...meta.parameters.reviewTest, + buildIsReviewable: false, + }, }, } satisfies Story; @@ -140,9 +146,12 @@ export const BatchAcceptOptions = { } satisfies Story; export const BatchAcceptedBuild = { - play: playAll(BatchAcceptOptions, async ({ args, canvasIndex }) => { + play: playAll(BatchAcceptOptions, async ({ args, canvasIndex, parameters }) => { const items = await screen.findAllByText("Accept entire build"); await userEvent.click(items[canvasIndex]); - await expect(args.onAccept).toHaveBeenCalledWith(args.selectedTest.id, "BUILD"); + await expect(parameters.reviewTest.acceptTest).toHaveBeenCalledWith( + args.selectedTest.id, + "BUILD" + ); }), } satisfies Story; diff --git a/src/screens/VisualTests/SnapshotControls.tsx b/src/screens/VisualTests/SnapshotControls.tsx index be0e58f2..a3b51b48 100644 --- a/src/screens/VisualTests/SnapshotControls.tsx +++ b/src/screens/VisualTests/SnapshotControls.tsx @@ -16,6 +16,8 @@ import { } from "../../gql/graphql"; import { summarizeTests } from "../../utils/summarizeTests"; import { useTests } from "../../utils/useTests"; +import { useControlsDispatch, useControlsState } from "./ControlsContext"; +import { useReviewTestState } from "./ReviewTestContext"; const Controls = styled.div({ gridArea: "controls", @@ -56,13 +58,6 @@ interface SnapshotSectionProps { browserResults: TestSummary["browserResults"]; onSelectMode: TestControls["onSelectMode"]; onSelectBrowser: TestControls["onSelectBrowser"]; - userCanReview: boolean; - isReviewable: boolean; - isReviewing: boolean; - onAccept: (testId: StoryTestFieldsFragment["id"], batch?: ReviewTestBatch) => void; - onUnaccept: (testId: StoryTestFieldsFragment["id"]) => void; - diffVisible: boolean; - setDiffVisible: (visible: boolean) => void; } export const SnapshotControls = ({ @@ -70,19 +65,18 @@ export const SnapshotControls = ({ changeCount, selectedTest, selectedComparison, + isInProgress, modeResults, browserResults, onSelectMode, onSelectBrowser, - userCanReview, - isInProgress, - isReviewable, - isReviewing, - onAccept, - onUnaccept, - diffVisible, - setDiffVisible, }: SnapshotSectionProps) => { + const { diffVisible } = useControlsState(); + const { toggleDiff } = useControlsDispatch(); + + const { isReviewing, buildIsReviewable, userCanReview, acceptTest, unacceptTest } = + useReviewTestState(); + if (isInProgress) return ( @@ -124,7 +118,7 @@ export const SnapshotControls = ({ setDiffVisible(!diffVisible)} + onClick={() => toggleDiff(!diffVisible)} > @@ -134,7 +128,7 @@ export const SnapshotControls = ({ {(isAcceptable || isUnacceptable) && ( - {userCanReview && isReviewable && isAcceptable && ( + {userCanReview && buildIsReviewable && isAcceptable && ( <> } @@ -144,7 +138,7 @@ export const SnapshotControls = ({ onAccept(selectedTest.id)} + onClick={() => acceptTest(selectedTest.id)} > Accept @@ -156,7 +150,7 @@ export const SnapshotControls = ({ id: "acceptStory", title: "Accept story", center: "Accept all unreviewed changes to this story", - onClick: () => onAccept(selectedTest.id, ReviewTestBatch.Spec), + onClick: () => acceptTest(selectedTest.id, ReviewTestBatch.Spec), disabled: isReviewing, loading: isReviewing, }, @@ -164,7 +158,7 @@ export const SnapshotControls = ({ id: "acceptComponent", title: "Accept component", center: "Accept all unreviewed changes for this component", - onClick: () => onAccept(selectedTest.id, ReviewTestBatch.Component), + onClick: () => acceptTest(selectedTest.id, ReviewTestBatch.Component), disabled: isReviewing, loading: isReviewing, }, @@ -172,7 +166,7 @@ export const SnapshotControls = ({ id: "acceptBuild", title: "Accept entire build", center: "Accept all unreviewed changes for every story in the Storybook", - onClick: () => onAccept(selectedTest.id, ReviewTestBatch.Build), + onClick: () => acceptTest(selectedTest.id, ReviewTestBatch.Build), disabled: isReviewing, loading: isReviewing, }, @@ -191,20 +185,20 @@ export const SnapshotControls = ({ )} - {userCanReview && isReviewable && isUnacceptable && ( + {userCanReview && buildIsReviewable && isUnacceptable && ( } trigger="hover" hasChrome={false} > - onUnaccept(selectedTest.id)}> + unacceptTest(selectedTest.id)}> Unaccept )} - {!(userCanReview && isReviewable) && ( + {!(userCanReview && buildIsReviewable) && ( } trigger="hover" diff --git a/src/screens/VisualTests/VisualTests.stories.tsx b/src/screens/VisualTests/VisualTests.stories.tsx index 0ccf6e21..6a4f0460 100644 --- a/src/screens/VisualTests/VisualTests.stories.tsx +++ b/src/screens/VisualTests/VisualTests.stories.tsx @@ -5,8 +5,8 @@ import { action } from "@storybook/addon-actions"; import { expect } from "@storybook/jest"; import type { Meta, StoryObj } from "@storybook/react"; import { + findByLabelText, findByRole, - findByTestId, fireEvent, screen, userEvent, @@ -28,10 +28,12 @@ import { withGraphQLMutationParameters, withGraphQLQueryParameters, } from "../../utils/gqlStoryHelpers"; -import { storyWrapper } from "../../utils/graphQLClient"; +import { GraphQLClientProvider } from "../../utils/graphQLClient"; import { playAll } from "../../utils/playAll"; import { makeComparison, makeTest, makeTests } from "../../utils/storyData"; +import { storyWrapper } from "../../utils/storyWrapper"; import { withFigmaDesign } from "../../utils/withFigmaDesign"; +import { ControlsProvider } from "./ControlsContext"; import { QueryBuild } from "./graphql"; import { acceptedBuild, @@ -109,7 +111,7 @@ type StoryArgs = Parameters[0] & { const meta = { title: "screens/VisualTests/VisualTests", component: VisualTestsWithoutSelectedBuildId, - decorators: [storyWrapper], + decorators: [storyWrapper(ControlsProvider), storyWrapper(GraphQLClientProvider)], parameters: { chromatic: { modes: panelModes } }, argTypes: { addNotification: { type: "function", target: "manager-api" }, @@ -751,7 +753,7 @@ export const ToggleSnapshot: Story = { ), }, play: playAll(async ({ canvasElement }) => { - const button = await findByTestId(canvasElement, "button-toggle-snapshot"); + const button = await findByLabelText(canvasElement, "Switch snapshot"); await fireEvent.click(button); }), } satisfies Story; diff --git a/src/screens/VisualTests/VisualTests.tsx b/src/screens/VisualTests/VisualTests.tsx index 3b5fa126..9ec78629 100644 --- a/src/screens/VisualTests/VisualTests.tsx +++ b/src/screens/VisualTests/VisualTests.tsx @@ -1,31 +1,21 @@ import { useStorybookApi } from "@storybook/manager-api"; import type { API_StatusState } from "@storybook/types"; import React, { useCallback, useEffect, useState } from "react"; -import { useMutation, useQuery } from "urql"; +import { useMutation } from "urql"; import { getFragment } from "../../gql"; -import { - ReviewTestBatch, - ReviewTestInputStatus, - StoryTestFieldsFragment, - TestStatus, -} from "../../gql/graphql"; +import { ReviewTestBatch, ReviewTestInputStatus } from "../../gql/graphql"; import { GitInfoPayload, LocalBuildProgress, UpdateStatusFunction } from "../../types"; -import { statusMap, testsToStatusUpdate } from "../../utils/testsToStatusUpdate"; +import { testsToStatusUpdate } from "../../utils/testsToStatusUpdate"; import { SelectedBuildInfo, updateSelectedBuildInfo } from "../../utils/updateSelectedBuildInfo"; +import { BuildProvider, useBuild } from "./BuildContext"; import { BuildResults } from "./BuildResults"; -import { - FragmentLastBuildOnBranchBuildFields, - FragmentLastBuildOnBranchTestFields, - FragmentSelectedBuildFields, - FragmentStatusTestFields, - MutationReviewTest, - QueryBuild, -} from "./graphql"; +import { FragmentStatusTestFields, MutationReviewTest } from "./graphql"; import { NoBuild } from "./NoBuild"; +import { ReviewTestProvider } from "./ReviewTestContext"; const createEmptyStoryStatusUpdate = (state: API_StatusState) => { - return Object.fromEntries(Object.entries(state).map(([id, update]) => [id, null])); + return Object.fromEntries(Object.entries(state).map(([id]) => [id, null])); }; interface VisualTestsProps { @@ -45,6 +35,55 @@ interface VisualTestsProps { storyId: string; } +interface ReviewTestInput { + testId: string; + status: ReviewTestInputStatus.Accepted | ReviewTestInputStatus.Pending; + batch?: ReviewTestBatch; +} + +const useReview = ({ + buildIsReviewable, + userCanReview, + onReviewSuccess, + onReviewError, +}: { + buildIsReviewable: boolean; + userCanReview: boolean; + onReviewSuccess?: (input: ReviewTestInput) => void; + onReviewError?: (err: any, input: ReviewTestInput) => void; +}) => { + const [{ fetching: isReviewing }, runMutation] = useMutation(MutationReviewTest); + + const reviewTest = useCallback( + async (input: ReviewTestInput) => { + try { + if (!buildIsReviewable) throw new Error("Build is not reviewable"); + if (!userCanReview) throw new Error("No permission to review tests"); + const { error } = await runMutation({ input }); + if (error) throw error; + onReviewSuccess?.(input); + } catch (err) { + onReviewError?.(err, input); + } + }, + [onReviewSuccess, onReviewError, runMutation, buildIsReviewable, userCanReview] + ); + + const acceptTest = useCallback( + (testId: string, batch: ReviewTestBatch = ReviewTestBatch.Spec) => + reviewTest({ status: ReviewTestInputStatus.Accepted, testId, batch }), + [reviewTest] + ); + + const unacceptTest = useCallback( + (testId: string, batch: ReviewTestBatch = ReviewTestBatch.Spec) => + reviewTest({ status: ReviewTestInputStatus.Pending, testId, batch }), + [reviewTest] + ); + + return { isReviewing, acceptTest, unacceptTest, buildIsReviewable, userCanReview }; +}; + export const VisualTestsWithoutSelectedBuildId = ({ selectedBuildInfo, setSelectedBuildInfo, @@ -60,169 +99,99 @@ export const VisualTestsWithoutSelectedBuildId = ({ }: VisualTestsProps) => { const { addNotification } = useStorybookApi(); - const [{ data, error: queryError, operation }, rerunQuery] = useQuery({ - query: QueryBuild, - variables: { - projectId, - storyId, - testStatuses: Object.keys(statusMap) as any as TestStatus[], - branch: gitInfo.branch || "", - ...(gitInfo.slug ? { repositoryOwnerName: gitInfo.slug.split("/", 1)[0] } : {}), - gitUserEmailHash: gitInfo.userEmailHash, - selectedBuildId: selectedBuildInfo?.buildId || "", - hasSelectedBuildId: !!selectedBuildInfo, - }, - }); - - // When you change story, for a period the query will return the previous set of data, and indicate - // that with the operation being for the previous query. - const storyDataIsStale = operation && storyId && operation.variables.storyId !== storyId; - - // Poll for updates - useEffect(() => { - const interval = setInterval(rerunQuery, 5000); - return () => clearInterval(interval); - }, [rerunQuery]); - - const { userCanReview = false } = data?.viewer?.projectMembership || {}; - - const [{ fetching: isReviewing }, reviewTest] = useMutation(MutationReviewTest); - - const onReview = useCallback( - async ( - status: ReviewTestInputStatus.Accepted | ReviewTestInputStatus.Pending, - testId: string, - batch?: ReviewTestBatch - ) => { - try { - const { error: reviewError } = await reviewTest({ - input: { testId, status, batch }, + const buildInfo = useBuild({ projectId, storyId, gitInfo, selectedBuildInfo }); + + const { + hasData, + hasProject, + hasSelectedBuild, + lastBuildOnBranch, + lastBuildOnBranchIsReady, + lastBuildOnBranchIsSelectable, + selectedBuild, + selectedBuildMatchesGit, + queryError, + rerunQuery, + userCanReview, + } = buildInfo; + + const reviewState = useReview({ + buildIsReviewable: !!selectedBuild && selectedBuild.id === lastBuildOnBranch?.id, + userCanReview, + onReviewSuccess: rerunQuery, + onReviewError: (err, update) => { + if (err instanceof Error) { + addNotification({ + id: "chromatic/errorAccepting", + // @ts-expect-error we need a better API for not passing a link + link: undefined, + content: { + headline: `Failed to ${ + update.status === ReviewTestInputStatus.Accepted ? "accept" : "unaccept" + } changes`, + subHeadline: err.message, + }, + icon: { + name: "cross", + color: "red", + }, }); - - if (reviewError) { - throw reviewError; - } - rerunQuery(); - } catch (err) { - if (err instanceof Error) { - addNotification({ - id: "chromatic/errorAccepting", - // @ts-expect-error we need a better API for not passing a link - link: undefined, - content: { - headline: `Failed to ${ - status === ReviewTestInputStatus.Accepted ? "accept" : "unaccept" - } changes`, - subHeadline: err.message, - }, - icon: { - name: "cross", - color: "red", - }, - }); - } } }, - [addNotification, rerunQuery, reviewTest] - ); - - const onAccept = useCallback( - async (testId: StoryTestFieldsFragment["id"], batch?: ReviewTestBatch) => - onReview(ReviewTestInputStatus.Accepted, testId, batch), - [onReview] - ); - - const onUnaccept = useCallback( - async (testId: string) => onReview(ReviewTestInputStatus.Pending, testId), - [onReview] - ); - - const lastBuildOnBranch = getFragment( - FragmentLastBuildOnBranchBuildFields, - data?.project?.lastBuildOnBranch - ); - - const lastBuildOnBranchStoryTests = [ - ...getFragment( - FragmentLastBuildOnBranchTestFields, - lastBuildOnBranch && "testsForStory" in lastBuildOnBranch && lastBuildOnBranch.testsForStory - ? lastBuildOnBranch.testsForStory.nodes - : [] - ), - ]; - const lastBuildOnBranchCompletedStory = - !!lastBuildOnBranch && - lastBuildOnBranchStoryTests.every(({ status }) => status !== TestStatus.InProgress); - - // Before we set the storyInfo, we use the lastBuildOnBranch for story data if it's ready - const selectedBuild = getFragment( - FragmentSelectedBuildFields, - data?.selectedBuild ?? - (lastBuildOnBranchCompletedStory ? data?.project?.lastBuildOnBranch : undefined) - ); + }); - const selectedBuildHasCorrectBranch = selectedBuild?.branch === gitInfo.branch; // Currently only used by the sidebar button to show a blue dot ("build outdated") - const isOutdated = - !selectedBuildHasCorrectBranch || - selectedBuild?.commit !== gitInfo.commit || - selectedBuild?.uncommittedHash !== gitInfo.uncommittedHash; - useEffect(() => setOutdated(isOutdated), [isOutdated, setOutdated]); + useEffect(() => setOutdated(!selectedBuildMatchesGit), [selectedBuildMatchesGit, setOutdated]); - // If the next build is *newer* than the current commit, we don't want to switch to the build - const lastBuildOnBranchNewer = - lastBuildOnBranch && lastBuildOnBranch.committedAt > gitInfo.committedAt; - const canSwitchToLastBuildOnBranch = !!lastBuildOnBranch && !lastBuildOnBranchNewer; - - // We always set status to the next build's status, as when we change to a new story we'll see - // the next builds + // We always set status to the next build's status, as when we change to a new story we'll see the + // next builds. The status update is calculated outside useEffect so it only reruns when changed. const testsForStatus = lastBuildOnBranch && "testsForStatus" in lastBuildOnBranch && - lastBuildOnBranch.testsForStatus && + lastBuildOnBranch.testsForStatus?.nodes && getFragment(FragmentStatusTestFields, lastBuildOnBranch.testsForStatus.nodes); - - const buildStatusUpdate = - canSwitchToLastBuildOnBranch && testsForStatus ? testsToStatusUpdate(testsForStatus) : {}; - + const statusUpdate = lastBuildOnBranchIsSelectable && testsToStatusUpdate(testsForStatus || []); useEffect(() => { // @ts-expect-error The return type of this function is wrong in the API, it should allow `null` values updateBuildStatus((state) => ({ ...createEmptyStoryStatusUpdate(state), - ...buildStatusUpdate, + ...statusUpdate, })); - // We use the stringified version of buildStatusUpdate to do a deep diff // eslint-disable-next-line react-hooks/exhaustive-deps - }, [JSON.stringify(buildStatusUpdate), updateBuildStatus]); + }, [JSON.stringify(statusUpdate), updateBuildStatus]); - const shouldSwitchToLastBuildOnBranch = - canSwitchToLastBuildOnBranch && lastBuildOnBranchCompletedStory; - - // Ensure we are holding the right story build + // Auto-select the last build on branch if it's selectable and ready useEffect(() => { setSelectedBuildInfo((oldSelectedBuildInfo) => updateSelectedBuildInfo(oldSelectedBuildInfo, { - shouldSwitchToLastBuildOnBranch, + shouldSwitchToLastBuildOnBranch: lastBuildOnBranchIsSelectable && lastBuildOnBranchIsReady, lastBuildOnBranchId: lastBuildOnBranch?.id, storyId, }) ); - }, [shouldSwitchToLastBuildOnBranch, lastBuildOnBranch?.id, storyId, setSelectedBuildInfo]); + }, [ + lastBuildOnBranchIsSelectable, + lastBuildOnBranchIsReady, + lastBuildOnBranch?.id, + setSelectedBuildInfo, + storyId, + ]); const switchToLastBuildOnBranch = useCallback( () => - canSwitchToLastBuildOnBranch && + lastBuildOnBranch?.id && + lastBuildOnBranchIsSelectable && setSelectedBuildInfo({ buildId: lastBuildOnBranch.id, storyId }), - [setSelectedBuildInfo, canSwitchToLastBuildOnBranch, lastBuildOnBranch?.id, storyId] + [setSelectedBuildInfo, lastBuildOnBranchIsSelectable, lastBuildOnBranch?.id, storyId] ); - return !selectedBuildHasCorrectBranch || !selectedBuild || storyDataIsStale || queryError ? ( + return !selectedBuild || !hasSelectedBuild || !hasData || queryError ? ( ) : ( - + + + + + ); }; diff --git a/src/utils/action.ts b/src/utils/action.ts new file mode 100644 index 00000000..1f7458b9 --- /dev/null +++ b/src/utils/action.ts @@ -0,0 +1,9 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import { action as act, type ActionOptions, type HandlerFunction } from "@storybook/addon-actions"; +import { jest } from "@storybook/jest"; + +export const action = (name: string, options?: ActionOptions): HandlerFunction => { + const spy = jest.fn(act(name, options)); + Object.defineProperty(spy, "name", { value: name, writable: false }); + return spy; +}; diff --git a/src/utils/graphQLClient.tsx b/src/utils/graphQLClient.tsx index 7e51cff4..33248178 100644 --- a/src/utils/graphQLClient.tsx +++ b/src/utils/graphQLClient.tsx @@ -106,8 +106,6 @@ export const client = new Client({ fetchOptions: getFetchOptions(), // Auth header (token) is handled by authExchange }); -export const storyWrapper = (Story: any) => ( - - - -); +export const GraphQLClientProvider = ({ children }: { children: React.ReactNode }) => { + return {children}; +}; diff --git a/src/utils/storyWrapper.tsx b/src/utils/storyWrapper.tsx new file mode 100644 index 00000000..ecd33449 --- /dev/null +++ b/src/utils/storyWrapper.tsx @@ -0,0 +1,14 @@ +import type { StoryContext } from "@storybook/types"; +import React from "react"; + +export const storyWrapper = ( + Wrapper: React.FunctionComponent, + getProps?: (context: StoryContext) => any +) => + function StoryWrapper(Story: React.FunctionComponent, context: StoryContext) { + return ( + + + + ); + }; diff --git a/src/utils/useRequiredContext.ts b/src/utils/useRequiredContext.ts new file mode 100644 index 00000000..4d202609 --- /dev/null +++ b/src/utils/useRequiredContext.ts @@ -0,0 +1,7 @@ +import { useContext } from "react"; + +export const useRequiredContext = (context: React.Context, name: string) => { + const value = useContext(context); + if (value === null || value === undefined) throw new Error(`Missing context value for ${name}`); + return value as NonNullable; +}; diff --git a/src/utils/useTests.ts b/src/utils/useTests.ts index 81bcdb10..43776397 100644 --- a/src/utils/useTests.ts +++ b/src/utils/useTests.ts @@ -11,7 +11,7 @@ type ModeData = Pick; */ export function useTests(tests: StoryTestFieldsFragment[]) { const [selectedBrowserId, onSelectBrowserId] = useState( - tests[0]?.comparisons[0].browser.id + tests[0]?.comparisons[0]?.browser.id ); const [selectedModeName, onSelectModeName] = useState(tests[0]?.mode.name);