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 React Storybook to the project #93

Merged
merged 3 commits into from Mar 20, 2017
Merged

Add React Storybook to the project #93

merged 3 commits into from Mar 20, 2017

Conversation

kadamwhite
Copy link
Collaborator

This implements Storybook, a UI component development sandbox environment, and begins the process of moving our existing UI components into stories so that we can iterate on components in isolation from the rest of the application.

To test,

  • Run npm install to get the storybook dev dependencies
  • Run npm run storybook to spin up the Storybook server
  • Open the storybook at localhost:6006

storybook-demo

Eventually we should have stories for most or all components, both the "pure" UI ones (like the ProgressBar being used as a test case here) and the more complex components that represent entire pages of the app.

Stories can also be used to prototype behavior before we implement the corresponding Redux reducers and selectors, which should come in handy in the future.

Resolved roadblocks I encountered while integrating this system:

  • Storybook uses Webpack 1. Solution: dynamically map our registered Webpack 2 loaders into a Webpack 1-compatible format, to avoid the need to maintaining parallel webpack config files.
  • Storybook's default install puts all stories in one directory, so stories are separated from the components they test. This is unwieldy in the extreme, so at @gnarf's suggestion I adapted a method from the Storybook documentation to dynamically find and execute stories in *.stories.jsx files, so that each component can have a sidecar file defining the stories we use to develop & test its UI configuration.

Outstanding issues:

  • I cannot convince ESLint to ignore *.stories.jsx files. Help figuring that one out would be appreciated.

Closes #89

This implements [Storybook](https://getstorybook.io), a UI component
development sandbox environment, and begins the process of moving our
existing UI components into stories so that we can iterate on components
in isolation from the rest of the application.

To test,

- Run `npm install` to get the storybook dev dependencies
- Run `npm run storybook` to spin up the Storybook server
- Open the storybook at [localhost:6006](http://localhost:6006)

![storybook-demo](https://cloud.githubusercontent.com/assets/442115/24120117/d5a5c9cc-0d89-11e7-90bc-56acf4767b74.gif)

Eventually we should have stories for most or all components, both the
"pure" UI ones (like the `ProgressBar` being used as a test case here)
and the more complex components that represent entire pages of the app.
Stories can also be used to prototype behavior before we implement the
corresponding Redux reducers and selectors, which should come in handy
in the future.

Resolved roadblocks I encountered while integrating this system:

- **Storybook uses Webpack 1**. Solution: dynamically map our registered
  Webpack 2 loaders into a Webpack 1-compatible format, to avoid the
  need to maintaining parallel webpack config files.
- **Storybook's default install puts all stories in one directory, so
  stories are separated from the components they test**. This is unwieldy
  in the extreme, so at @gnarf's suggestion I adapted a method from the
  Storybook documentation to dynamically find and execute stories in
  `*.stories.jsx` files, so that each component can have a sidecar file
  defining the stories we use to develop & test its UI configuration.

Outstanding issues:

- I cannot convince ESLint to ignore `*.stories.jsx` files. Help figuring
  that one out would be appreciated.

Closes #89
Copy link
Collaborator

@gnarf gnarf left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me!

'*.js',
'**/__tests__/*',
'jest/**/*',
'**/*.stories.jsx',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And yet,

image-annotator/frontend/components/ProgressBar/ProgressBar.stories.jsx
  2:1  error  '@kadira/storybook' should be listed in the project's dependencies, not devDependencies  import/no-extraneous-dependencies

Totally stumped here, passing this to glob manually finds the right file, I don't know why ESLint isn't whitelisting this pattern.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gnarf identified that we had a duplicative .eslintrc.js file inside frontend that was defining an override for this rule specifically, preventing the parent config from taking effect inside frontend/. Since the intent of the nested file was already accomplished by the existing config in the parent, I've removed the nested .eslintrc.js.

@kadamwhite
Copy link
Collaborator Author

The errors in travis confuse me, they look like this:

/home/travis/build/bocoup/image-annotator/frontend/components/Home.jsx
  10:2  error  Expected indentation of 6 space characters but found 1  react/jsx-indent
  16:1  error  Expected indentation of 6 space characters but found 0  react/jsx-indent
  17:2  error  Expected indentation of 6 space characters but found 1  react/jsx-indent
  18:2  error  Expected indentation of 6 space characters but found 1  react/jsx-indent
/home/travis/build/bocoup/image-annotator/frontend/components/PerceivedDemographics.jsx
  146:1  error  Expected indentation of 20 space characters but found 0  react/jsx-indent
  158:1  error  Expected indentation of 14 space characters but found 0  react/jsx-indent
  166:1  error  Expected indentation of 12 space characters but found 0  react/jsx-indent
/home/travis/build/bocoup/image-annotator/frontend/components/ProgressBar/index.jsx
  17:2  error  Expected indentation of 8 space characters but found 1  react/jsx-indent

but e.g. Home.jsx seems to have fine indentation. Not sure how to proceed

@@ -87,7 +91,7 @@
"eslint-loader": "^1.6.3",
"eslint-plugin-import": "^2.2.0",
"eslint-plugin-jsx-a11y": "^3.0.2",
"eslint-plugin-react": "^6.9.0",
"eslint-plugin-react": "6.9.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was the source of the mystery build errors: eslint-plugin-react v6.10 introduced a breaking change, and there's some discussion about it in the repo (see jsx-eslint/eslint-plugin-react#1117). Pinning to 6.9.0 directly will maintain the expected behavior until that issue gets sorted.

@kadamwhite
Copy link
Collaborator Author

Finally green! @gnarf 👍 'd this, so mergin'

@kadamwhite kadamwhite merged commit fa9614e into master Mar 20, 2017
@kadamwhite kadamwhite deleted the storybook branch March 20, 2017 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants