Add parameters to stories in the story store, and render them in app layers #2679

tmeasday · 2018-01-08T05:53:38Z

Issue: #993

What I did

Added support for .add(name, fn, { parameters }), and similarly passing the options parameters as part of the story context.

Notes

~~This PR is incomplete, I have another one with some tests for the storystore that I will augment this with. I am interested in feedback on the feature and the approach.~~

Note that the various app layers no longer call .getStory() (now deprecated I think) and instead call the wrapped .getStoryWithContext() that handles the (generic) task of setting a context for the story. I think I caught all the places where we do this but let me know if not. ~~Also need to test properly for all 4 app layers.~~

TODO

Add tests
Add API to react-native
Add documentation
Update addons to make use of it? [separate PR?]

tmeasday · 2018-01-08T05:54:28Z

examples/official-storybook/stories/core.stories.js

+    </div>
+  ),
+  { myOption: 'value' }
+);


See this example for insight into how this feature works. More importantly, decorators and addons get passed the context also.

joscha · 2018-01-08T05:59:27Z

This is quite cool, it would remove the need for the special abstraction around stories in percy-storybook as well. We currently introduce our own story type, add the options to a separate map and then extract them later again. See https://github.com/percy/percy-storybook/blob/3609b6f013e5a6f94a9b294a008b30bad425b9bc/packages/percy-storybook/src/index.js#L43-L62

cc @timhaines

tmeasday · 2018-01-08T05:59:33Z

app/angular/src/client/preview/angular/helpers.ts

@@ -3,9 +3,9 @@ import {
  enableProdMode,
  NgModule,
  Component,
-  NgModuleRef
+  NgModuleRef,


Not quite sure why prettier is reformatting this file. Any insights @igor-dv?

tmeasday · 2018-01-08T06:00:44Z

Yes! Keen to get insight from such use cases.

shilman · 2018-01-08T06:12:56Z

@alterx @tmeasday @ndelangen @igor-dv Any idea why prettier was not applied to the original app/angular source files? Inconvenient to review the changes when a lot of the changes are formatting changes. If there's something wrong with our tooling we should fix it? I was under the impression that we run prettier automatically on precommits.

igor-dv · 2018-01-08T06:26:08Z

Looks like tslint is not configured to run on precommit at all. And eslint handles only js files. Maybe it's your IDE refactored it ? Will take a look on it soon.

Hypnosphi · 2018-01-08T13:58:48Z

After resolving the conflicts, the noisy diff should be gone

tmeasday · 2018-01-09T07:15:56Z

Ok, angular guff sorted.

igor-dv · 2018-01-09T15:26:25Z

Looks legit. What if we have a few decorators, that need different options? Will it be like this :

add('..', 
 () => ..., 
 {
  notes: {},
  info: {},
 ...
})

By convention ?

tmeasday · 2018-01-12T01:53:21Z

@igor-dv I suspect it might but I think that is the next question to answer after merging this one. There are some other related things, like special options:

.add(..., () => ..., { decorators: ... });

codecov · 2018-02-09T00:40:40Z

Codecov Report

Merging #2679 into master will increase coverage by 0.07%.
The diff coverage is 52.38%.

@@            Coverage Diff            @@
##           master   #2679      +/-   ##
=========================================
+ Coverage   35.83%   35.9%   +0.07%     
=========================================
  Files         440     440              
  Lines        9636    9669      +33     
  Branches      907     905       -2     
=========================================
+ Hits         3453    3472      +19     
- Misses       5634    5643       +9     
- Partials      549     554       +5

Impacted Files	Coverage Δ
app/angular/src/client/preview/render.js	`0% <ø> (ø)`	⬆️
app/vue/src/client/preview/render.js	`0% <ø> (ø)`	⬆️
app/react/src/client/preview/render.js	`0% <ø> (ø)`	⬆️
app/polymer/src/client/preview/render.js	`0% <ø> (ø)`	⬆️
app/react-native/src/preview/index.js	`0% <0%> (ø)`	⬆️
app/react-native/src/index.js	`0% <0%> (ø)`	⬆️
app/angular/src/client/preview/index.js	`0% <0%> (ø)`	⬆️
app/react/src/client/preview/index.js	`0% <0%> (ø)`	⬆️
...t-native/src/preview/components/StoryView/index.js	`0% <0%> (ø)`	⬆️
app/polymer/src/client/index.js	`0% <0%> (ø)`	⬆️
... and 74 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 c8373ff...5a81b74. Read the comment docs.

tmeasday · 2018-02-09T00:46:50Z

This is now ready to merge, barring any docs changes we want to make.

I don't think we currently document what the context looks like for a story---perhaps we should, but I suspect it's mainly addon authors that care.

I think maybe if we are OK with merging this as is, we could follow it up with an PR that uses the options instead of the more awkward foo({options})(story) HoC style API in our built-in addons and then document the technique that we settle on in the addon documentation.

Hypnosphi · 2018-02-09T08:37:46Z

Just to be clear: those are not the same options which are set via addon-options?

tmeasday · 2018-02-11T10:15:37Z

Just to be clear: those are not the same options which are set via addon-options?

No, but this is a good point, we should have a way to set options per-app and per-chapter.

tmeasday · 2018-02-11T10:54:05Z

Let me know what you think about the global and chapter addOptions API.

I also remembered that react-native isn't yet using the @storybook/core module, so I'll either need to reimplement the API there or change it to do so before we merge this.

Hypnosphi · 2018-02-17T15:52:16Z

I'll either need to reimplement the API there or change it to do so before we merge this

The latter sounds better to me =)

