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

Addon Info: Omit empty inline info header #1306

Merged

Conversation

duncanbeevers
Copy link
Contributor

Issue: Creating stories using addWithInfo, configured not to show headers, and to render info inline displays a blank header container on each story.

What I did

Instead of unconditionally rendering the header container and filling it with the (possibly null) result of _getInfoHeader(), I check the result first, and render the container conditionally.

How to test

Check the 'simple usage (no header)' example. Before this change is applied, it appears with a large empty white area above the example.
screen shot 2017-06-17 at 2 35 02 pm

After this change is applied, the empty white area is absent.
screen shot 2017-06-17 at 2 32 22 pm

@duncanbeevers duncanbeevers force-pushed the addon-info-omit-empty-inline-header branch from e4649ac to b62b685 Compare June 17, 2017 21:36
@codecov
Copy link

codecov bot commented Jun 17, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@95ec300). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1306   +/-   ##
=========================================
  Coverage          ?   13.75%           
=========================================
  Files             ?      202           
  Lines             ?     4623           
  Branches          ?      510           
=========================================
  Hits              ?      636           
  Misses            ?     3547           
  Partials          ?      440
Impacted Files Coverage Δ
addons/info/src/components/Story.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 95ec300...e32629f. Read the comment docs.

@duncanbeevers duncanbeevers force-pushed the addon-info-omit-empty-inline-header branch 2 times, most recently from 04385e2 to d80e1c5 Compare June 19, 2017 14:14
@duncanbeevers duncanbeevers force-pushed the addon-info-omit-empty-inline-header branch from d80e1c5 to 549e55e Compare June 22, 2017 17:55
@duncanbeevers
Copy link
Contributor Author

Looks like this is a duplicate of https://github.com/storybooks/storybook/pull/1297/files

Feel free to take whichever approach seems appropriate.

@shilman
Copy link
Member

shilman commented Jun 23, 2017

@duncanbeevers this one is a little cleaner, so I'm going to merge it. thanks for the PR!

@shilman
Copy link
Member

shilman commented Jun 23, 2017

@duncanbeevers NOTE: we are planning to move the info addon into a panel soon: #1147

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!

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