ithaka / pharos

JSTOR's design system
https://pharos.jstor.org
MIT License
113 stars 15 forks source link

Storybook: Docs not being themed properly in Storybook 7 + Vite builder #544

Open daneah opened 1 year ago

daneah commented 1 year ago

Expected behavior

Docs with the @storybook/addon-styling theme applied should have the color choices applied to links and so on.

Actual behavior

Docs have the default Storybook styles.

Steps to reproduce the issue

  1. Visit the Pharos Storybook
  2. Note the Introduction page has a lot of blue links (pending fixing the composed Storybook after #543...)

Screenshots or code

Before After
Screenshot 2023-06-13 at 18 53 53 Screenshot 2023-06-13 at 19 48 26

Additional information

Possibly related to this Storybook issue, which claims to have been addressed.

satya-achanta-venkata commented 10 months ago

@daneah I looked into this and followed the related storybook issue as well. Most of the comments from the storybook issue suggests to set sideEffects to true or completely remove sideEffects from packages/pharos/package.json, but nothing worked.

In addition to that, I also followed this issue from storybook but couldn't understand quite exactly how esbuild and sideEffects are related.

Any suggestions/thoughts for me here ?

daneah commented 10 months ago

@satya-achanta-venkata unfortunately you're as deep in it as I've been, at this point. I'd welcome any of the @ithaka/pharos-maintainers to look with you as well!

satya-achanta-venkata commented 10 months ago

After looking into this, I learned that @storybook/styling is deprecated and it is recommended to use @storybook/addon-themes. I followed the migration guid from the docs below and still not seeing our theme is applied as expected. I feel I am missing something very small.

Here are the links relates to @storybook/styling and migration guide to @storybook/addon-themes

https://storybook.js.org/addons/@storybook/addon-styling https://github.com/storybookjs/addon-styling/blob/main/MIGRATION.md

Also, please correct me on this if I am wrong. In the migration guide, I dont think webpack section of the guide is applicable to use because we use vite, so I skipped webpack part.

daneah commented 10 months ago

@satya-achanta-venkata you're probably on the right track, there. It's all a bit confusing because even the @storybook/addon-themes documentation contains references to @storybook/addon-styling imports, despite the migration guide saying to remove the @storybook/addon-styling package as a dependency. This feels like a juncture where Storybook has too many migratory things in flight and we might want to wait a bit.

I did try the migration briefly and it will at a minimum require the additional work of converting our theme in .storybook/theme.js to some other format, possibly putting that stuff in what Storybook seems to call "global styles." If you want to look there at some point, that might bear fruit.

daneah commented 3 months ago

This appears to continue being an issue in Storybook v8, as was probably expected.