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

Addons: avoid mixing manager and preview code together #3068

Merged
merged 1 commit into from
Feb 24, 2018

Conversation

Hypnosphi
Copy link
Member

Issue: #2275 accidentally broke React Native. The problem is that glamorous contains browser-specific code, and this line makes it leak into RN: https://github.com/storybooks/storybook/blob/master/addons/actions/src/index.js#L6

What I did

Ensured that manager and preview code is separated in all existing addons

@Hypnosphi Hypnosphi requested review from igor-dv and a team February 24, 2018 16:21
@codecov
Copy link

codecov bot commented Feb 24, 2018

Codecov Report

Merging #3068 into master will decrease coverage by 1.18%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3068      +/-   ##
=========================================
- Coverage   36.78%   35.6%   -1.19%     
=========================================
  Files         420     420              
  Lines        9873    9889      +16     
  Branches      831     827       -4     
=========================================
- Hits         3632    3521     -111     
- Misses       5788    5893     +105     
- Partials      453     475      +22
Impacted Files Coverage Δ
addons/actions/src/index.js 100% <ø> (ø) ⬆️
addons/links/src/index.js 55.55% <ø> (ø) ⬆️
addons/storysource/src/index.js 0% <ø> (ø) ⬆️
addons/links/register.js 0% <0%> (ø) ⬆️
addons/actions/register.js 0% <0%> (ø) ⬆️
addons/events/register.js 0% <0%> (ø) ⬆️
addons/storysource/register.js 0% <0%> (ø) ⬆️
...ddons/actions/src/components/ActionLogger/style.js 0% <0%> (-100%) ⬇️
...ddons/actions/src/components/ActionLogger/index.js 0% <0%> (-73.69%) ⬇️
...ddons/actions/src/containers/ActionLogger/index.js 0% <0%> (-49.26%) ⬇️
... and 59 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 4d4d974...3eb012f. Read the comment docs.

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.

LGTM.
I hope people don't use "register" from the index file 🤷‍♂️

@Hypnosphi Hypnosphi merged commit fed8c21 into master Feb 24, 2018
@Hypnosphi Hypnosphi deleted the avoid-mixing-manager-and-preview branch February 24, 2018 16:46
@Hypnosphi Hypnosphi added patch:done Patch/release PRs already cherry-picked to main/release branch and removed patch:done Patch/release PRs already cherry-picked to main/release branch labels Mar 6, 2018
Hypnosphi added a commit that referenced this pull request Mar 6, 2018
…view

Addons: avoid mixing manager and preview code together
# Conflicts:
#	addons/storysource/register.js
#	addons/storysource/src/index.js
@Hypnosphi Hypnosphi added the patch:done Patch/release PRs already cherry-picked to main/release branch label Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: actions addon: events addon: links addon: storysource bug patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch react-native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants