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

Add Docgen info to PropTable #1505

Closed
wants to merge 0 commits into from
Closed

Conversation

jribeiro
Copy link
Contributor

Issue:
Storybook Info Addon currently does not support React docgen for documenting components. This was actually implemented on the old repo but got lost on the migration:
storybook-eol/react-storybook-addon-info@092e10a

What I did

Added support for react docgen on the PropTable component and documentation

How to test

Follow the instructions on the readme on a repo supporting flowtype. normal npm link approaches should be helpful

@shilman
Copy link
Member

shilman commented Jul 22, 2017

@jribeiro Thanks for contributing this! Is there any chance you can add this to the examples/cra-kitchen-sink example as part of this PR? We're starting to make sure that all PRs have good examples there.

To get the example running on your machine:

yarn && yarn bootstrap
cd examples/cra-kitchen-sink
yarn storybook

Many thanks again and please let me know if you have any questions!

@codecov
Copy link

codecov bot commented Jul 24, 2017

Codecov Report

Merging #1505 into master will decrease coverage by 0.13%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1505      +/-   ##
==========================================
- Coverage   14.61%   14.47%   -0.14%     
==========================================
  Files         202      202              
  Lines        4653     4670      +17     
  Branches      505      520      +15     
==========================================
- Hits          680      676       -4     
- Misses       3533     3542       +9     
- Partials      440      452      +12
Impacted Files Coverage Δ
addons/info/src/components/PropTable.js 0% <0%> (ø) ⬆️
app/react/src/client/preview/client_api.js 39.28% <0%> (-2.76%) ⬇️
app/react/src/demo/Welcome.js 0% <0%> (ø) ⬆️
addons/storyshots/src/index.js 0% <0%> (ø) ⬆️
.../ui/src/modules/ui/components/left_panel/header.js 28.57% <0%> (ø) ⬆️
addons/info/src/components/Story.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/init_panels.js 25% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_keyevents.js 33.33% <0%> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 33.33% <0%> (ø) ⬆️
addons/info/src/components/markdown/code.js 0% <0%> (ø) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 548a5bf...5a2d0b6. Read the comment docs.

@jribeiro
Copy link
Contributor Author

Hi @shilman, thanks for that. I've added an example to the examples/cra-kitchen-sink. Let me know if you need any other change.

Copy link
Member

@danielduan danielduan left a comment

Choose a reason for hiding this comment

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

Thanks for bringing back the commit that got lost during the merge.

See if you can get rid of these dependencies and give it a test. Let me know if you need any help!

// @flow
import React from 'react';

type PropsType = {
Copy link
Member

Choose a reason for hiding this comment

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

does this mean flowtype is supported by our babel-plugin-react-docgen?

storybookjs/babel-plugin-react-docgen#29

"polyfill": false,
"regenerator": true
}],
"react-docgen"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's a good idea to add this .babelrc since these settings should have been the default included by Storybook.

If there's anything that should be inserted, we should include it here:
https://github.com/storybooks/storybook/blob/master/app/react/src/server/babel_config.js#L73

Looks like the babel plugin is already getting inserted.

"@storybook/addon-links": "^3.0.0",
"@storybook/addon-notes": "^3.0.0",
"@storybook/addon-options": "^3.0.0",
"@storybook/addon-storyshots": "^3.0.0",
"@storybook/addons": "^3.0.0",
"@storybook/react": "^3.0.0",
"babel-plugin-react-docgen": "^1.5.0",
Copy link
Member

Choose a reason for hiding this comment

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

This is included by the package.json in the react app already and it should be used by the babel there already.

https://github.com/storybooks/storybook/blob/master/app/react/package.json#L34

If it's not, let's fix it.

@jribeiro
Copy link
Contributor Author

Hi @danielduan, is this not needed anymore?

@danielduan
Copy link
Member

danielduan commented Jul 31, 2017

@jribeiro Sorry about that, I tried to fix the merge conflicts and when I pushed to your branch, this got closed automatically. New PR is above ^

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.

3 participants