Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

WIP: feat(storybook): add addon-docs #241

Closed
wants to merge 13 commits into from
Closed

WIP: feat(storybook): add addon-docs #241

wants to merge 13 commits into from

Conversation

ocBruno
Copy link
Contributor

@ocBruno ocBruno commented Jan 26, 2020

fix #238

Changes proposed in this pull request:
-Remove superceded addon-info by its new replacement addon-docs
-Refactor navbar storiesOfApi to recommendad CSF format

Newly added dependencies with Bundlephobia links:

  • @storybook/addon-docs

add @storybook/addon-docs package and react-is dependency and configure addon-docs loader in webpack

BREAKING CHANGE: @storybook/addon-docs and react-is development dependencies

fix #238
@jackcmeyer jackcmeyer added the enhancement New feature or request label Jan 26, 2020
@jackcmeyer jackcmeyer added this to the v1.0.0 milestone Jan 26, 2020
@jackcmeyer
Copy link
Member

Seems reasonable to me, however I don't have too much storybook knowledge. What do you think @fox1t and @tehkapa?

@fox1t
Copy link
Member

fox1t commented Jan 26, 2020

Here we need to summon @irvelervel too!

@matteovivona
Copy link
Contributor

@ocBruno to understand this PR, with this new add-on this part here

storybook-old

will it have to be placed here?

storbook-addons-docs

and then we'll have something like this?

storbook-new-docs

Copy link
Contributor

@matteovivona matteovivona left a comment

Choose a reason for hiding this comment

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

conflicts must be solved

@ocBruno ocBruno changed the title feat(storybook): add addon-docs WIP: feat(storybook): add addon-docs Jan 26, 2020
@ocBruno
Copy link
Contributor Author

ocBruno commented Jan 26, 2020

conflicts must be solved

@ocBruno to understand this PR, with this new add-on this part here

storybook-old

will it have to be placed here?

storbook-addons-docs

and then we'll have something like this?

storbook-new-docs

This one I wll also leave WIP as I am very new to webpack loaders and storybook so if anyone wants to pitch in some advice I will take a lot at the addon docs documentation to try to apply the code preview to the docs page as well as well as resolve the conflicts.

Thanks for the feedback @jackcmeyer @fox1t @tehkapa @irvelervel !

ocBruno and others added 7 commits January 26, 2020 13:49
change awesome-typescript-loader for more trending ts-loader, upgrade all @storybook packages to
match addon-docs version, remove unnecssary react is, modify webpack and storybook config for
package upgrades

