Skip to content

Commit

Permalink
Modernize Perseus' components folder stories (#1802)
Browse files Browse the repository at this point in the history
## Summary:

I spent some of Hackathon 2024 looking into a Storybook upgrade (unfinished). During this time, I read up on how Storybook [recommends](https://storybook.js.org/docs/get-started/whats-a-story) writing stories. The new format ([CSF](https://storybook.js.org/docs/api/csf)) makes Typescript understand the stories much better. 

So, it turns out, with a few changes, the Storybook tooling becomes a bunch more helpful and reduces how much code we need to write in our stories. I'll outline the types of changes this PR contains so that it's easy to follow if you want to adjust other stories in this repo. 

Here is what a very, simple, modern Storybook file looks like: 

```ts
import MyComponent from "./my-component";

import type {Meta, StoryObj> from "@storybook/react";

const meta: Meta = {
    title: "Perseus/Components/MyComponent",
    component: MyComponent,
}
export default meta; 

type Story = StoryObj<typeof MyComponent>;

export const Default: Story = {};
```

1. Always define the default export as: 

   ```
   const meta: Meta = {
       title: "..path where this component should show up in storybook", 
       component: MyComponent
   }
   export default meta; 
   ```

   Two things are important here: 

     1. The default export is typed as `Meta` (from `@storybook/react`). This enables Storybook to infer the props this component has and enables "intellisense" on the different parameters to `meta`.
     1. We _do not_ use `as Meta` on this object definition. I'm unclear why, but that doesn't seem to work as well as the example above. 

1. Always define a type in the file as `type Story = StoryObj<typeof MyComponent>`. (again, the `StoryObj<T>` type comes from `@storybook/react`. 

3. Define stories as `export const MyStory: Story = {}`. By defining stories as simple data objects we get better type checking on the args we provide and reduce alot of boilerplate that our stories typically had. 

4. Use [`render`](https://github.com/Khan/perseus/pull/1802/files#diff-f821d3794fd8ed90901679eb1306920bc224c9b028b4f5bc8eae31f9b2e1fc62R15) and/or [`decorators`](https://github.com/Khan/perseus/pull/1802/files#diff-096ad8bf05853f7a13ab3a8d0ec4f50e7799d835babcf1275730e6571f8e0c7cR25) to reduce duplication. Often times we need to wrap our component in order for it to behave properly or to look better in Storybook. Instead of writing each story with this wrapper, move the common wrapping code into the `meta` object's `render` key. (note: I'm not 100% clear when to use the `render` key and when to use the `decorators`. I was able to accomplish similar things with both. I _suspect_ `decorators` is more useful if you have multiple things you want to adorn the component with but do not want to merge them together into a single `render` function.

Issue: "none"

## Test plan:

Review the stories in `Perseus/components` (`yarn start; open http://localhost:6006/?path=/docs/perseus-components-button-group--docs`)

Author: jeremywiebe

Reviewers: catandthemachines, nishasy, jandrade, jeremywiebe, #perseus, mark-fitzgerald

Required Reviewers:

Approved By: catandthemachines, nishasy

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1802
  • Loading branch information
jeremywiebe authored Oct 31, 2024
1 parent 33891dc commit d6381f7
Show file tree
Hide file tree
Showing 45 changed files with 823 additions and 886 deletions.
6 changes: 6 additions & 0 deletions .changeset/old-lamps-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/math-input": patch
"@khanacademy/perseus": patch
---

Improve prop types for various components
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ MafsWithLockedFiguresCurrent.parameters = {
chromatic: {
// Disabling because this isn't visually testing anything on the
// initial load of the editor page.
disable: true,
disableSnapshot: true,
},
};

Expand Down Expand Up @@ -413,7 +413,7 @@ WithSaveWarnings.parameters = {
// Disabling because this isn't testing anything visually on the
// editor page. It's testing the error message, which don't
// even show up on the initial load.
disable: true,
disableSnapshot: true,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Controlled.parameters = {
chromatic: {
// Disable the snapshot for this story because it's testing
// behavior, not visuals.
disable: true,
disableSnapshot: true,
},
};

Expand Down Expand Up @@ -77,6 +77,6 @@ LongPageScroll.parameters = {
chromatic: {
// Disable the snapshot for this story because it's testing
// behavior, not visuals.
disable: true,
disableSnapshot: true,
},
};
27 changes: 11 additions & 16 deletions packages/perseus/src/components/__stories__/graph.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,22 @@
import * as React from "react";

import Graph from "../graph";

import type {StoryObj, Meta} from "@storybook/react";

type StoryArgs = StoryObj<Graph>;

type Story = Meta<Graph>;
type Story = StoryObj<typeof Graph>;

const size = 200;

export default {
const meta: Meta = {
title: "Perseus/Components/Graph",
} as Story;

export const SquareBoxSizeAndOtherwiseEmpty = (
args: StoryArgs,
): React.ReactElement => {
return <Graph box={[size, size]} />;
component: Graph,
args: {
box: [size, size],
},
};
export default meta;

export const SquareBoxSizeAndOtherwiseEmpty: Story = {};

export const LabeledSquaredBox = (args: StoryArgs): React.ReactElement => {
return (
<Graph box={[size, size]} labels={["First label", "Second label"]} />
);
export const LabeledSquaredBox: Story = {
args: {labels: ["First label", "Second label"]},
};
33 changes: 16 additions & 17 deletions packages/perseus/src/components/__stories__/graphie.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,27 @@ import Graphie from "../graphie";

import type {StoryObj, Meta} from "@storybook/react";

type StoryArgs = StoryObj<Graphie>;

type Story = Meta<Graphie>;
type Story = StoryObj<typeof Graphie>;

const size = 200;

export default {
const meta: Meta = {
title: "Perseus/Components/Graphie",
} as Story;

export const SquareBoxSizeAndOtherwiseEmpty = (
args: StoryArgs,
): React.ReactElement => {
return (
<Graphie
box={[size, size]}
setDrawingAreaAvailable={() => {}}
setup={() => {}}
/>
);
component: Graphie,
args: {
box: [size, size],
setup: () => {},
setDrawingAreaAvailable: () => {},
},
};
export default meta;

export const SquareBoxSizeAndOtherwiseEmpty: Story = {};

export const PieChartGraphieLabels = (args: StoryArgs): React.ReactElement => {
/**
* A demonstration of a Graphie rendered using the Perseus `Renderer` complete
* with overlaid labels and an image caption below.
*/
export const PieChartGraphieLabels = () => {
return <ServerItemRendererWithDebugUI item={itemWithPieChart} />;
};
38 changes: 12 additions & 26 deletions packages/perseus/src/components/__stories__/hud.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,35 +1,21 @@
import * as React from "react";
import {action} from "@storybook/addon-actions";

import Hud from "../hud";

import type {StoryObj, Meta} from "@storybook/react";

type StoryArgs = StoryObj<typeof Hud>;
type Story = StoryObj<typeof Hud>;

type Story = Meta<typeof Hud>;

export default {
const meta: Meta = {
title: "Perseus/Components/HUD",
} as Story;

export const TestMessageDisabled = (args: StoryArgs): React.ReactElement => {
return (
<Hud
fixedPosition={false}
message="Test message"
enabled={false}
onClick={() => {}}
/>
);
component: Hud,
args: {
enabled: true,
fixedPosition: false,
message: "Test message",
onClick: action("onClick"),
},
};
export default meta;

export const TestMessageEnabled = (args: StoryArgs): React.ReactElement => {
return (
<Hud
fixedPosition={false}
message="Test message"
enabled={true}
onClick={() => {}}
/>
);
};
export const Default: Story = {};
27 changes: 12 additions & 15 deletions packages/perseus/src/components/__stories__/icon.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import * as React from "react";

import * as IconPaths from "../../icon-paths";
import IconComponent from "../icon";

import type {StoryObj, Meta} from "@storybook/react";

type StoryArgs = StoryObj<IconComponent>;

type Story = Meta<IconComponent>;
type Story = StoryObj<typeof IconComponent>;

export default {
title: "Perseus/Components",
const meta: Meta = {
title: "Perseus/Components/Icon",
component: IconComponent,
args: {
color: "#808",
size: 25,
Expand All @@ -27,12 +24,12 @@ export default {
control: "select",
},
},
} as Story;
};
export default meta;

export const Icon = (args: StoryArgs): React.ReactElement => (
<IconComponent
style={{display: "block"}}
icon={IconPaths.iconCheck}
{...args}
/>
);
export const Icon: Story = {
args: {
style: {display: "block"},
icon: IconPaths.iconCheck,
},
};
Original file line number Diff line number Diff line change
@@ -1,60 +1,48 @@
/* eslint-disable @khanacademy/ts-no-error-suppressions */
import * as React from "react";

type StoryArgs = Record<any, any>;

type Story = {
title: string;
};

import ImageLoader from "../image-loader";

import type {Meta, StoryObj} from "@storybook/react";

const svgUrl = "http://www.khanacademy.org/images/ohnoes-concerned.svg";
const imgUrl = "https://www.khanacademy.org/images/hand-tree.new.png";

export default {
const meta: Meta = {
title: "Perseus/Components/Image Loader",
} as Story;

export const SvgImage = (args: StoryArgs): React.ReactElement => {
return (
<ImageLoader
src={svgUrl}
preloader={null}
imgProps={{
alt: "ALT",
}}
onUpdate={() => {}}
/>
);
component: ImageLoader,
args: {
preloader: null,
imgProps: {
alt: "ALT",
},
onUpdate: () => {},
},
parameters: {
chromatic: {
// This component only deals with loading images and providing a
// fallback if it fails. This is not very useful to snapshot so
// we're disabling it.
disableSnapshot: true,
},
},
};
export default meta;

type Story = StoryObj<typeof ImageLoader>;

export const SvgImage: Story = {
args: {
src: svgUrl,
},
};

export const PngImage = (args: StoryArgs): React.ReactElement => {
return (
<ImageLoader
src={imgUrl}
preloader={null}
imgProps={{
alt: "ALT",
}}
onUpdate={() => {}}
/>
);
export const PngImage: Story = {
args: {src: imgUrl},
};

export const InvalidImageWithChildrenForFailedLoading = (
args: StoryArgs,
): React.ReactElement => {
return (
<ImageLoader
src="http://abcdefiahofshiaof.noway.badimage.com"
preloader={null}
imgProps={{
alt: "ALT",
}}
onUpdate={() => {}}
>
<span>You can see me! The image failed to load.</span>
</ImageLoader>
);
export const InvalidImageWithChildrenForFailedLoading: Story = {
args: {
src: "http://abcdefiahofshiaof.noway.badimage.com",
children: <span>You can see me! The image failed to load.</span>,
},
};
50 changes: 28 additions & 22 deletions packages/perseus/src/components/__stories__/info-tip.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,40 @@ import * as React from "react";

import InfoTip from "../info-tip";

type StoryArgs = Record<any, any>;
import type {Meta, StoryObj} from "@storybook/react";

type Story = {
title: string;
const meta: Meta = {
title: "Perseus/Components/Info Tip",
component: InfoTip,
};
export default meta;

export default {
title: "Perseus/Components/Info Tip",
} as Story;
type Story = StoryObj<typeof InfoTip>;

export const TextOnMouseover = (args: StoryArgs): React.ReactElement => {
return <InfoTip>Sample text</InfoTip>;
export const TextOnMouseover: Story = {
args: {
children: "Sample text",
},
};

export const CodeInText = (args: StoryArgs): React.ReactElement => {
return (
<InfoTip>
Settings that you add here are available to the program as an object
returned by <code>Program.settings()</code>
</InfoTip>
);
export const CodeInText: Story = {
args: {
children: (
<>
Settings that you add here are available to the program as an
object returned by <code>Program.settings()</code>
</>
),
},
};

export const MultipleElements = (args: StoryArgs): React.ReactElement => {
return (
<InfoTip>
<p>First paragraph</p>
<p>Second paragraph</p>
</InfoTip>
);
export const MultipleElements: Story = {
args: {
children: (
<>
<p>First paragraph</p>
<p>Second paragraph</p>
</>
),
},
};
Loading

0 comments on commit d6381f7

Please sign in to comment.