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

Storybook: Convert stories to CSF 3 #435

Merged
merged 46 commits into from Nov 7, 2022
Merged

Storybook: Convert stories to CSF 3 #435

merged 46 commits into from Nov 7, 2022

Conversation

daneah
Copy link
Member

@daneah daneah commented Oct 31, 2022

This change: (check at least one)

  • Adds a new feature
  • Fixes a bug
  • Improves maintainability
  • Improves documentation
  • Is a release activity

Is this a breaking change? (check one)

  • Yes
  • No

Is the: (complete all)

  • Title of this pull request clear, concise, and indicative of the issue number it addresses, if any?
  • Test suite(s) passing?
  • Code coverage maximal?
  • Changeset added?
  • Component status page up to date?

What does this change address?

Resolves #244

How does this change work?

Note

Note to reviewers: The fact that Chromatic reports on visual changes and isn't reporting on much here should be a strong signal of parity; the main reported differences were intentional ones due to improvements I made along the way, or were otherwise trivial ones. You can see the full change set here.

Convert Alert stories from MDX to CSF.

Compare the new page to the current page.

Notable improvements:

  • Remove source: { type: 'code' } } from the React story configuration so that the Code option below examples shows copy/pastable code instead of some raw story code. This is how the web component stories already work.
  • We get most all of the value of how the Docs tab used to work, with less code and configuration (ostensibly).
  • The web component Storybook will now automatically show the component's JSDoc description at the top of its Docs page. I can't think of a way to make that work for the React component beyond doing dynamic code generation instead of a functional wrapper like we do now.
  • Several inconsistencies between the web component and React component stories, such as missing stories or value differences, have been freshly aligned.
  • Many components' stories have had the defaultArgs and the argTypes extracted to a storyArgs module so both the web component and the React component story can make use of them.

Some things to look out for:

  • It's more cumbersome to limit which controls show up for a given story—it used to be just those specified in argTypes, but now it's all of them unless otherwise specified in a story's include/exclude parameters. Maybe this is fine; more stories end up being something you can fiddle with and link to for collaborative purposes, which is useful.
  • It's no longer easy to sort stories in a very specific order. They're easiest to sort alphabetically (default), and anything aside from that requires coming up with some kind of scheme.
  • addon-storysource has not yet caught up to CSF 3 and doesn't display any code in the Story panel when using the Canvas. The code block on the Docs tab does still work, however.

Additional context

The two story files look more similar than ever—the only difference is really that one uses e.g. the <pharos-alert> web component while the other uses e.g. the <PharosAlert> React component.

@changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2022

🦋 Changeset detected

Latest commit: 945a8d9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@ithaka/pharos Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2022

size-limit report 📦

Path Size
packages/pharos/lib/index.js 46.46 KB (-0.04% 🔽)

@daneah daneah changed the title feat(storybook): try CSF for Alert stories Storybook: Try CSF for Alert stories Oct 31, 2022
* develop:
  ToggleButton: indicate toggle button state on underlying button elements (#438)
  Tooltip, DropdownMenu: replace popperjs with floating-ui (#434)
@daneah daneah changed the title Storybook: Try CSF for Alert stories Storybook: Convert stories to CSF 3 Nov 3, 2022
@daneah daneah mentioned this pull request Nov 4, 2022
@daneah daneah marked this pull request as draft November 4, 2022 12:41
@daneah daneah marked this pull request as ready for review November 6, 2022 21:04
Comment on lines +1 to +25
import {
ArgsTable,
Description,
Primary,
PRIMARY_STORY,
Stories,
Subtitle,
Title,
} from '@storybook/addon-docs';

import { GuidelineLink } from '@config/GuidelineLink';

export const configureDocsPage = (componentName) => {
return () => (
<>
<Title />
<Subtitle />
<Description />
{componentName && <GuidelineLink path={componentName} />}
<Primary />
<ArgsTable story={PRIMARY_STORY} />
<Stories />
</>
);
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a noteworthy file; I extracted the common ordering of items for the Docs tab of each component into a reusable piece. This was previously specified implicitly in each story using MDX, but now gets added in the parameters.docs.page configuration of the default export for each component.

Comment on lines +2 to +4
features: {
previewCsfV3: true,
},
Copy link
Member Author

Choose a reason for hiding this comment

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

This allows us to use Component Story Format (CSF) version 3 in the stories, and will become the default in Storybook v7.

This necessitated getting rid of dynamic component titles, namely for
showing e.g. "Pharos 12.13.0" in the left nav. I opted instead to inject
the version as the title of the intro page. I had to wrestle with MDX a
bit to do this; it appears the "components" and "props" features
documented on their site are v2 features, but our Storybook is still
using v1. I attemped to enable the v2 preview, but HTML comments cause a
parsing error currently and we need them for our README setup to
function properly.
@daneah daneah merged commit 6cbeb91 into develop Nov 7, 2022
@daneah daneah deleted the feat/try-csf-for-alert branch November 7, 2022 15:56
daneah added a commit that referenced this pull request Nov 10, 2022
* develop:
  Tabs: fix initial tab selection in more cases (#449)
  Storybook: add eslint-plugin-storybook and fix issues (#446)
  feat(storybook): install Measure and Outline addons (#445)
  Docs: update copy for Button's on-background usage (#444)
  Storybook: Convert stories to CSF 3 (#435)
  feat(tabs): fire event when tab selected programmatically (#442)
  ToggleButton: indicate toggle button state on underlying button elements (#438)
  Tooltip, DropdownMenu: replace popperjs with floating-ui (#434)
  Site: improve imports (#433)
@github-actions github-actions bot mentioned this pull request Nov 10, 2022
sirrah-tam pushed a commit to sirrah-tam/pharos that referenced this pull request Dec 1, 2023
* feat(storybook): try CSF for Alert stories

* chore(storybook): convert alert WC story to CSF 3

* chore(storybook): convert alert React story to CSF 3

* chore(storybook): extract DocsPage config and convert breadcrumb to CSF3

* chore(storybook): convert Button stories to CSF 3

* chore(storybook): convert Checkbox stories to CSF 3

* chore(storybook): convert CheckboxGroup stories to CSF 3

* chore(storybook): convert Combobox stories to CSF 3

* chore(storybook): convert DropdownMenu stories to CSF 3

* chore(storybook): convert DropdownMenuNav stories to CSF 3

* chore(storybook): convert Footer stories to CSF 3

* chore(storybook): convert Header stories to CSF 3

* chore(storybook): convert Heading stories to CSF 3

* chore(storybook): convert Icon stories to CSF 3

* chore(storybook): convert ImageCard stories to CSF 3

* chore(storybook): convert InputGroup stories to CSF 3

* chore(storybook): convert Layout stories to CSF 3

* chore(storybook): convert Link stories to CSF 3

* chore(storybook): convert LoadingSpinner stories to CSF 3

* chore(storybook): convert Modal stories to CSF 3

* chore(storybook): convert Pagination stories to CSF 3

* chore(storybook): convert ProgressBar stories to CSF 3

* chore(storybook): convert RadioButton stories to CSF 3

* chore(storybook): convert RadioButtonGroup stories to CSF 3

* chore(storybook): convert Select stories to CSF 3

* chore(storybook): convert Sidenav stories to CSF 3

* chore(storybook): convert Tabs stories to CSF 3

* chore(storybook): convert TextInput stories to CSF 3

* chore(storybook): convert Textarea stories to CSF 3

* chore(storybook): convert Toast stories to CSF 3

* chore(storybook): convert ToggleButtonGroup stories to CSF 3

* chore(storybook): convert Tooltip stories to CSF 3

* chore(storybook): convert Page stories to CSF 3

* chore(storybook): convert Styles stories to CSF 3

* fix(storybook): make diverged stories consistent

* chore(storybook): clean up imports

* fix: add type declarations and remove console log

* chore: add changeset

* fix(storybook): fix React Storybook build

* feat(storybook): enable new build options

This necessitated getting rid of dynamic component titles, namely for
showing e.g. "Pharos 12.13.0" in the left nav. I opted instead to inject
the version as the title of the intro page. I had to wrestle with MDX a
bit to do this; it appears the "components" and "props" features
documented on their site are v2 features, but our Storybook is still
using v1. I attemped to enable the v2 preview, but HTML comments cause a
parsing error currently and we need them for our README setup to
function properly.

* fix(storybook): update webpack config to fix GitHub Actions

* fix(storybook): remove new Babel config to fix build

* fix(storybook): remove inline render to try fixing build

* fix(storybook): remove story store v7 to fix build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert stories to Component Story Format
4 participants