BREAKING CHANGE: replace awesome-typescript-loader for ts-loader, upgrade @storybook/*

fix #238
BREAKING CHANGE: dependency upgrades and modifications

fix #238
@irvelervel
Copy link

Hi @ocBruno! First of all thank you for your contribution, as always.
Nice work, can you answer some questions for me?

  • Which advantages does ts-loader provide over awesome-typescript-loader?
  • I believed our versions of storybook and addons were already up-to-date because of the tilde in the dependency requirements (e.g. @storybook/addon-info": "~5.3.0"); I suggest you to take the same approach with your new inclusions, because many times the compilation did broke because of a silent minor upgrade
  • the theming option is very nice, does it affect every story or just the selected ones?
  • I cannot really see the benefit of addon-docs, because right now it is just adding a new tab for every story showing the same exact information actually present in the Canvas tab. Am I missing something? How do you think we can customize this behavior, adding value to existing content?

Have a nice day! 🙂

@ocBruno
Copy link
Contributor Author

ocBruno commented Jan 31, 2020

Hi @ocBruno! First of all thank you for your contribution, as always.
Nice work, can you answer some questions for me?

  • Which advantages does ts-loader provide over awesome-typescript-loader?
  • I believed our versions of storybook and addons were already up-to-date because of the tilde in the dependency requirements (e.g. @storybook/addon-info": "~5.3.0"); I suggest you to take the same approach with your new inclusions, because many times the compilation did broke because of a silent minor upgrade
  • the theming option is very nice, does it affect every story or just the selected ones?
  • I cannot really see the benefit of addon-docs, because right now it is just adding a new tab for every story showing the same exact information actually present in the Canvas tab. Am I missing something? How do you think we can customize this behavior, adding value to existing content?

Have a nice day! 🙂

Hello @irvelervel thanks for the feedback!

-The typescript loader was a suggestion I had but I should've opened a separate issue for it. I had suggested it because it could prove to be more reliable considering some have commented they had compiling speed improvements from it and it is trending more.
-https://www.npmtrends.com/awesome-typescript-loader-vs-ts-loader
-s-panferov/awesome-typescript-loader#497
I do think we should open a separate issue for someone with more knowledge with loaders can give their opinion considering it's not relevent to this issue anyways.

The versioning was a beginner mistake but the addition of addon-docs was suggested by one of the storybook contributors in the following issue, saying that it fixed a bug with displaying code previews with too many nested objects in the component paramaters .
Apparently it is being superceded by addon-docs but I attempted removing addon-info and was having some issues so I wasn't sure if I was to remove or not addon-info.
-https://github.com/storybookjs/storybook/issues/9618

Take a look at this for more info on addon-docs and what's coming to storybook
-https://medium.com/storybookjs/storybook-docs-sneak-peak-5be78445094a
-https://www.npmjs.com/package/@storybook/addon-docs#typescript-configuration

The themes are applied to the storybook global UI and is documented here
-https://storybook.js.org/docs/configurations/theming/#global-theming
I think it is good for improving customization of the storybook interface and improve the design appeal and user experience.

I will be reverting the typescript loader change and will dive deeper into storybook to be able to better contribute to the stories and configuration before removing the WIP from this PR. My main focus was fixing the preview styling bug but after seeing how addon-docs is part of storybooks near future I think I will also work on the other necessary changes to adapt the project for the initial upgrade to addon-docs if agreed upon.

Thanks again and have a nice day too! 🚀

@irvelervel
Copy link

Ok, great @ocBruno! Are we facing this preview styling bug in some components at the moment? I'm not sure I'm fully understanding the problem you're talking about. Have a great day.

@ocBruno
Copy link
Contributor Author

ocBruno commented Jan 31, 2020

Ok, great @ocBruno! Are we facing this preview styling bug in some components at the moment? I'm not sure I'm fully understanding the problem you're talking about. Have a great day.

Not sure if we still are but I am about to merge my branch with upstream to make sure they weren't resolved but I was having problems reproducible in the following issue.
#238
You too!

Update:
-Resolved conflicts, merged with most recent changes, currently working on updating the story formats to CSF considering the storiesApi seems to be the "old-style" and there is even a new format coming called MDX. There are codemods to convert storiesApi to CSF and to MDX so I think I will give those a shot and see if I can get everything working.

@ocBruno
Copy link
Contributor Author

ocBruno commented Jan 31, 2020

Hey @irvelervel @tehkapa @jackcmeyer @fox1t,
I decided to mention everyone considering this is a significant upgrade suggestion!

According to
-https://storybook.js.org/docs/formats/component-story-format/
Storybook’s Component Story Format (CSF) is the recommended way to write stories since Storybook 5.2 so I decided to configure an example branch with some boilerplate code but am still running into the code styling issue even though this upgrade was supposed to fix this issue.

I will do some more debugging but I will leave the working config here for now in case anyone is interested in checking it out. Some of the reasons for the upgrade, differences of the storiesOf API and benefits of the new markup can also be found here.
-https://medium.com/storybookjs/component-story-format-66f4c32366df

I only left the navbar incase this request doesn't go through but am willing to refactor the rest of the stories!
Have a good night everyone and take care!

@matteovivona
Copy link
Contributor

Closed for inactivity and to save build time

@ocBruno ocBruno deleted the fix/replace-addon-info-addon-docs branch December 22, 2020 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storybook: Outdated addon-info leading to wrong code styling
5 participants