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 (addon/backgrounds): addon panel doesn't hide #6042

Merged
merged 3 commits into from
Mar 15, 2019

Conversation

divslinger
Copy link
Contributor

Fixes #6041

Instead of turning the preview transparent, addon-backgrounds should set the background color to the theme's default.
I added the background import and set background.content as the replacement for all transparent returns

Instead of turning the preview transparent, addon-backgrounds should set the background color to the theme's default.
I added the background import and set background.content as the replacement for all transparent returns
@shilman shilman added bug addon: backgrounds patch:yes Bugfix & documentation PR that need to be picked to main branch labels Mar 12, 2019
@ndelangen
Copy link
Member

This would make the preview background not respect the user's theme.

@ndelangen ndelangen self-assigned this Mar 15, 2019
@codecov
Copy link

codecov bot commented Mar 15, 2019

Codecov Report

Merging #6042 into next will decrease coverage by 1.86%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #6042      +/-   ##
==========================================
- Coverage   37.65%   35.78%   -1.87%     
==========================================
  Files         642      648       +6     
  Lines        9388     9520     +132     
  Branches     1364     1380      +16     
==========================================
- Hits         3535     3407     -128     
- Misses       5293     5536     +243     
- Partials      560      577      +17
Impacted Files Coverage Δ
.../backgrounds/src/containers/BackgroundSelector.tsx 0% <0%> (ø) ⬆️
addons/a11y/src/components/Report/index.tsx 0% <0%> (-100%) ⬇️
addons/a11y/src/constants.ts 0% <0%> (-100%) ⬇️
addons/a11y/src/components/A11YPanel.tsx 0% <0%> (-94.45%) ⬇️
addons/a11y/src/components/Tabs.tsx 0% <0%> (-90.48%) ⬇️
addons/a11y/src/components/Report/Info.tsx 0% <0%> (-85.72%) ⬇️
addons/a11y/src/components/Report/Item.tsx 0% <0%> (-85%) ⬇️
addons/a11y/src/components/Report/Tags.tsx 0% <0%> (-71.43%) ⬇️
addons/a11y/src/components/Report/Rules.tsx 0% <0%> (-66.67%) ⬇️
addons/a11y/src/components/Report/Elements.tsx 0% <0%> (-63.64%) ⬇️
... and 38 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 4bc05ee...c70b7ad. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 15, 2019

Codecov Report

Merging #6042 into next will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##             next    #6042   +/-   ##
=======================================
  Coverage   37.65%   37.65%           
=======================================
  Files         642      642           
  Lines        9388     9388           
  Branches     1364     1364           
=======================================
  Hits         3535     3535           
  Misses       5293     5293           
  Partials      560      560
Impacted Files Coverage Δ
.../backgrounds/src/containers/BackgroundSelector.tsx 0% <0%> (ø) ⬆️

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 4bc05ee...9dabf94. Read the comment docs.

ndelangen added a commit that referenced this pull request Mar 15, 2019
…le, when they are not supposed to be

Thanks to glitchperfect's #6042
@ndelangen
Copy link
Member

I refactored this addon quite a bit in my PR for migrating the core to typescript.

I incorporated this fix into that PR: 5592373

I also made sure the user's theme is respected.

@ndelangen
Copy link
Member

I just pushed a little fix to this PR, because I want @GlitchPerfect to have this PR merged.

I really appreciate the effort.

I think this should be good to merge now.

I'll deal with the merge conflict in my api branch.

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.

LGTM

@shilman shilman added this to the 5.0.x milestone Mar 15, 2019
@shilman shilman merged commit 79a5243 into storybookjs:next Mar 15, 2019
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Mar 17, 2019
shilman added a commit that referenced this pull request Mar 17, 2019
fix (addon/backgrounds): addon panel doesn't hide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: backgrounds 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants