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 #6077: issue with selectedPanel reset #6149

Merged
merged 3 commits into from
Mar 20, 2019
Merged

Conversation

malykhinvi
Copy link
Contributor

@malykhinvi malykhinvi commented Mar 18, 2019

Issue: #6077

What I did

Added check for options.selectedPanel presence, so that if it is undefined it won't reset the currently selected panel.

How to test

  • Is this testable with Jest or Chromatic screenshots? Yes, I added unit tests
  • Does this need a new example in the kitchen sink apps? No
  • Does this need an update to the documentation? No

@codecov
Copy link

codecov bot commented Mar 18, 2019

Codecov Report

Merging #6149 into next will increase coverage by 0.38%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##            next    #6149      +/-   ##
=========================================
+ Coverage   37.9%   38.29%   +0.38%     
=========================================
  Files        645      643       -2     
  Lines       9642     9420     -222     
  Branches     354     1344     +990     
=========================================
- Hits        3655     3607      -48     
+ Misses      5940     5254     -686     
- Partials      47      559     +512
Impacted Files Coverage Δ
lib/ui/src/core/layout.js 51.51% <100%> (ø)
lib/ui/src/settings/shortcuts_page.js 0% <0%> (-25%) ⬇️
lib/ui/src/containers/panel.js 14.28% <0%> (-19.05%) ⬇️
lib/ui/src/containers/nav.js 11.76% <0%> (-4.91%) ⬇️
addons/notes/src/Panel.tsx 39.62% <0%> (-4.28%) ⬇️
app/angular/src/client/preview/angular/helpers.ts 38.63% <0%> (-0.14%) ⬇️
app/polymer/src/client/index.js 0% <0%> (ø) ⬆️
app/html/standalone.js 0% <0%> (ø) ⬆️
lib/core/src/server/config.js 0% <0%> (ø) ⬆️
lib/core/src/server/cli/prod.js 0% <0%> (ø) ⬆️
... and 231 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 934da0d...047d712. Read the comment docs.

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

LGTM and great job on the test cases 👌 Thanks so much for taking care of this!

@malykhinvi
Copy link
Contributor Author

malykhinvi commented Mar 20, 2019

I see a conflict, looks like layout.js has been moved to lib/api/src/modules/layout.ts, but test is left untouched. Do you want me to resolve conflicts?

@shilman
Copy link
Member

shilman commented Mar 20, 2019

@malykhinvi yes please! 🙏

@malykhinvi
Copy link
Contributor Author

@shilman done!

@shilman shilman merged commit 320c610 into storybookjs:next Mar 20, 2019
@shilman shilman added patch:done Patch/release PRs already cherry-picked to main/release branch and removed patch:yes Bugfix & documentation PR that need to be picked to main branch patch:done Patch/release PRs already cherry-picked to main/release branch labels Mar 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants