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

Allow forced renders in @storybook/html #6190

Merged
merged 1 commit into from
Mar 25, 2019
Merged

Allow forced renders in @storybook/html #6190

merged 1 commit into from
Mar 25, 2019

Conversation

dwightjack
Copy link
Contributor

Issue: #5017

What I did

This PR adds the ability to setup an html object parameter to control whether force re-renders are allowed or not like this:

// .storybook/config.js
import { configure, addParameters } from '@storybook/html';

addParameters({
  html: {
    preventForcedRender: false, // or true
  }
});

// ...

This addresses a change introduced in #4822. I understand the rationale around it, but right now it is just preventing @storybook/html to re-render stories returning a DOM node, which looks much more like a bug than a feature on normal use cases.

How to test

I added a test story in examples/html-kitchen-sink/stories/addon-knobs.stories.js (Addons > Knobs > DOM) and the relevant configuration to examples/html-kitchen-sink/.storybook/config.js

@shilman shilman added this to the v5.1.0 milestone Mar 20, 2019
@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #6190 into next will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##            next   #6190      +/-   ##
========================================
- Coverage   37.9%   37.9%   -0.01%     
========================================
  Files        645     645              
  Lines       9642    9643       +1     
  Branches     354     354              
========================================
  Hits        3655    3655              
- Misses      5940    5941       +1     
  Partials      47      47
Impacted Files Coverage Δ
app/html/src/client/preview/render.js 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 934da0d...4b9ec19. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Mar 21, 2019

Codecov Report

Merging #6190 into next will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##            next   #6190      +/-   ##
========================================
- Coverage   37.9%   37.9%   -0.01%     
========================================
  Files        645     645              
  Lines       9642    9643       +1     
  Branches     354     354              
========================================
  Hits        3655    3655              
- Misses      5940    5941       +1     
  Partials      47      47
Impacted Files Coverage Δ
app/html/src/client/preview/render.js 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 934da0d...4b9ec19. Read the comment docs.

@ndelangen
Copy link
Member

@Atekon I think this is good to merge, but after releasing this you'll likely have to update your stories.

Technically a breaking change. WDYT, about releasing this in 5.1 though?

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.

3 participants