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

fix(addons/info): Cannot read property 'props' of undefined #1258

Merged
merged 5 commits into from
Jun 12, 2017

Conversation

evenchange4
Copy link
Contributor

@evenchange4 evenchange4 commented Jun 12, 2017

Issue:

This error occurred when I want to update storybook. (I use yarn to lock versions.)

-    "@storybook/addon-info": "^3.0.1",
-    "@storybook/addon-options": "^3.0.1",
-    "@storybook/addon-storyshots": "^3.0.0",
-    "@storybook/react": "^3.0.1",
+    "@storybook/addon-info": "^3.1.3",
+    "@storybook/addon-options": "^3.1.2",
+    "@storybook/addon-storyshots": "^3.1.2",
+    "@storybook/react": "^3.1.3",

2017-06-12 11 02 52

What I did

Remove the this for functional component.

How to test

Add some links for your story description in the second paramter of addWithInfo:

storiesOf('DomAlign', module).addWithInfo(
  'API',
  `
      With config: ${JSON.stringify(alignConfig)}.
+     Reference: React.js [dom-align](https://github.com/yiminghe/dom-align) integration component.
    `,
  () => <Container />,
  { inline: true, propTables: [DomAlign] },
);

There will be an a tag rendered on the page, and this PR fixed the A component error.

Source code
Demo link
Related issue mcs-lite PR#357

fix(addons/info): Cannot read property 'props' of undefined
@codecov
Copy link

codecov bot commented Jun 12, 2017

Codecov Report

Merging #1258 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1258   +/-   ##
=======================================
  Coverage   13.73%   13.73%           
=======================================
  Files         207      207           
  Lines        4638     4638           
  Branches      513      504    -9     
=======================================
  Hits          637      637           
- Misses       3557     3567   +10     
+ Partials      444      434   -10
Impacted Files Coverage Δ
addons/info/src/components/markdown/text.js 0% <0%> (ø) ⬆️
...react-native/src/manager/components/PreviewHelp.js 0% <0%> (ø) ⬆️
...codemod/src/transforms/update-organisation-name.js 40.62% <0%> (ø) ⬆️
app/react-native/src/preview/story_kind.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/down_panel.js 23.52% <0%> (ø) ⬆️
addons/events/src/components/Event.js 0% <0%> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 33.33% <0%> (ø) ⬆️
app/react/src/client/preview/init.js 0% <0%> (ø) ⬆️
addons/storyshots/src/storybook-channel-mock.js 0% <0%> (ø) ⬆️
app/react-native/src/bin/storybook-start.js 0% <0%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a81283f...106514f. Read the comment docs.

@shilman
Copy link
Member

shilman commented Jun 12, 2017

@evenchange4 Thanks for the contribution! It looks good to me. Can you explain how to reproduce the problem and test the change? Thanks!

@evenchange4
Copy link
Contributor Author

@shilman Sure, I have updated my description above.

@shilman
Copy link
Member

shilman commented Jun 12, 2017

Thanks! Confirmed the problem and added a test in #1259

shilman added a commit that referenced this pull request Jun 12, 2017
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Requested a small change. what do you think?

@@ -42,8 +42,8 @@ export function A(props) {
const style = {
color: '#3498db',
};
return <a href={this.props.href} style={style}>{props.children}</a>;
return <a href={props.href} style={style}>{props.children}</a>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: add target="_blank" rel="noopener noreferrer"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I always put external links here, and there is already @storybook/addon-links solution for linking between stories at same site.

@shilman shilman merged commit 8bfdc25 into storybookjs:master Jun 12, 2017
@evenchange4 evenchange4 deleted the patch-1 branch June 12, 2017 12:29
@shilman shilman added the bug label Jun 15, 2017
@shilman
Copy link
Member

shilman commented Jun 15, 2017

released https://github.com/storybooks/storybook/releases/tag/v3.1.4

thanks @evenchange4 !

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

Successfully merging this pull request may close these issues.

3 participants