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

Action logger breaks stateful components #116

Closed
willdady opened this issue Apr 18, 2016 · 10 comments
Closed

Action logger breaks stateful components #116

willdady opened this issue Apr 18, 2016 · 10 comments
Labels

Comments

@willdady
Copy link

willdady commented Apr 18, 2016

I've just started using storybook and so far it's been great. I've ran into the following issue. Basically if I have a component which maintains it's own internal state the action logger causes the component to be re-rendered when called, effectively creating the component anew.

The following breaks the component by re-rendering the component...

storiesOf('ColorPicker', module)
  .add('default', () => (
    <ColorPicker onChange={action('onChange')} />
  ));

...but the following does not.

storiesOf('ColorPicker', module)
  .add('default', () => (
    <ColorPicker onChange={(color) => console.log(color)} />
  ));

Is this the expected behaviour? Should calling action() really cause a re-render?

@arunoda arunoda added the bug label Apr 19, 2016
@arunoda
Copy link
Member

arunoda commented Apr 19, 2016

I can see what's going on here. We should not render in that case.

Note:
Find out a way to stop rendering preview iframe if there's no related data change.

@necolas
Copy link
Contributor

necolas commented Apr 20, 2016

That sounds good! The re-render also causes assets like images to flash every time an action is taken as the whole story gets re-rendered.

@arunoda
Copy link
Member

arunoda commented Apr 20, 2016

Actually, I find out a simple way to fix this.
Here's the line we need to focus: https://github.com/kadirahq/react-storybook/blob/master/src/client/ui/preview.js#L22

We can check for equality of both selectedKind and selectedStory and proceed if only at least one of them has changed.

@ghost ghost mentioned this issue Apr 20, 2016
@arunoda
Copy link
Member

arunoda commented Apr 20, 2016

Guys, now we've a fix for this with PR #126.
Try this version: v1.16.0

@akblurton
Copy link
Contributor

akblurton commented Apr 20, 2016

I'm not sure if I'm doing anything wrong (just picked up storybook, loving it). But it seems like I still get this error if I'm passing the event object from a form submit, rather than a click. More specifically the Storage quote error. Any ideas? I'm assuming that React is sending a different event type that isn't handled.

Edit: Doing some additional testing, it is because I was proxying the default submit event listener and sending the form data as the first parameter to the passed action, and the event as the second i.e.

this.props.onSubmit(values, e)

This causes the system to hang.

@willdady
Copy link
Author

This update seems to have fixed my original issue above. Thanks.

@arunoda
Copy link
Member

arunoda commented Apr 21, 2016

@willdady great.
@de-monkeyz could you submit a new issue.

@arunoda arunoda closed this as completed Apr 21, 2016
@arunoda
Copy link
Member

arunoda commented Apr 21, 2016

Note: @de-monkeyz mentioned issue fixed by his PR: #132

@kevinSuttle
Copy link
Contributor

I came here confused as to how to exec JS in the actions call. I'd rather just call a function on the component that sends data to the action logger.

 <ColorPicker onChange={action('state changed', this.state.colorValue)} />

@iwarner
Copy link

iwarner commented Jun 4, 2018

handleEntry: () => action('entry-change')

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

No branches or pull requests

6 participants