Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement horizontal panel layout #109

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions .storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ const Panel = styled.div<{ orientation: "right" | "bottom" }>(
overflow: "auto",
}),
({ theme }) => ({
containerType: "size",
containerName: "storybookRoot",
position: "relative",
outline: `1px solid ${theme.color.border}`,
outline: `1px solid ${theme.appBorderColor}`,
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
background: theme.background.content,
color: theme.color.defaultText,
fontSize: theme.typography.size.s2 - 1,
Expand All @@ -70,7 +72,16 @@ const withTheme = (StoryFn, { globals, parameters }) => {
return theme === "light" || theme === "dark" ? (
<ThemeProvider theme={convert(themes[theme])}>
<Global styles={createReset} />
<Global styles={{ "#storybook-root": { height: "100vh", padding: 0 } }}></Global>
<Global
styles={{
"#storybook-root": {
height: "100vh",
padding: 0,
containerType: "size",
containerName: "storybookRoot",
},
}}
></Global>
<ThemedSetRoot />
<StoryFn />
</ThemeProvider>
Expand All @@ -94,7 +105,7 @@ const withTheme = (StoryFn, { globals, parameters }) => {
</Panels>
</>
);
}
};

const preview: Preview = {
decorators: [withTheme],
Expand Down Expand Up @@ -132,7 +143,7 @@ const preview: Preview = {
],
},
},
}
}
},
};

export default preview;
export default preview;
2 changes: 1 addition & 1 deletion src/components/Accordions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const Accordions = styled.div({
export const Accordion = styled.div(({ theme }) => ({
padding: 15,
lineHeight: "18px",
borderBottom: `1px solid ${theme.color.border}`,
borderBottom: `1px solid ${theme.appBorderColor}`,

p: {
margin: "10px 0",
Expand Down
3 changes: 3 additions & 0 deletions src/components/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ export const Button = styled(BaseButton)<{
lineHeight: "14px",
padding: "9px 12px",
alignItems: "center",
"@container (min-width: 800px)": {
padding: "8px 10px",
},
svg: {
marginRight: 6,
},
Expand Down
2 changes: 1 addition & 1 deletion src/components/Placeholder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const Placeholder = styled.div<{
height?: number;
marginLeft?: number;
marginRight?: number;
}>(({ theme, width = 14, height = 14, marginLeft = 7, marginRight = 14 }) => ({
}>(({ theme, width = 14, height = 14, marginLeft = 7, marginRight = 8 }) => ({
display: "inline-block",
backgroundColor: theme.appBorderColor,
borderRadius: 3,
Expand Down
5 changes: 1 addition & 4 deletions src/components/icons/BackIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ export const BackIcon = (props: any) => (
xmlns="http://www.w3.org/2000/svg"
{...props}
>
<path
d="M2.7591 7.0955C2.77741 7.19005 2.82317 7.28033 2.89639 7.35355L8.39639 12.8536C8.59166 13.0488 8.90824 13.0488 9.1035 12.8536C9.29876 12.6583 9.29876 12.3417 9.1035 12.1464L3.95705 7L9.1035 1.85355C9.29876 1.65829 9.29876 1.34171 9.1035 1.14645C8.90824 0.951184 8.59166 0.951184 8.39639 1.14645L2.8969 6.64594C2.77486 6.76798 2.72859 6.93792 2.7591 7.0955Z"
fill="#73828C"
/>
<path d="M2.7591 7.0955C2.77741 7.19005 2.82317 7.28033 2.89639 7.35355L8.39639 12.8536C8.59166 13.0488 8.90824 13.0488 9.1035 12.8536C9.29876 12.6583 9.29876 12.3417 9.1035 12.1464L3.95705 7L9.1035 1.85355C9.29876 1.65829 9.29876 1.34171 9.1035 1.14645C8.90824 0.951184 8.59166 0.951184 8.39639 1.14645L2.8969 6.64594C2.77486 6.76798 2.72859 6.93792 2.7591 7.0955Z" />
</svg>
);
2 changes: 1 addition & 1 deletion src/components/icons/StatusIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const StatusIcon = styled(Icons)<{ icon: "passed" | "changed" | "failed"
({ icon, theme }) => ({
width: 12,
height: 12,
margin: "3px 6px",
margin: "3px 3px 3px 6px",
verticalAlign: "top",

color: {
Expand Down
44 changes: 34 additions & 10 deletions src/components/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,33 @@ export const Sections = styled.div<{ hidden?: boolean }>(

export const Section = styled.div<{ grow?: boolean }>(({ grow, theme }) => ({
flexGrow: grow ? 1 : "auto",
borderBottom: `1px solid ${theme.color.border}`,
borderBottom: `1px solid ${theme.appBorderColor}`,
"&:last-of-type": {
borderBottom: 0,
},
}));

export const Row = styled.div({
display: "flex",
flexDirection: "row",
margin: 15,
});
export const Row = styled.div<{ header?: boolean }>(
{
display: "flex",
flexDirection: "row",
margin: 15,
},
({ header, theme }) =>
header && {
margin: 0,
padding: 15,
borderBottom: `1px solid ${theme.appBorderColor}`,

"@container (min-width: 800px)": {
height: 40,
alignItems: "center",
justifyContent: "space-between",
backgroundColor: theme.background.app,
padding: "5px 15px",
},
}
);

export const Bar = styled(Row)({
alignItems: "center",
Expand All @@ -34,9 +50,6 @@ export const Col = styled.div<{ push?: boolean }>(
display: "flex",
flexDirection: "column",
alignItems: "center",
"&:not(:last-of-type)": {
marginRight: 6,
},
},
({ push }) => push && { marginLeft: "auto" }
);
Expand All @@ -49,7 +62,7 @@ export const Text = styled.div(({ theme }) => ({
},
code: {
fontSize: theme.typography.size.s1,
border: `1px solid ${theme.color.border}`,
border: `1px solid ${theme.appBorderColor}`,
borderRadius: 3,
padding: "0 3px",
},
Expand All @@ -62,4 +75,15 @@ export const Text = styled.div(({ theme }) => ({
svg: {
verticalAlign: "top",
},
"@container (min-width: 800px)": {
br: {
verticalAlign: "top",
display: "inline-block",
content: "''",
background: theme.appBorderColor,
width: 1,
height: "100%",
margin: "0 6px",
},
},
}));
21 changes: 7 additions & 14 deletions src/screens/VisualTests/BuildResults.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export const BuildResults = ({
)}

<Section grow hidden={settingsVisible || warningsVisible}>
<StoryInfo
<SnapshotComparison
{...{
tests: storyTests,
startedAt,
Expand All @@ -174,21 +174,14 @@ export const BuildResults = ({
isBuildFailed,
shouldSwitchToLastBuildOnBranch,
switchToLastBuildOnBranch,
userCanReview,
isReviewable,
isReviewing,
onAccept,
onUnaccept,
baselineImageVisible,
}}
/>
{!isSelectedBuildStarting && storyTests && storyTests.length > 0 && (
<SnapshotComparison
{...{
tests: storyTests,
userCanReview,
isReviewable,
isReviewing,
onAccept,
onUnaccept,
baselineImageVisible,
}}
/>
)}
</Section>

<Section grow hidden={!settingsVisible}>
Expand Down
125 changes: 20 additions & 105 deletions src/screens/VisualTests/SnapshotComparison.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
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, { ComponentProps } from "react";

import {
Browser,
CaptureErrorKind,
ComparisonResult,
StoryTestFieldsFragment,
TestStatus,
} from "../../gql/graphql";
import { Browser, ComparisonResult, StoryTestFieldsFragment, TestStatus } from "../../gql/graphql";
import { playAll } from "../../utils/playAll";
import { makeTest, makeTests } from "../../utils/storyData";
import { interactionFailureTests } from "./mocks";
import { SnapshotComparison } from "./SnapshotComparison";

const meta = {
Expand All @@ -30,12 +24,16 @@ const meta = {
{ 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,
},
} satisfies Meta<typeof SnapshotComparison>;

Expand All @@ -47,21 +45,26 @@ export const InProgress = {
tests: makeTests({
browsers: [Browser.Chrome, Browser.Safari],
viewports: [
{ status: TestStatus.InProgress, viewport: 480 },
{ status: TestStatus.InProgress, viewport: 800 },
{
status: TestStatus.Pending,
viewport: 480,
comparisonResults: [ComparisonResult.Changed, ComparisonResult.Equal],
},
{ status: TestStatus.Passed, viewport: 800 },
{ status: TestStatus.InProgress, viewport: 1200 },
],
}),
},
} satisfies Story;

export const WithMultipleTests = {} satisfies Story;
export const Default = {} satisfies Story;

/**
* Sort of confusing situation where the only comparison with changes (1200px/Saf) is on the
* "opposite" side of the current comparison (800px/Chrome)
* Sort of confusing situation where the only comparison with changes (1200px/Safari) is on the
* "opposite" side of the current comparison (800px/Chrome). In this case we still show the first
* test, which does not have a visual diff.
*/
export const WithMultipleTestsFirstPassed = {
export const FirstPassed: Story = {
args: {
tests: makeTests({
browsers: [Browser.Chrome, Browser.Safari],
Expand All @@ -77,35 +80,8 @@ export const WithMultipleTestsFirstPassed = {
},
} satisfies Story;

export const WithSingleTest = {
args: {
tests: [makeTest({ status: TestStatus.Pending })],
},
} satisfies Story;

export const WithSingleTestAccepting = {
args: {
...WithSingleTest.args,
isReviewing: true,
},
} satisfies Story;

export const WithSingleTestAccepted = {
args: {
tests: [makeTest({ status: TestStatus.Accepted })],
},
} satisfies Story;

export const WithSingleTestOutdated = {
export const ShowingBaseline: Story = {
args: {
...WithSingleTest.args,
isReviewable: false,
},
} satisfies Story;

export const WithSingleTestShowingBaseline = {
args: {
tests: [makeTest({ status: TestStatus.Pending })],
baselineImageVisible: true,
},
} satisfies Story;
Expand Down Expand Up @@ -164,67 +140,6 @@ export const SwitchingTests = {

export const InteractionFailure = {
args: {
tests: [
makeTest({
status: TestStatus.Broken,
captureError: {
kind: CaptureErrorKind.InteractionFailure,
error: {
name: "Error",
message: `Unable to find an element by: [data-testid="button-toggle-snapshot"]`,
stack: `Error: Unable to find an element by: [data-testid="button-toggles-snapshot"]

Ignored nodes: comments, script, style
<div
class="css-nlyae3"
data-canvas="right"
orientation="right"
>
<div
class="css-1g4yje1"
>
<div
class="css-3fce27"
>
<div
class="css-1o56ikb"
>
<div
class="css-gghy96"
>
<div
class="css-k4d9wy"
>
<b>
1 change
</b>
<svg
class="css-1g8ys9d css-6m3b1s-Svg e82dnwa0"
height="14px"
viewBox="0 0 14 14"
width="14px"
>`,
},
},
}),
],
tests: interactionFailureTests,
},
} satisfies Story;

export const BatchAcceptOptions = {
args: WithSingleTest.args,
play: playAll(async ({ canvasElement }) => {
const canvas = within(canvasElement);
const menu = await canvas.findByRole("button", { name: "Batch accept" });
await userEvent.click(menu);
}),
} satisfies Story;

export const BatchAcceptedBuild = {
args: WithSingleTest.args,
play: playAll(BatchAcceptOptions, async ({ args, canvasIndex }) => {
const items = await screen.findAllByText("Accept entire build");
await userEvent.click(items[canvasIndex]);
await expect(args.onAccept).toHaveBeenCalledWith(args.tests[0].id, "BUILD");
}),
} satisfies Story;
};
Loading
Loading