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

TASK: Storybook integration #153

Merged
merged 5 commits into from
Jun 11, 2016
Merged

TASK: Storybook integration #153

merged 5 commits into from
Jun 11, 2016

Conversation

dimaip
Copy link
Contributor

@dimaip dimaip commented May 18, 2016

Storybook allows running each UI component in isolation from the rest of UI. It even allows developing UI components without even setting up Neos installation itself!

We can also integrate http://storybooks.io/ when it's launched, so we'd be able to have an online reference and reviewing of UI code, without needing to test the pull request locally.

  • Add storeys for all components
  • Documentat the Storybook workflow

@dimaip
Copy link
Contributor Author

dimaip commented May 19, 2016

There are some essential documentation features missing for Storybook, but it seems these guys plan to improve on that: storybookjs/storybook#166 (comment)

@dimaip
Copy link
Contributor Author

dimaip commented May 30, 2016

I have created storybooks for all of our components.
There are numerous problems with individual components that storybook helped me to spot, but the should be fixed in an independent branch.
I will document the storybook workflow and then I think we can merge this branch.

@dimaip
Copy link
Contributor Author

dimaip commented May 30, 2016

/cc @Inkdpixels please have a look!

@Inkdpixels
Copy link
Contributor

Looks pretty solid by reading! :) 👍

@dimaip
Copy link
Contributor Author

dimaip commented May 30, 2016

@Inkdpixels do you know perhaps why the test is failing? I couldn't have touched anything related to it...

@Inkdpixels
Copy link
Contributor

I will take a look at it this evening :)

@Inkdpixels
Copy link
Contributor

Hey @dimaip - I checked out your PR locally and I can indeed reproduce this error we are getting on CI. Until tomorrow evening I don't have much time to look into it, but I've found a storybook related issue of some guy which has the same error message, I think that's a good path we can follow.

I you don't mind waiting, I will try to fix it tomorrow evening :)

storybookjs/storybook#174

@Inkdpixels
Copy link
Contributor

Hey @dimaip - I've found the location of the error. Basically the problem lies in the karma.entry.js file, which will in fact require all .js files, since this is important for coverage statistics. The workaround is to adopt the regular expression to something like /(?!.*story.js$)^.*js$/ which will import all .js files but ignores all files which end with .story.js.

I've tested this locally, and it works.Could you either commit this, or give me access to your fork? Thanks buddy :)

@dimaip
Copy link
Contributor Author

dimaip commented Jun 3, 2016

@Inkdpixels thanks a lot for helping me out on this! Granted you access. I myself will be able to fix it on Monday, as well as document it.

@Inkdpixels
Copy link
Contributor

Inkdpixels commented Jun 6, 2016

yay, done 💯
Shall we merge this already, or is the task of documenting still open? :)

@Inkdpixels
Copy link
Contributor

Hey @dimaip - I will merge this if you dont mind, since the only task left seems to be documentation and I also need to work on the previuosly changed karma.entry file. :)

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.

2 participants