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

Storybook for hyperapp components #3767

Closed
wants to merge 12 commits into from

Conversation

Zmoki
Copy link

@Zmoki Zmoki commented Jun 15, 2018

Issue:

Hyperapp is a JavaScript micro-framework that uses JSX.

I started using hyperapp in a new project and I want to use storybook with it.

What I did

I added hyperapp to app and to gerenerators.

How to test

Is this testable with Jest or Chromatic screenshots?
Does this need a new example in the kitchen sink apps?
Does this need an update to the documentation?

Ok, I'll add example soon and update main docs.

Do I need add test? How I can do it?

@ndelangen
Copy link
Member

WOW awesome @Zmoki !!!

@ndelangen
Copy link
Member

Indeed adding an example will allow us to test it 👍

Copy link
Member

@igor-dv igor-dv left a comment

Choose a reason for hiding this comment

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

Nice 👍

"@storybook/hyperapp": "4.0.0-alpha.9",
"babel-core": "^6.26.3",
"babel-preset-env": "^1.7.0",
"parcel": "^1.9.0"
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to bundle it with parcel?

"picostyle": "^2.0.1"
},
"devDependencies": {
"@storybook/addon-actions": "4.0.0-alpha.9",
Copy link
Member

Choose a reason for hiding this comment

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

Are you going to add examples for the addons support in this PR? If not, let's remove unneeded deps. Anyway please try to add addon-storysource, it will help with the live examples.

@Zmoki
Copy link
Author

Zmoki commented Jun 15, 2018

After the weekend I'll continue to work on this PR

@@ -0,0 +1,3 @@
docs
src
.babelrc
Copy link
Member

Choose a reason for hiding this comment

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

Is this file actually needed here?

Copy link
Member

Choose a reason for hiding this comment

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

nah it shouldn't I think

@@ -0,0 +1 @@
export default config => config;
Copy link
Member

Choose a reason for hiding this comment

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

wrapInitialConfig is optional, you can omit it if it's not needed

"pragma": "h"
}
]
]
Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about adding it automatically like @storybook/react does?

@Hypnosphi
Copy link
Member

Hypnosphi commented Jun 16, 2018

Things left to do here:

For now, there's nothing on https://storybooks-hyperapp.netlify.com because it's deployed from master. Examples for this branch are available on https://deploy-preview-3767--storybooks-hyperapp.netlify.com

'react/jsx-uses-vars': 2,
'react/react-in-jsx-scope': 0,
},
};
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this, you can add /** @jsx Foo.bar */ pragma to files that use jsx
See https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/react-in-jsx-scope.md

Copy link
Member

Choose a reason for hiding this comment

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

We use this approach for mithril, and it works well for us

Copy link
Member

Choose a reason for hiding this comment

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

that sounds like a good suggestion!

.add('welcome', () => <Welcome />);

storiesOf('Demo', module).add('button', () => (
<Button onclick={() => action('button-click')}>Click me</Button>
Copy link
Member

Choose a reason for hiding this comment

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

action(name) returns a function, so this should be onclick={action('button-click')} instead

@@ -0,0 +1 @@
module.exports = require('./dist/server/options');
Copy link
Member

Choose a reason for hiding this comment

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

This file is already unneeded.

@Hypnosphi
Copy link
Member

Please apply changes from #3744

@ndelangen
Copy link
Member

Hey @Zmoki what do you need from me to be able to finish this?

@Zmoki
Copy link
Author

Zmoki commented Jul 26, 2018

Sorry for the delay. I'll is given work time next week and I will be able to finish it.

@ndelangen
Copy link
Member

Awesome!

@stale
Copy link

stale bot commented Aug 16, 2018

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Aug 16, 2018
@ndelangen
Copy link
Member

@Zmoki are you interested in finishing this so all the tests pass? No worries if you need more time!

@stale stale bot removed the inactive label Aug 18, 2018
@igor-dv
Copy link
Member

igor-dv commented Aug 22, 2018

I've temporarily disabled netlify for hyperapp. Will return it back when it's relevant

@ndelangen ndelangen closed this Sep 6, 2018
@ndelangen
Copy link
Member

Happy to re-open when there's interest!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants