React storybook

[dmose]

I've removed the storybooks.io integration for the moment, in the interest of getting something landed, and because the current version of React storybook seems to have an odd install issue with it.

Presumably I'll need to squash this before landing, I've left the commits for reviewing, though, as they might be useful in understanding the motivation for given changes.

r? @k88hudson

---

This change is

[dmose]

"npm run travis" gets much further locally than on travis itself. It kinda feels like somehow webpack is being run in a context on travis where the JS dialect doesn't support destructuring assignment. Hmmm...

Will investigate further tomorrow.

[coveralls]

Coverage remained the same at 92.116% when pulling e7fc8e4 on dmose:react-storybook into 8a5ab07 on mozilla:master.

[pdehaan]

Nit: odd mix of single and double quotes in this file, which will surely (hopefully) make ESLint cry.

[pdehaan]

Nit: I think this should be "glyph-showmore-16.svg" (not hyphenated "show-more").

[pdehaan]

Filed as #1057

[dmose]

content-src/styles/icons.scss, line 67 [r3] (raw file):

Previously, pdehaan (Peter deHaan) wrote…

Nit: I think this should be "glyph-showmore-16.svg" (not hyphenated "show-more").

This patch is not changing any filenames. Let's not entrain more than is necessary in the interest of landing this sooner.

---

Comments from Reviewable

[k88hudson]

This might actually be better as a static directory (it would reduce the additional configuration needed in webpack) https://github.com/kadirahq/react-storybook/blob/master/docs/configure_storybook.md#static-directory

[k88hudson]

Looks great!

Do you think when you're ready to land this you could squash some of your commits? We use the angular commit style with the github issue added in if necessary (for example feat(content): #1 Added a feature)

I noticed the webpack build is throwing out weird messages and being really slow, I'm thinking it might be because react storybook is trying to process node_modules with babel loader instead of just source files:

[BABEL] Note: The code generator has deoptimised the styling of "/Users/khudson/github/activity-stream/node_modules/es6-shim/es6-shim.js" as it exceeds the max of "100KB".
[BABEL] Note: The code generator has deoptimised the styling of "/Users/khudson/github/activity-stream/node_modules/moment/moment.js" as it exceeds the max of "100KB".
[BABEL] Note: The code generator has deoptimised the styling of "/Users/khudson/github/activity-stream/node_modules/faker/lib/locales/en/system/mimeTypes.js" as it exceeds the max of "100KB".

[k88hudson]

Weirdly, their code appears to exclude node_modules: https://github.com/kadirahq/react-storybook/blob/0be9294e485e51fa7c5f7d1cc5570269227d4ffd/src/server/config/paths.js

[dmose]

After merging your latest commit, these messages are gone, so I'm betting your guess was correct.

---

Comments from Reviewable

[dmose]

package.json, line 185 [r3] (raw file):

Previously, k88hudson (Kate Hudson) wrote…

Ah, so this is already in here! Should work without the require css line then, no?

Removing the require("main.css") breaks things (i.e. no CSS is loaded), which makes sense to me. It has to get the CSS from somewhere, and activity stream itself gets it from a link rel="stylesheet" in the HTML. We could do that here too, I guess, but it seems like 6 of 1, half-dozen of the other...

---

Comments from Reviewable

[dmose]

.storybook/config.js, line 3 [r3] (raw file):

Previously, k88hudson (Kate Hudson) wrote…

This might actually be better as a static directory (it would reduce the additional configuration needed in webpack) https://github.com/kadirahq/react-storybook/blob/master/docs/configure_storybook.md#static-directory

So would you prefer this to be loaded via an HTML tag? (which itself would require a bit of special config)

---

Comments from Reviewable

[dmose]

.storybook/webpack.config.js, line 14 [r3] (raw file):

Previously, dmose (Dan Mosedale) wrote…

(and fix the quoting issues)

Done.

---

Comments from Reviewable

[dmose]

content-test/components/Spotlight.story.js, line 23 [r3] (raw file):

Previously, k88hudson (Kate Hudson) wrote…

Might be lorempixel which is providing the faker images

I've updated the comment to include that.

---

Comments from Reviewable

[dmose]

content-src/styles/icons.scss, line 67 [r3] (raw file):

Previously, pdehaan (Peter deHaan) wrote…

Filed as #1057

Ok

---

Comments from Reviewable

[coveralls]

Coverage remained the same at 92.016% when pulling a2afaf5 on dmose:react-storybook into d51c73c on mozilla:master.

[coveralls]

Coverage remained the same at 91.992% when pulling cbc2cad on dmose:react-storybook into e4697d7 on mozilla:master.

[dmose]

OK, here's an updated patch. Highlights:

• stop using webpack to load CSS, which simplifies webpack config and removes 2 deps
• switch all the url()s in the scss files to the new syntax
• move more stuff into webpack.common
• updated to latest react-storybook available on npm (by the time you read this, there will probably be an even slightly newer on)
• fixed eslint issues.

[k88hudson]

@dmose 🔥 🔥 🔥 🔥 🔥 Land it!