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 #5785 - welcome component in dark theme #5998

Merged
merged 1 commit into from
Mar 11, 2019

Conversation

malykhinvi
Copy link
Contributor

Issue: #5785 - missing background in the Welcome component makes content unreadable in dark theme.

What I did

  1. Added white background for all /demo "Welcome" components
  2. Added white background for all /examples "Welcome" components
  3. Changed from gap from margin to padding, so there would be bigger gap between container and content
  4. Removed max width, so that in dark theme container with white background would look better. That is very subjective change, so let me know if I should revert it or not.

How to test

  • Is this testable with Jest or Chromatic screenshots?
    Yes, I updated jest snapshots. It can be tested with some visual regression testing tool, but I'm not sure what is the proper way to do it here.
  • Does this need a new example in the kitchen sink apps?
    No
  • Does this need an update to the documentation?
    No

I checked all kitchen-sink examples. Also I set up storybook in one of my react projects, and run storybook with linked @storybook/react. Looks like it works, however, I didn't check cli with other frameworks.

Below you can find before/after screenshots
SN-5785-PR

@shilman shilman added bug cli patch:yes Bugfix & documentation PR that need to be picked to main branch labels Mar 10, 2019
@shilman shilman added this to the 5.0.x milestone Mar 10, 2019
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.

Thanks so much for taking care of this! 🎉 LGTM!!!

@codecov
Copy link

codecov bot commented Mar 11, 2019

Codecov Report

Merging #5998 into next will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #5998      +/-   ##
==========================================
- Coverage   34.93%   34.92%   -0.01%     
==========================================
  Files         648      648              
  Lines        9515     9512       -3     
  Branches     1380     1379       -1     
==========================================
- Hits         3324     3322       -2     
+ Misses       5574     5573       -1     
  Partials      617      617
Impacted Files Coverage Δ
app/angular/src/demo/welcome.component.ts 0% <ø> (ø) ⬆️
app/react/src/demo/Welcome.js 0% <ø> (ø) ⬆️
lib/ui/src/core/stories.js 87.23% <0%> (-0.27%) ⬇️
addons/a11y/src/components/Report/Item.tsx 0% <0%> (ø) ⬆️
addons/viewport/src/Tool.js 0% <0%> (ø) ⬆️
addons/a11y/src/components/Report/Rules.tsx 0% <0%> (ø) ⬆️
addons/a11y/src/components/A11YPanel.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 3b18b02...3eb19ca. Read the comment docs.

@shilman shilman merged commit 8e68e14 into storybookjs:next Mar 11, 2019
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Mar 11, 2019
@shilman shilman 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 12, 2019
shilman added a commit that referenced this pull request Mar 15, 2019
Fix #5785 - welcome component in dark theme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cli 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.

2 participants