-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
App for Mithril #3244
App for Mithril #3244
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3244 +/- ##
==========================================
- Coverage 36.05% 35.33% -0.72%
==========================================
Files 445 470 +25
Lines 9793 10018 +225
Branches 908 954 +46
==========================================
+ Hits 3531 3540 +9
- Misses 5707 5863 +156
- Partials 555 615 +60
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool!
Missing things:
- https://github.com/storybooks/storybook/#supported-frameworks
- https://github.com/storybooks/storybook/blob/master/ADDONS_SUPPORT.md
- Section in docs?
- Netlify (@ndelangen) + live examples section
app/mithril/package.json
Outdated
"prepare": "node ../../scripts/prepare.js" | ||
}, | ||
"dependencies": { | ||
"@storybook/addon-links": "3.4.0-rc.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to be dependant on specific addons. Every addon example should be in a dedicated example-app.
app/mithril/src/demo/Welcome.js
Outdated
/** @jsx m */ | ||
|
||
import m from 'mithril'; | ||
import { linkTo, hrefTo } from '@storybook/addon-links'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be used in the example-app.
@@ -0,0 +1,7 @@ | |||
import deprecate from 'util-deprecate'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's deprecated, I think you don't need to add it.
<meta content="IE=edge" http-equiv="X-UA-Compatible" /> | ||
<base target="_parent"> | ||
<script> | ||
if (window.parent !== window) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it's redundant
@@ -0,0 +1,3 @@ | |||
# Storybook Demo | |||
|
|||
This is a demo app to test Mithril integration with Storybook. Run `npm install` to sync Storybook module with the source code and run `npm run storybook` to start the Storybook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why "npm install", doesn't it bootstrapped with yarn workspace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied it from the vue-kitchen-sink. I'm glad to tell me a good readme since I'm not good at English.
I'm done as much as I could. But I don't know how to setup netlify, so I skipped a live example. |
@ndelangen , maybe you can help with it? |
README.md
Outdated
@@ -72,6 +72,7 @@ For additional help, join us [in our Slack](https://now-examples-slackin-rrirkqo | |||
- [Vue](app/vue) | |||
- [Angular](app/angular) | |||
- [Polymer](app/polymer) <sup>alpha</sup> | |||
- [Mithril](app/mithril) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make it alpha as well?
/** @jsx m */ | ||
|
||
import m from 'mithril'; | ||
import { linkTo, hrefTo } from '@storybook/addon-links'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant in my previous comment, is that we don't want our (demo) components to be dependant on any storybook addons. Think about the real world app. Probably you will expose some callbacks and will use them inside a stories.js
file with bindings to the particular addon.
Sorry for the late reply. Yeah I can setup a netlify deploy for the mythril example no problem 🎉 |
#3297 should fix the CLI tests failure |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @sixmen
Released as |
Issue:
What I did
Add a new storybook app for Mithril framework
How to test
Is this testable with jest or storyshots?
Does this need a new example in the kitchen sink apps?
Does this need an update to the documentation?
If your answer is yes to any of these, please make sure to include it in your PR.