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 Storybook to the user guide #738

Merged
merged 6 commits into from
Nov 20, 2016
Merged

Add Storybook to the user guide #738

merged 6 commits into from
Nov 20, 2016

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Sep 24, 2016

Add instructions on how to add Storybook to the User Guide.
I also mentioned a bit of information about benefits of using Storybook.

Related to: #297 (comment)

@ghost ghost added the CLA Signed label Sep 24, 2016
@usirin
Copy link

usirin commented Sep 27, 2016

Congrats @arunoda! It's awesome to see react-storybook getting into a mainstream tool like create-react-app. Can't wait.

@ghost ghost added the CLA Signed label Sep 27, 2016
Copy link

@sylvainbannier sylvainbannier left a comment

Choose a reason for hiding this comment

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

seems there's a missing link in the doc

* Screencast: [Getting Started with React Storybook](https://egghead.io/lessons/react-getting-started-with-react-storybook)
* [GitHub Repo](https://github.com/kadirahq/react-storybook)
* [Documentation](https://getstorybook.io/docs)
* Snapshot Testing with React Storybook

Choose a reason for hiding this comment

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

missing link ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah.

@@ -41,6 +41,7 @@ You can find the most recent version of this guide [here](https://github.com/fac
- [Continuous Integration](#continuous-integration)
- [Disabling jsdom](#disabling-jsdom)
- [Experimental Snapshot Testing](#experimental-snapshot-testing)
- [Turbo Charge Component Development with React Storybook](#turbo-charge-component-development-with-react-storybook)
Copy link
Contributor

@gaearon gaearon Sep 28, 2016

Choose a reason for hiding this comment

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

This looks too much like an ad :-) Can we make it appear a bit more utilitarian?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:D sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this?

Developing UI Components with React Storybook.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

* With some emoji
* With disabled mode

With React Storybook, you can manage these different states (or stories) and develop them without running your app. You can also deploy your Storybook as a static app.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here it would be great to mention that this is a third party tool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep.

Copy link
Contributor Author

@arunoda arunoda Sep 30, 2016

Choose a reason for hiding this comment

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

How about this?

NOTE: React Storybook is an open source third party tool. It's maintained by the community, but not the core React team.

Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than making a separate note, would be nice to integrate organically into the paragraph as a first sentence. It doesn't need to be super thorough. Just saying "React Storybook is a project that <...>" makes it clear enough.

Then, apply following command inside your app:

```sh
getstorybook
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this guaranteed to work? Does Storybook support our Babel preset, NODE_PATH resolution, react-native-web alias, and other stuff in our Webpack configs? Is it being tested on npm 2?

Copy link
Contributor Author

@arunoda arunoda Sep 30, 2016

Choose a reason for hiding this comment

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

We need to get the NODE_PATH support and RN alias. It's pretty similar Webpack config and yep tested on NPM2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to finish these by next Wednesday: storybookjs/storybook#468

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I finished all of them and we are ready to go.
See: storybookjs/storybook#468

Old title was looks like a marketing pitch. Change it to something looks great.
The new one is: Developing UI Components with React Storybook.
@arunoda
Copy link
Contributor Author

arunoda commented Oct 5, 2016

@gaearon I've apply changes as suggested and now getstorybook and @kadira/storybook par with CRA features.

@gaearon
Copy link
Contributor

gaearon commented Oct 5, 2016

I’m almost happy to merge it but I’m not quite satisfied with the first run experience.

The URL should be highlighted: storybookjs/storybook#531.
The errors should be more obvious: storybookjs/storybook#532.

Another thing I found confusing is it’s not clear what I should do next. The guide explains:

Just like that, you can add your own components as stories.
You can also edit those components and see changes right away.
(Try editing the Button component located at src/stories/Button.js.)

This is okay but I can see it causing confusion:

  • The user might think they need to create components inside src/stories rather than inside their app.
  • The user might not realize they can import existing components into Storybook.

I would rather want to see a step-by-step guide to adding an existing component to storybook:

  1. Import your component from src/stories/index.js
  2. Add a storiesOf('MyComponent', module) call
  3. Add add() calls for different states of your component
import React from 'react';
import { storiesOf } from '@kadira/storybook';
import App from '../App';

storiesOf('App', module)
  .add('normal', () => (
    <App />
  ));

And then mention that we recommend using it on smaller components than App.

@arunoda
Copy link
Contributor Author

arunoda commented Oct 5, 2016

@gaearon I think I can work on those issues on Storybook. (You've submitted)

We use Button as an example because it's something we can use without knowing user's components.

I can see the confusion it might cause.

We can change the example on Writing Stories page to use some real component in his app. Assuming Header as a hypothetical components inside his app.

I'm quite not sure, we can should mention using App as a story. Initially just after setup this is fine. But, in a real project we have more complex setup inside the App, may be with some Redux stuff. May be App is no longer available.

That's why I like to use something like Header instead of App for your suggested getting started workflow.

@yanivtal
Copy link

yanivtal commented Oct 5, 2016

I understand Kadira is monetizing their hosted Storybook service. Will Create React App also provide free marketing for other tools?

@arunoda
Copy link
Contributor Author

arunoda commented Oct 5, 2016

@yanivtal I want to clear about what we are doing here.

We don't mention anything about our hosted services here.
Storybook is not a marketing tool for any product. It's a totally 100% open source product where developers love to use.
We've no intention to make any of that closed source.

Yes, we are trying to build some hosted services and make development of Storybook self sustaining without relying on donations. I think there's nothing wrong with that.

@yanivtal
Copy link

yanivtal commented Oct 5, 2016

@gaearon @arumoda I don't think there's anything wrong with what you're doing. More people should be able to make a living off their tools. But Storybook isn't a library. It's a freemium service. Get started for free or use our hosted service for a better experience. There's nothing wrong with that, but I don't think Create React App should favor some paid services over others.

@stevewillard
Copy link

@yanivtal But, it is a library. It's not a freemium service.
https://github.com/kadirahq/react-storybook

npm i -g getstorybook

@yanivtal
Copy link

yanivtal commented Oct 5, 2016

K, I'll withdraw my concerns. Keep up the great work everyone!

@arunoda
Copy link
Contributor Author

arunoda commented Oct 7, 2016

@gaearon I've fixed issues you've mentioned on the React Storybook repo.
Also, now the welcome screen looks like this. (I've published a new version of the getstorybook tool)

screen shot 2016-10-07 at 1 01 59 pm

I assume this is much better than the previous one.

@sylvainbannier
Copy link

also it would be nice to have default stories for actual CRA components instead of new ones.

storiesOf('App', module)
  .add('normal', () => (
    <App />
  ));

instead of :

storiesOf('Welcome', module)
  .add('to Storybook', () => (
    <Welcome showApp={linkTo('Button')}/>
  ));

storiesOf('Button', module)
  .add('with text', () => (
    <Button onClick={action('clicked')}>Hello Button</Button>
  ))
  .add('with some emoji', () => (
    <Button onClick={action('clicked')}>😀 😎 👍 💯</Button>
  ));

@arunoda
Copy link
Contributor Author

arunoda commented Oct 7, 2016

@sylvainbannier We tried that.
Some users add storybook to their CRA powered app after they've refactored it into a real app structure.
So, we can't 100% sure App component is exists.
Even if that exists, if might container stuff like Redux, Relay or something that doesn't work on Storybook.

That's why we go with some example Component.

@just-boris
Copy link
Contributor

I have tried to follow this guide and set up storybook into my project. Actually, I didn't find any create-react-app specific "integration".

getstorybook command creates two new folders: .storybook with configuration and stories with some stories configurations. These folders don't know anything about create-react-app structure, I can add storybook into any other project same way, but I can't say that this will be an "integration".

To name this thing integration, I expected at least that getstorybook will try to find my components in src folder and create stories for them, not just put some extra components that have nothing in common with my code.

Also, I don't understand the purpose of separate stories folder. At the time, when we promote benefits of the colocating source and test code, the separate stories folder looks like a step back.

@arunoda
Copy link
Contributor Author

arunoda commented Oct 9, 2016

@just-boris yep it's not like a direct integration to CRA.

Storybook has a it's own webpack setup and allow developers to customize it as they want.
With the emerge of CRA, we've added a default webpack config which is almost similar to CRA. That's the kind of integration we've right now.

CRA doesn't offer ways to extend it for good reasons. So, this is our kind of integration which works pretty fine and not hard to manage. It creates few files.

You've a pretty valid point about the stories directory. I think we should move it to the src directory by default. Since CRA doesn't impose a way to manage project architecture, it's up to user to load stories from where he/she needs. We offer ways to do that.

This is a completely optional thing. That's why I hope we don't do any direct integration to CRA codebase or send it by default with CRA. This is just a section on the docs.
Adding this won't do any harm, but this may help developers to build their components in a pretty nice way.

@just-boris
Copy link
Contributor

Hey, it seems like there is no any progress.
As well as in my alternative PR #921.

Does is mean that not very many people interested in this feature, so we can close this?

@arstin
Copy link

arstin commented Nov 15, 2016

FWIW, I hope Storybook gets the small promotion of being in the CRA docs as an optional tool. Styleguidist is nice and it may be worthwhile to also add it to CRA in the "optional ecosystem", but it's not really comparable to Storybook. Storybook is the leading and best supported option among a number of solutions tackling a true component-based workflow. The point isn't just to collaborate with designers or to have a reference page for widgets. It's to support a whole new frontend development workflow which helps to scale development teams, improve UI testing, make edge cases explicit, speed up new developer onboarding, support user research, provide a standard on which new tools for designers can be built, share modules across projects, develop new features in isolation from the app itself, and on and on.

Is Storybook there yet? No. Are details like the stories directory exactly right? Probably not. But they keep pushing forward and, given it's true potential will only be realized as it scales in users and contributors, could use the very modest help of being featured in CRA docs.

FTR, I have no personal investment in either option. I don't (yet?) use Storybook, and only tried Styleguidist for a few weeks before deciding it didn't fit my personal needs. I'm just speaking as someone whose imagination was captured by trying devcards in Clojurescript a couple years ago and is excited the future is slowly starting to arrive.

@gaearon
Copy link
Contributor

gaearon commented Nov 20, 2016

Does is mean that not very many people interested in this feature, so we can close this?

It means I haven't had time to review it 😉 .

@gaearon
Copy link
Contributor

gaearon commented Nov 20, 2016

LGTM.

@gaearon gaearon merged commit 8cf45e1 into facebook:master Nov 20, 2016
jarlef pushed a commit to jarlef/create-react-app that referenced this pull request Nov 28, 2016
* Add Storybook to the user guide

* Add the missing "Snapshot Testing" link.

* Change the title to something nicer

Old title was looks like a marketing pitch. Change it to something looks great.
The new one is: Developing UI Components with React Storybook.

* Mention React Storybook as a third party tool.

* Nits

* Minor changes
alexdriaguine pushed a commit to alexdriaguine/create-react-app that referenced this pull request Jan 23, 2017
* Add Storybook to the user guide

* Add the missing "Snapshot Testing" link.

* Change the title to something nicer

Old title was looks like a marketing pitch. Change it to something looks great.
The new one is: Developing UI Components with React Storybook.

* Mention React Storybook as a third party tool.

* Nits

* Minor changes
randycoulman pushed a commit to CodingZeal/create-react-app that referenced this pull request May 8, 2017
* Add Storybook to the user guide

* Add the missing "Snapshot Testing" link.

* Change the title to something nicer

Old title was looks like a marketing pitch. Change it to something looks great.
The new one is: Developing UI Components with React Storybook.

* Mention React Storybook as a third party tool.

* Nits

* Minor changes
@robfe
Copy link

robfe commented Mar 7, 2018

Should this now suggest you use npm i -g @storybook/cli instead?

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants