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

[WEB-3364] Storybook migration: Add Storybook #302

Merged
merged 12 commits into from
Mar 20, 2024
Merged

Conversation

dpiatek
Copy link
Contributor

@dpiatek dpiatek commented Dec 13, 2023

Update (18/3/24), jamiehenson:
This PR fundamantally does the following things:

  • Adds Storybook as a dependency and sets up preliminary configs and theming
  • Adds [COMPONENT_NAME].stories.tsx files that contain Stories for each of the components represented in the Rails ably-ui
  • To support this, TypeScript linting support has also been set up here
  • In reference to the quandaries below, I chose source components over compiled. As a caveat to that, I've removed usages of Tailwind's @layer directive as it was a blocker here (despite what various Storybook/Tailwind/PostCSS blogs and docs promise), and ultimately not something (in my view) that would necessitate the massive workarounds needed to make it play nice with SB.

Cypress is inactive with this release, the tests need to be rewritten - probably as part of #323.

To test, pull down, grab the latest packages, and run yarn storybook from root.

Original PR description:
Replace the preview server with Storybook.

Explore what kind of issues this kind of migration would encounter:

  • tailwind continues to be a bit of pain whenever any kind of build setup is involved; definitely need to simplify all of that setup both here and for consumers
  • a choice will need to be made whether to use compiled components or use the ones from source:
    • source has prop-types which allow us to autogenerate docs to some extent
    • hot reload is likely easier with source
    • CSS reload might be harder
    • compiled components will require a side process to run webpack and compile things
  • mock services will have to be written for things like sessions and blog posts
  • not completely sure we can move off cypress to storybook - would have to research that more

@ably-ci ably-ci temporarily deployed to ably-ui-add-storybook-c1af4rzn December 13, 2023 15:52 Inactive
@ably-ci ably-ci temporarily deployed to ably-ui-add-storybook-jtacsiol December 14, 2023 16:26 Inactive
@dpiatek dpiatek changed the base branch from main to remove-view-component-support December 14, 2023 16:27
@jamiehenson jamiehenson force-pushed the remove-view-component-support branch from 6c7eede to fe55d05 Compare March 11, 2024 15:45
Base automatically changed from remove-view-component-support to main March 13, 2024 11:17
@jamiehenson jamiehenson force-pushed the add-storybook branch 3 times, most recently from f14aa44 to c4163aa Compare March 15, 2024 12:53
@jamiehenson jamiehenson marked this pull request as ready for review March 15, 2024 12:54
@jamiehenson jamiehenson changed the title Add storybook [WEB-3364] Storybook migration: Add Storybook Mar 15, 2024
Copy link
Member

@kennethkalmer kennethkalmer left a comment

Choose a reason for hiding this comment

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

This looks great @jamiehenson (and @dpiatek), thanks so much!

A few small comments to give it some shine :)

  • Squash the existing featured link fixup commit down
  • Squash chore: don't allow SB version ads or telemetry into Add storybook scaffold
  • Drop the first commit, then chore: resolve preview merge commit and remove TW base layer usage will be reduced to only the meganav test data comment, and can be squashed into feat: port existing ably-ui content to SB stories, strip out TW layer…

(Be careful with the squashes, I can help if that would make it more comfortable)

src/core/FeaturedLink/component.jsx Outdated Show resolved Hide resolved
tailwind.config.js Outdated Show resolved Hide resolved
@jamiehenson
Copy link
Member

@kennethkalmer done the squashing, thanks for the suggestions.

Copy link
Member

@kennethkalmer kennethkalmer left a comment

Choose a reason for hiding this comment

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

@jamiehenson jamiehenson merged commit 0cded76 into main Mar 20, 2024
3 checks passed
@jamiehenson jamiehenson deleted the add-storybook branch March 20, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants