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

chore: upgrade to storybook 7.x #1726

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
18 changes: 18 additions & 0 deletions .babelrc.json
booc0mtaco marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"sourceType": "unambiguous",
"presets": [
[
"@babel/preset-env",
{
"targets": {
"chrome": 100,
"safari": 15,
"firefox": 91
}
}
],
"@babel/preset-typescript",
"@babel/preset-react"
],
"plugins": []
}
2 changes: 2 additions & 0 deletions .storybook/components/Docs/GettingStarted.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Getting Started" />

# Welcome to EDS
Expand Down
35 changes: 18 additions & 17 deletions .storybook/components/Docs/Guidelines/CodeGuidelines.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Documentation/Guidelines/Code Guidelines" />

# Code Guidelines
Expand Down Expand Up @@ -281,7 +283,7 @@ Use:

Instead of:

```tsx
```
import { EdsThemeColorUtilitySuccessForeground } from 'src/tokens-dist/ts/colors';

<Icon color={EdsThemeColorUtilitySuccessForeground} />;
Expand Down Expand Up @@ -351,10 +353,9 @@ The framework follows a specific ordering/clustering for importing modules into
Here's an example:

```js
import React, { ReactNode } from 'react'; // 1
import styles from './Tags.module.css'; // 2
import { Icon } from '../Icon/Icon'; // 3
// 4
import React, { ReactNode } from 'react';
import styles from './Tags.module.css';
import { Icon } from '../Icon/Icon';
```

1. Import utility/library dependencies first, in alphabetical order of the library name
Expand Down Expand Up @@ -385,15 +386,15 @@ The comment should begin with an import example, include a general description o

Example:

````tsx
```tsx
/**
* `import {ButtonGroup} from "@chanzuckerberg/eds";`
*
* A container for buttons grouped together horizontally or vertically.
*
* Example usage:
*
* ```tsx
* ```
* <ButtonGroup
* className={componentClassName}
* spacing='1x'
Expand All @@ -404,14 +405,14 @@ Example:
* </ButtonGroup>
* ```
*/
export const ButtonGroup = ({
````
export const ButtonGroup = ({ ... })
```

Do not use [jsdoc tags](https://devhints.io/jsdoc) (e.g. `@example`) if possible because these will break the documentation in storybook and cause all following text to not be shown on the page. For important jsdoc tags that we really want to include, place them at the end of the comment to avoid hiding comment content. For example, we use the `@deprecated` tag so Visual Studio Code will indicate a component is deprecated for developers, but we place that at the end of a component's docstring to avoid disrupting any of the other text.

Example:

````tsx
```tsx
/**
* The Banner component is deprecated and will be removed in an upcoming release.
*
Expand All @@ -434,7 +435,7 @@ Example:
*
* @deprecated
*/
````
```

### Export module

Expand All @@ -455,13 +456,13 @@ The `src/components/{componentFolder}/index.ts` file should should import and re

i.e. in `src/components/{componentFolder}/index.ts`

```ts
```tsx
export { ComponentName as default } from './ComponentName';
```

and in `src/index.ts`

```ts
```tsx
...
export { default as ComponentName } from './components/ComponentName';
...
Expand Down Expand Up @@ -542,15 +543,15 @@ By default, we err towards more centralized control over the component architect
- Compound components never have an associated `.stories.tsx` file as they rely on the parent component's stories to render properly.
- Compound components should be exported as subcomponents from their parent component file for easier usage. For example, at the bottom of `Card.tsx`, add the lines:

```tsx
```ts
// This demonstrates how bound subcomponents are attached
Card.Header = CardHeader;
Card.Footer = CardFooter;
```

### Prop Naming conventions

- \*_camelCase for multi-word props_ - \*
- _camelCase for multi-word props_
- **All props declared at top of `render` method** - this defines all the props available to a component in one place and keeps the rest of the component code cleaner (you don't have to repeat `this.props.[thing]` everywhere).
- Don't use ternaries for most things, especially blocks of JSX.
- Update aria attribute prop names to native HTML names (`aria-label`, `aria-describedby`, `aria-labelledby`)
Expand Down Expand Up @@ -596,11 +597,11 @@ The default option should be the one most commonly used in order to reduce frict

ID attributes used for accessibility (e.g. associating `<label>` and `<input>` elements) should be unique and stable.

We currently use the [`useId` hook](https://reactjs.org/docs/hooks-reference.html#useid) for ID generation for React 18, and polyfill with an incrementing number for React <18.
We currently use the [`useId` hook](https://reactjs.org/docs/hooks-reference.html#useid) for ID generation for React 18, and polyfill with an incrementing number for React &lt; 18.
The polyfill is not ssr friendly as id generation is render timing dependent.
To ensure stable results, they cannot be invoked within conditionals or callbacks.

```tsx
```ts
const generatedId = useId();
```

Expand Down
2 changes: 2 additions & 0 deletions .storybook/components/Docs/Guidelines/Components.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Documentation/Guidelines/Components" />

# Working with EDS Components
Expand Down
2 changes: 2 additions & 0 deletions .storybook/components/Docs/Guidelines/Icons.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Documentation/Guidelines/Icons" />

# Icons
Expand Down
5 changes: 3 additions & 2 deletions .storybook/components/Docs/Guidelines/Layout.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<Meta title="Documentation/Guidelines/Layout" />

import { Canvas, Meta } from '@storybook/blocks';
import {InlineNotification} from "../../../../src";

<Meta title="Documentation/Guidelines/Layout" />

# Layout

<InlineNotification text="The layout components are being deprecated in favor of using TailwindCSS" variant="warning" />
Expand Down
2 changes: 2 additions & 0 deletions .storybook/components/Docs/Guidelines/Theming.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Documentation/Theming" />

# Theming overview
Expand Down
2 changes: 2 additions & 0 deletions .storybook/components/Docs/Guidelines/Tokens.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Documentation/Guidelines/Tokens" />

# Design Tokens
Expand Down
2 changes: 2 additions & 0 deletions .storybook/components/Docs/Guidelines/Typography.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Canvas, Meta } from '@storybook/blocks';

<Meta title="Documentation/Guidelines/Typography" />

# Typography
Expand Down
19 changes: 12 additions & 7 deletions .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ module.exports = {
'./**/*.stories.mdx',
'./**/*.stories.@(js|jsx|ts|tsx)',
],

addons: [
'@storybook/addon-essentials',
'@storybook/addon-a11y',
'@storybook/addon-links',
'storybook-css-modules-preset',
'@storybook/addon-interactions',
'@geometricpanda/storybook-addon-badges',
{
Expand All @@ -29,15 +29,20 @@ module.exports = {
},
},
{
name: '@storybook/addon-postcss',
name: '@storybook/addon-styling',
options: {
postcssLoaderOptions: {
implementation: require('postcss'),
},
postCss: true,
cssModules: true,
},
},
],
core: {
builder: 'webpack5',

framework: {
name: '@storybook/react-webpack5',
options: {},
},

docs: {
autodocs: true,
},
};
38 changes: 20 additions & 18 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"build:declarations": "tsc --emitDeclarationOnly --project tsconfig.build.json",
"build:tokens": "rm -rf src/tokens-dist/ && node ./style-dictionary.config.js && yarn prettier-tokens-dist",
"build:js": "rollup --config",
"build:storybook": "build-storybook -o storybook-static -s src/design-tokens/tier-1-definitions/fonts",
"build:storybook": "storybook build -o storybook-static -s src/design-tokens/tier-1-definitions/fonts",
"build:styles": "postcss \"src/components/**/*.css\" --dir lib/ --base src/ --verbose",
"chromatic": "chromatic",
"copy-fonts-to-lib": "copyfiles -u 3 src/design-tokens/tier-1-definitions/fonts.css src/design-tokens/tier-1-definitions/fonts/**/* lib/tokens",
Expand All @@ -59,7 +59,7 @@
"release:patch": "yarn release --release-as patch",
"release:alpha": "yarn release --prerelease alpha",
"start": "yarn build:tokens && yarn storybook",
"storybook": "start-storybook -p 9000 -s src/design-tokens/tier-1-definitions/fonts",
"storybook": "storybook dev -p 9000 -s src/design-tokens/tier-1-definitions/fonts",
"storybook:axe": "yarn run build:storybook && yarn run storybook:axeOnly",
"storybook:axeOnly": "axe-storybook --build-dir storybook-static",
"plop": "plop component",
Expand All @@ -86,6 +86,7 @@
"dependencies": {
"@headlessui/react": "^1.7.16",
"@popperjs/core": "^2.11.8",
"@storybook/addon-styling": "^1.3.6",
booc0mtaco marked this conversation as resolved.
Show resolved Hide resolved
"@tippyjs/react": "^4.2.6",
"@types/lodash": "^4.14.197",
"clsx": "^1.2.1",
Expand All @@ -97,32 +98,33 @@
"react-portal": "^4.2.2"
},
"devDependencies": {
"@chanzuckerberg/axe-storybook-testing": "^6.3.1",
"@babel/preset-env": "^7.22.10",
"@babel/preset-react": "^7.22.5",
"@babel/preset-typescript": "^7.22.5",
"@chanzuckerberg/axe-storybook-testing": "^7.1.2",
"@chanzuckerberg/eslint-config-edu-js": "^1.1.0",
"@chanzuckerberg/eslint-config-edu-ts": "^1.0.9",
"@chanzuckerberg/eslint-plugin-edu-react": "^1.1.9",
"@chanzuckerberg/eslint-plugin-stories": "^3.2.14",
"@chanzuckerberg/prettier-config-edu": "^1.0.5",
"@chanzuckerberg/story-utils": "^3.0.14",
"@chanzuckerberg/story-utils": "^4.0.0",
"@commitlint/cli": "^17.7.1",
"@commitlint/config-conventional": "^17.7.0",
"@geometricpanda/storybook-addon-badges": "^1.1.1",
"@geometricpanda/storybook-addon-badges": "^2.0.0",
"@omlet/cli": "^1.0.1-beta.32",
"@rollup/plugin-node-resolve": "^15.1.0",
"@rollup/plugin-typescript": "^11.1.2",
"@size-limit/file": "^8.2.6",
"@storybook/addon-a11y": "^6.5.16",
"@storybook/addon-docs": "^6.5.16",
"@storybook/addon-essentials": "^6.5.16",
"@storybook/addon-interactions": "^6.5.16",
"@storybook/addon-links": "^6.5.16",
"@storybook/addon-postcss": "^2.0.0",
"@storybook/builder-webpack5": "^6.5.16",
"@storybook/manager-webpack5": "^6.5.16",
"@storybook/react": "^6.5.16",
"@storybook/addon-a11y": "^7.3.1",
"@storybook/addon-docs": "^7.3.1",
"@storybook/addon-essentials": "^7.3.1",
"@storybook/addon-interactions": "^7.3.1",
"@storybook/addon-links": "^7.3.1",
"@storybook/react": "^7.3.1",
"@storybook/react-webpack5": "^7.3.1",
"@storybook/storybook-deployer": "^2.8.16",
"@storybook/testing-library": "^0.0.13",
"@storybook/testing-react": "^1.3.0",
"@storybook/testing-library": "^0.2.0",
"@storybook/testing-react": "^2.0.1",
"@testing-library/jest-dom": "^5.17.0",
"@testing-library/react": "^13.4.0",
"@testing-library/user-event": "^14.4.3",
Expand All @@ -140,7 +142,7 @@
"eslint-config-prettier": "^8.10.0",
"eslint-plugin-jest": "^27.2.3",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-storybook": "^0.6.11",
"eslint-plugin-storybook": "^0.6.13",
"eslint-plugin-testing-library": "^5.11.1",
"husky": "^8.0.3",
"identity-obj-proxy": "^3.0.0",
Expand All @@ -164,7 +166,7 @@
"size-limit": "^8.2.6",
"snake-case": "^3.0.4",
"standard-version": "^9.5.0",
"storybook-css-modules-preset": "^1.1.1",
"storybook": "^7.3.1",
"style-dictionary": "^3.8.0",
"stylelint": "^15.10.2",
"stylelint-config-recommended": "^10.0.1",
Expand Down
40 changes: 31 additions & 9 deletions src/components/Accordion/Accordion.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { generateSnapshots } from '@chanzuckerberg/story-utils';
import type { StoryFile } from '@storybook/testing-react';
import { composeStories } from '@storybook/testing-react';
import { render, screen } from '@testing-library/react';
import { act, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';
import { Accordion } from './Accordion';
Expand All @@ -9,16 +10,22 @@ import * as stories from './Accordion.stories';
const { Default } = composeStories(stories);

describe('<Accordion />', () => {
generateSnapshots(stories);
generateSnapshots(stories as StoryFile);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice some usages of generateSnapshots require as StoryFile and others don't, do you know what's happening here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not exactly sure, but it could be the type changes related to some of the storybook libraries. The first sweep was to satisfy typescript to restore parity on yarn test, and we can see if something systematic is causing certain story files to need a type


it('should open and close Accordion panel clicking Accordion button', async () => {
const user = userEvent.setup();
render(<Default />);
expect(screen.queryByTestId('accordion-panel')).not.toBeInTheDocument();
const accordionButton = screen.getByTestId('accordion-button');
await user.click(accordionButton);

await act(async () => {
await user.click(accordionButton);
});
Comment on lines +21 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these events have to be wrapped in act, how'd you come to realize this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after upgrading dependencies, yarn test would output gigantic errors in console. what's strange is that for some of these user events, if you add the act() around them, it will complain that it's redundant 👎

For whatever reason, not all user. calls need this, and on older versions of the libraries, none did. It doesnt make obvious sense which is which

expect(screen.getByTestId('accordion-panel')).toBeInTheDocument();
await user.click(accordionButton);

await act(async () => {
await user.click(accordionButton);
});
expect(screen.queryByTestId('accordion-panel')).not.toBeInTheDocument();
});

Expand All @@ -27,13 +34,25 @@ describe('<Accordion />', () => {
render(<Default />);
const accordionButton = screen.getByTestId('accordion-button');
accordionButton.focus();
await user.keyboard(' ');

await act(async () => {
await user.keyboard(' ');
});
expect(screen.getByTestId('accordion-panel')).toBeInTheDocument();
await user.keyboard(' ');

await act(async () => {
await user.keyboard(' ');
});
expect(screen.queryByTestId('accordion-panel')).not.toBeInTheDocument();
await user.keyboard('{enter}');

await act(async () => {
await user.keyboard('{enter}');
});
expect(screen.getByTestId('accordion-panel')).toBeInTheDocument();
await user.keyboard('{enter}');

await act(async () => {
await user.keyboard('{enter}');
});
expect(screen.queryByTestId('accordion-panel')).not.toBeInTheDocument();
});

Expand All @@ -51,7 +70,10 @@ describe('<Accordion />', () => {
</Accordion>,
);
const accordionButton = screen.getByTestId('accordion-button');
await user.click(accordionButton);

await act(async () => {
await user.click(accordionButton);
});
expect(onClose).toHaveBeenCalledTimes(1);
});
});
Loading
Loading