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

Document how to use info addon as decorator #1592

Merged
merged 5 commits into from Aug 5, 2017

Conversation

4 participants
@slorber
Contributor

slorber commented Aug 4, 2017

I want infos by default on all my components and it's not very important for me to add a custom text line per story

It is possible to add infos by default to all components by using a global or story decorator. The drawback is you won't be able to display a distinct info message per story.
```
addDecorator(withInfo()(story => story()));

This comment has been minimized.

@denkristoffer

denkristoffer Aug 4, 2017

Wouldn't the info addon show information on the WrapStory component instead of the actual story component with this?

This comment has been minimized.

@UsulPro

UsulPro Aug 4, 2017

Member

@denkristoffer it could be only if you add other decorators. In this case, it's important to keep the right decorators order.

@UsulPro

UsulPro approved these changes Aug 4, 2017

Thanks! This is really an important clarification that now it is possible to use this addon in decorators as well!

@codecov

This comment has been minimized.

codecov bot commented Aug 4, 2017

Codecov Report

Merging #1592 into master will decrease coverage by 0.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1592      +/-   ##
==========================================
- Coverage   21.05%   20.41%   -0.64%     
==========================================
  Files         244      241       -3     
  Lines        5310     5255      -55     
  Branches      658      638      -20     
==========================================
- Hits         1118     1073      -45     
- Misses       3692     3716      +24     
+ Partials      500      466      -34
Impacted Files Coverage Δ
lib/ui/src/modules/ui/components/search_box.js 16.66% <0%> (-67.71%) ⬇️
lib/ui/src/modules/ui/containers/search_box.js 33.33% <0%> (-66.67%) ⬇️
lib/ui/src/compose.js 20% <0%> (-13.34%) ⬇️
...ponents/left_panel/stories_tree/tree_decorators.js 29.57% <0%> (-3.76%) ⬇️
lib/ui/src/modules/ui/containers/left_panel.js 25% <0%> (-0.72%) ⬇️
addons/knobs/src/components/types/Number.js 8.06% <0%> (ø) ⬆️
...les/ui/components/left_panel/stories_tree/index.js 100% <0%> (ø) ⬆️
...b/ui/src/modules/ui/components/left_panel/index.js 100% <0%> (ø) ⬆️
.../ui/src/modules/ui/components/left_panel/header.js 28.57% <0%> (ø) ⬆️
addons/info/src/components/PropVal.js 43.15% <0%> (ø) ⬆️
... and 16 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 e01fd7a...523fae4. Read the comment docs.

@slorber

This comment has been minimized.

Contributor

slorber commented Aug 4, 2017

@UsulPro wait I think @denkristoffer is right and there's something not ok with my code even if it almost works

@denkristoffer

This comment has been minimized.

denkristoffer commented Aug 4, 2017

@slorber I just tested it, @UsulPro seems to be right. It should probably be mentioned in the text, so no one gets confused.

This works:

addDecorator(withInfo()(story => story()))
addDecorator(withKnobs)

This shows the WrapStory component:

addDecorator(withKnobs)
addDecorator(withInfo()(story => story()))
@slorber

This comment has been minimized.

Contributor

slorber commented Aug 4, 2017

@denkristoffer wouldn't it work better with this syntax?

addDecorator(story => withInfo()(() => story()))

But in all cases currently when displaying more infos on my component, the "more info" overlay does not include the component. I think I'm missing something but can't understand how to display the component on the more info overlay

@slorber

This comment has been minimized.

Contributor

slorber commented Aug 4, 2017

Honnestly I'm not sure to know exactly what I'm doing.

I've mentionned to declare this decorator first as you asked but don't really understand why so I'll let you merge or not ;)

@UsulPro

This comment has been minimized.

Member

UsulPro commented Aug 4, 2017

@slorber

I guess in general it should be:

addDecorator((story, context) => withInfo('common info')(story)(context));

(because addon Info needs context to show the title)

the "more info" overlay does not include the component

Do you mean that it shows withKnobs decorator or something like this?

(it could looks like this:

<WrapStory
  context={kind: "Button", story: "with text"}
  storyFn=anonymous()
  channel={_sender: "21d22b66d0135", _transport: {_config: {page: "preview"}, _buffer: [], _handler: bound _handleEvent()}, _listeners: {setCurrentStory: [anonymous()], addon:knobs:knobChange: [bound knobChanged()], addon:knobs:reset: [bound resetKnobs()]}}
  knobStore={store: {}, callbacks: [bound setPaneKnobs()]}
  initialContent=<Button />
/>

)

@shilman

the example is @UsulPro 's comment is correct.

@tmeasday and I talked about updating the API in 3.4 so you can just do:

addDecorator(withInfo('some common docs'))

but for now the adapter is the right way to go.

@slorber

This comment has been minimized.

Contributor

slorber commented Aug 4, 2017

I've updated. Is it still required to use it as first decorator now?

@UsulPro

This comment has been minimized.

Member

UsulPro commented Aug 4, 2017

I would say that the order of decorators matters! addon-info will show information about the child component provided into it and it doesn't know whether it's your component or another decorator. We have hot discussions about how to improve this behavior but at this moment it is how Storybook decorators work.

So if you need to show info about your component it should be the *deepest` decorator and it's important to take into account that "local" decorators are deeper than "global"

@shilman

shilman approved these changes Aug 5, 2017

@UsulPro UsulPro merged commit 38ff4cb into storybooks:master Aug 5, 2017

11 of 12 checks passed

ci/circleci: build_accept_deploy/deploy Your job is on hold on CircleCI!
Details
CodeFactor No issues found.
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: example-kitchen-sink Your tests passed on CircleCI!
Details
ci/circleci: example-react-native Your tests passed on CircleCI!
Details
ci/circleci: example-test-cra Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: unit-test Your tests passed on CircleCI!
Details
ci/circleci: validate Your tests passed on CircleCI!
Details
codebeat no reportable quality changes
Details
security/snyk No new vulnerabilities
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment