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

add forwardRef support to addon-info #4961

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

yjcxy12
Copy link

@yjcxy12 yjcxy12 commented Dec 10, 2018

Issue: closes #4787

What I did

  • added react-is to dependencies in addon-info.
  • in both Node and Story checks if the element is forwardRef element and parses accordingly.
  • add new example of forwardRef component in kitchen sink app.

How to test

  • Is this testable with Jest or Chromatic screenshots?
    Yes.
  • Does this need a new example in the kitchen sink apps?
    Yes.
  • Does this need an update to the documentation?
    Don't think so.

If your answer is yes to any of these, please make sure to include it in your PR.

@igor-dv
Copy link
Member

igor-dv commented Dec 11, 2018

In docs they mentioned to just use a dislpayName.
Is your solution better ?

@codecov
Copy link

codecov bot commented Dec 11, 2018

Codecov Report

Merging #4961 into next will increase coverage by <.01%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #4961      +/-   ##
==========================================
+ Coverage   35.06%   35.06%   +<.01%     
==========================================
  Files         563      563              
  Lines        6970     6975       +5     
  Branches      932      934       +2     
==========================================
+ Hits         2444     2446       +2     
- Misses       4031     4034       +3     
  Partials      495      495
Impacted Files Coverage Δ
addons/info/src/components/Node.js 87.5% <33.33%> (-5.61%) ⬇️
addons/info/src/components/Story.js 88.11% <50%> (-0.78%) ⬇️

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 53c4d36...29fe3b9. Read the comment docs.

@ndelangen ndelangen merged commit f0961c3 into storybookjs:next Dec 12, 2018
@ndelangen
Copy link
Member

@yjcxy12 this is awesome! thank you!

@ndelangen
Copy link
Member

This code throws when the component uses a react hook.

See this issue: #6848

I've been able to make some adjustments to make that use-case work, I'll open a PR momentarily. Will you be able to review @yjcxy12

Perhaps a solution based on displayName would be better indeed @igor-dv

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.

addon-info doesn't handle React.forwardRef appropriately
3 participants