-
-
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
Move LinkTo component to a separate addon-links/react
endpoint
#2239
Conversation
LGTM! |
Isn't this a breaking change? Can we keep the old behavior & deprecate it until 4.0? |
@shilman The feature was introduced only in prerelease, so we're free to change it. And the bug I fix here blocks storybook for RN from working |
Codecov Report
@@ Coverage Diff @@
## release/3.3 #2239 +/- ##
===============================================
- Coverage 22.63% 22.62% -0.01%
===============================================
Files 344 346 +2
Lines 6763 6770 +7
Branches 890 905 +15
===============================================
+ Hits 1531 1532 +1
+ Misses 4578 4554 -24
- Partials 654 684 +30
Continue to review full report at Codecov.
|
|
||
storiesOf('Welcome', module).add('to Storybook', () => <Welcome showApp={linkTo('Button')} />); | ||
storiesOf('Welcome', module).add('to Storybook', () => <Welcome showKind="Button" />); |
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.
do we need to update these in the cli templates?
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.
This was already updated in #1829. Actually I just did some copy-pasting from commented code in stories/index.js
to partially restore the release/3.3
changes.
If you feel like it, you can sync the rest of the changes in another PR
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 does this need to change?
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.
It has changed in #1829 not now. It’basically adding a usage of a new feature
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.
Ok, well I'm OK with this change.
Issue: #2096
What I did
Moved react/browser-specific stuff to a separate endpoint
How to test
Run crna-kitchen-sink