wellcomecollection.org icon indicating copy to clipboard operation
wellcomecollection.org copied to clipboard

Cardigan: Update to Storybook 8

Open rcantin-w opened this issue 2 years ago • 3 comments

EDIT: (23/10/23) We'll move to Storybook 7, which uses Vite by default. There are a few other tickets to consider as part of this work


Work had been attempted by @davidpmccormick in an older PR: https://github.com/wellcomecollection/wellcomecollection.org/pull/8711

We've decided to split it in multiple tickets. This is to take up the work again and ensure we're using latest versions (and hopefully faster ones 🤞 )

https://storybook.js.org/blog/storybook-for-vite/

rcantin-w avatar Jan 26 '23 10:01 rcantin-w

Storybook 7 findings so far:

  • CSF files need
    1. a default export that describes the overall component
    2. named exports for individual stories
    3. no explicit render function if component is defined in the (default exported) meta
  • .mdx files shouldn’t be .stories.mdx and index needs to be updated accordingly
  • I think we can get rid of our webpack config because it deals with react/NextJS stuff which is now handled by the @storybook/nextjs plugin. We'd also no longer need to mock anything that NextJS gives us. I think we still have to do a bit of setup for styled-components, because the blessed solution for styling in NextJS seem to be the annoyingly-similarlly-titled-but-different styled-JSX
  • Single stories can be hoisted (to appear without docs/containing folder) by having a single story whose name matches the component name
  • if Meta needs a title, it has to be a string literal (not {JSX})
  • imported .md files (e.g. component READMEs) should be included via the Markdown export of @storybook/blocks
  • add a title in form e.g. Components/BackToResults to the default (meta) output of the CSF file to create sidebar hierarchy (and disabling ‘roots’ seems nicer?)
  • use the Primary and Controls exports from @storybook/blocks to render the first export from the CSF, and any arguments, respectively. Controls can also take an of prop to point to a specific story
  • Handle Contexts with decorators
  • ~I've had no luck getting Vite off the ground so far, but will look at this next. The issue I seem to be hitting~. Actually, this
  • Do we want to keep the 'edit this on github' links (to me, feels well-intentioned, but I don't think ever used, and I think we agreed to keep 'non-dev' documentation about components in GitBook)
  • ~I upgraded to Yarn 4 because of a known bug~ <– red herring
  • Added actions plugin (unsure of how useful this is yet, but interesting to see NextLink actions including prefetch stuff on hover
  • Backgrounds maybe useful for quick contrast a11y tests?
  • A decorator wraps every story to optionally add gridSizes classes so we can see how a component will appear at different sizes without having to resize the browser. This will also be useful if/when we start using container queries
  • If we want to expose NextJS 'pages' as templates, we'd have to extract the component out to it's own pure function (because any server-side logic in an imported file will cause Storybook to crash)
  • Should we/how can we surface which underlying React components make up the things displayed in stories? I get the feeling this might be quite manual and not sure how much benefit it would bring (plus, would obviously have high potential to get out of date if it wasn't automated)
  • I've simplified our custom Readme so it's part of the default decorator and if you include a readme parameter, it will render under the story (need to check that this would also work for e.g. multiple stories with potentially different readmes)

We should discuss and decide how do we want to structure the new storybook, in terms of sections. Do we include larger page 'templates'? How do we decide on what should constitute a 'story' – so far it's been sort of a one-to-one mapping of React components to stories (but not quite, and not when the components don't feel like they serve a user purpose in themselved (atoms/tokens?) cc @j-jaworski

Update: after discussing with Jason, I think we were coming to the idea that keeping Storybook as a fairly simple list of React components is the way to go (similar to how it is currently), then any documentation that needs doing around these components should be done in GitBook. Perhaps rethink what we mean when we say 'Cardigan' to be a section of GitBook that houses documentation around embedded stories, rather than the Storybook instance (although maybe this is slightly pointlessly philosophical).

Pros

  • Being up-to-date is good
  • NextJS is now a first-class citizen (so we can remove all our custom Webpack cruft)
  • New CSF format seems a bit more logical/straightforward
  • Chance to tidy up/fix/add/remove etc. stories

Cons

  • A lot of manual conversion of stories
  • Not much of an obvious improvement from an end-user perspective

Work so far is in the sb7 branch

davidpmccormick avatar Oct 26 '23 18:10 davidpmccormick

Moving this back to Backlog, something to go over in the new year when David gets back. Whatever we do we'll probably want to team up and do a "sprint job", see Slack chat. We'll have more to discuss around what we actually want to document in Cardigan.

rcantin-w avatar Dec 01 '23 14:12 rcantin-w

... Storybook 8? https://storybook.js.org/releases/8.0 (requires node 18 and next 13.5)

rcantin-w avatar Apr 22 '24 15:04 rcantin-w

[!NOTE] Please remember to remove the relevant packages from Dependabot's ignore list once upgraded.

rcantin-w avatar Oct 07 '24 11:10 rcantin-w

Closing since Storybook has been upgraded and the remaining unchecked todos have related issues.

davidpmccormick avatar Oct 21 '24 11:10 davidpmccormick