tmeasday · 2018-02-18T11:01:03Z

Yep, on it (slowly)

…

On Sun, 18 Feb 2018 at 2:52 am, Filipp Riabchun ***@***.***> wrote: I'll either need to reimplement the API there or change it to do so before we merge this The latter sounds better to me =) — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#2679 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIFylYA14aOs1j175RSFEt2X0gBaw0Vks5tVvW4gaJpZM4RV9_3> .

tmeasday · 2018-03-01T04:23:57Z

Ok, renamed to parameters. I think this is ready to merge.

tmeasday · 2018-03-01T04:48:48Z

Well except for the Angular snapshot changes, which are a bit baffling. Should I just accept them @igor-dv?

igor-dv · 2018-03-01T06:52:52Z

Yeah. I don't know yet how to get rid of those ng-💩 things...

tmeasday · 2018-03-20T23:19:49Z

🎉 going to start working on the next phase of my master plan now ;)

tmeasday added the in progress label Jan 8, 2018

tmeasday commented Jan 8, 2018

View reviewed changes

tmeasday requested a review from a team January 8, 2018 05:55

tmeasday commented Jan 8, 2018

View reviewed changes

igor-dv mentioned this pull request Jan 8, 2018

Add tslint to the linting pipe #2682

Merged

tmeasday force-pushed the tmeasday/add-options-to-story-store branch 2 times, most recently from 4ee31ea to bcbc87a Compare January 9, 2018 07:15

tmeasday mentioned this pull request Jan 18, 2018

Support adding decorators to groups of stories by path prefix #2769

Closed

tmeasday mentioned this pull request Feb 1, 2018

Allow passing options to storiesOf().add() as hook to customizing story behaviour in plugins #993

Closed

tmeasday force-pushed the tmeasday/add-options-to-story-store branch from 9d9aaab to 8fc09c7 Compare February 9, 2018 00:40

tmeasday removed the in progress label Feb 9, 2018

tmeasday self-assigned this Feb 11, 2018

tmeasday added the in progress label Feb 11, 2018

tmeasday mentioned this pull request Feb 20, 2018

Use client api in react native #3036

Merged

tmeasday force-pushed the tmeasday/add-options-to-story-store branch from 8942d79 to 9795fa6 Compare February 20, 2018 09:52

tmeasday force-pushed the tmeasday/add-options-to-story-store branch from 1ad8532 to 6c68af0 Compare March 1, 2018 04:23

Hypnosphi added ready and removed in progress labels Mar 1, 2018

Hypnosphi mentioned this pull request Mar 4, 2018

fix screenshots tests & add getScreenshotOption to storyshots #3102

Merged

tmeasday added 2 commits March 6, 2018 12:28

update snapshots

cc0b67f

Remove references to filesystem paths from stories

f654b4b

tmeasday force-pushed the tmeasday/add-options-to-story-store branch from 97b5af3 to f654b4b Compare March 6, 2018 03:40

Hypnosphi added this to the v4.0.0 milestone Mar 8, 2018

Hypnosphi approved these changes Mar 20, 2018

View reviewed changes

Merge branch 'master' into tmeasday/add-options-to-story-store

5a81b74

Hypnosphi merged commit 9340dde into master Mar 20, 2018

Hypnosphi deleted the tmeasday/add-options-to-story-store branch March 20, 2018 12:20

dependencies bot mentioned this pull request Apr 17, 2018

redux versions available: 4.0.0 #3436

Closed

BPScott mentioned this pull request Dec 18, 2018

Use storybook parameters instead of maintaining a separate store percy/percy-storybook#50

Closed

This was referenced Feb 11, 2019

Switch to using Storybook's new story parameters (Storybook v5) percy/percy-storybook#64

Merged

Update getStorybook to return story parameters #5549

Closed

lili2311 mentioned this pull request Sep 23, 2023

[Snyk-dev] Security upgrade riot from 3.13.2 to 4.0.0 lili2311/storybook#166

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parameters to stories in the story store, and render them in app layers #2679

Add parameters to stories in the story store, and render them in app layers #2679

tmeasday commented Jan 8, 2018 •

edited

Loading

tmeasday Jan 8, 2018

joscha commented Jan 8, 2018 •

edited

Loading

tmeasday Jan 8, 2018

tmeasday commented Jan 8, 2018

shilman commented Jan 8, 2018 •

edited

Loading

igor-dv commented Jan 8, 2018

Hypnosphi commented Jan 8, 2018

tmeasday commented Jan 9, 2018

igor-dv commented Jan 9, 2018

tmeasday commented Jan 12, 2018

codecov bot commented Feb 9, 2018 •

edited

Loading

tmeasday commented Feb 9, 2018

Hypnosphi commented Feb 9, 2018

tmeasday commented Feb 11, 2018

tmeasday commented Feb 11, 2018

Hypnosphi commented Feb 17, 2018

tmeasday commented Feb 18, 2018 via email

tmeasday commented Mar 1, 2018

tmeasday commented Mar 1, 2018

igor-dv commented Mar 1, 2018

tmeasday commented Mar 20, 2018

Add parameters to stories in the story store, and render them in app layers #2679

Add parameters to stories in the story store, and render them in app layers #2679

Conversation

tmeasday commented Jan 8, 2018 • edited Loading

What I did

Notes

TODO

tmeasday Jan 8, 2018

Choose a reason for hiding this comment

joscha commented Jan 8, 2018 • edited Loading

tmeasday Jan 8, 2018

Choose a reason for hiding this comment

tmeasday commented Jan 8, 2018

shilman commented Jan 8, 2018 • edited Loading

igor-dv commented Jan 8, 2018

Hypnosphi commented Jan 8, 2018

tmeasday commented Jan 9, 2018

igor-dv commented Jan 9, 2018

tmeasday commented Jan 12, 2018

codecov bot commented Feb 9, 2018 • edited Loading

Codecov Report

tmeasday commented Feb 9, 2018

Hypnosphi commented Feb 9, 2018

tmeasday commented Feb 11, 2018

tmeasday commented Feb 11, 2018

Hypnosphi commented Feb 17, 2018

tmeasday commented Feb 18, 2018 via email

tmeasday commented Mar 1, 2018

tmeasday commented Mar 1, 2018

igor-dv commented Mar 1, 2018

tmeasday commented Mar 20, 2018

tmeasday commented Jan 8, 2018 •

edited

Loading

joscha commented Jan 8, 2018 •

edited

Loading

shilman commented Jan 8, 2018 •

edited

Loading

codecov bot commented Feb 9, 2018 •

edited

Loading