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

Persist background for @addon/background #3331

Merged
merged 4 commits into from
Apr 2, 2018

Conversation

wuweiweiwu
Copy link
Member

@wuweiweiwu wuweiweiwu commented Apr 1, 2018

Issue: #2383

What I did

don't unset background url when changing stories.

How to test

Is this testable with Jest or Chromatic screenshots?
yes
Does this need a new example in the kitchen sink apps?
no
Does this need an update to the documentation?
no
If your answer is yes to any of these, please make sure to include it in your PR.


this.channel.on('background-set', backgrounds => {
this.setState({ backgrounds });
const currentBackground = api.getQueryParam('background');
const currentBackground = api.getQueryParam('background') || this.state.currentBackground;
Copy link
Member

@Hypnosphi Hypnosphi Apr 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer URL to be the single source of truth here. We already persist background changes in URL, let's just stop resetting that on story change:
https://github.com/wuweiweiwu/storybook/blob/12c5af9e513f989e03db87475811b86a4cdd718a/addons/background/src/BackgroundPanel.js#L91

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! Will update :)

@codecov
Copy link

codecov bot commented Apr 2, 2018

Codecov Report

Merging #3331 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3331      +/-   ##
==========================================
- Coverage   35.63%   35.62%   -0.01%     
==========================================
  Files         472      472              
  Lines       10122    10121       -1     
  Branches      970      937      -33     
==========================================
- Hits         3607     3606       -1     
- Misses       5900     5918      +18     
+ Partials      615      597      -18
Impacted Files Coverage Δ
addons/background/src/BackgroundPanel.js 92.3% <ø> (-0.2%) ⬇️
app/mithril/src/server/utils.js 0% <0%> (-100%) ⬇️
app/react-native/src/bin/storybook-start.js 0% <0%> (ø) ⬆️
addons/info/src/components/types/PropertyLabel.js 78.94% <0%> (ø) ⬆️
addons/a11y/src/components/Report/index.js 0% <0%> (ø) ⬆️
addons/viewport/src/preview/withViewport.js 77.27% <0%> (ø) ⬆️
addons/knobs/src/angular/utils.js 82.14% <0%> (ø) ⬆️
addons/a11y/src/components/Report/Rules.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/libs/hierarchy.js 49.03% <0%> (ø) ⬆️
...i/src/modules/ui/components/stories_panel/index.js 21.66% <0%> (ø) ⬆️
... and 68 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 d54fb75...7bbcd7d. Read the comment docs.

@wuweiweiwu
Copy link
Member Author

@Hypnosphi Done! :)

@wuweiweiwu wuweiweiwu changed the title Add currentBackground for @addon/background Add persist background for @addon/background Apr 2, 2018
@wuweiweiwu wuweiweiwu changed the title Add persist background for @addon/background Persist background for @addon/background Apr 2, 2018
@Hypnosphi Hypnosphi merged commit 6357dbb into storybookjs:master Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants