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

Attempt to make propTablesExclude usage clearer #2568

Merged
merged 4 commits into from
Dec 27, 2017

Conversation

mattferderer
Copy link
Contributor

See issue #2266, specifically #2266 (comment) for discussion.

Issue: #2266

What I did

Added text to documentation make propTablesExclude clearer

How to test

No changes to code made.

Is this testable with jest or storyshots?
No

Does this need a new example in the kitchen sink apps?
No but would maybe be a good idea.

Does this need an update to the documentation?
This is an update.

If your answer is yes to any of these, please make sure to include it in your PR.

@codecov
Copy link

codecov bot commented Dec 26, 2017

Codecov Report

Merging #2568 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2568   +/-   ##
=======================================
  Coverage   32.68%   32.68%           
=======================================
  Files         398      398           
  Lines        8838     8838           
  Branches      943      947    +4     
=======================================
  Hits         2889     2889           
- Misses       5293     5295    +2     
+ Partials      656      654    -2
Impacted Files Coverage Δ
app/vue/src/server/config/babel.js 0% <0%> (-100%) ⬇️
app/react/src/server/utils.js 0% <0%> (-53.58%) ⬇️
app/react-native/src/bin/storybook.js 0% <0%> (ø) ⬆️
.../src/modules/ui/components/stories_panel/header.js 29.62% <0%> (ø) ⬆️
addons/actions/src/lib/retrocycle.js 54.54% <0%> (ø) ⬆️
addons/info/src/components/types/PropertyLabel.js 78.94% <0%> (ø) ⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js 22.41% <0%> (ø) ⬆️
addons/knobs/src/angular/helpers.js 0% <0%> (ø) ⬆️
.../ui/src/modules/ui/components/layout/dimensions.js 15.62% <0%> (ø) ⬆️
addons/info/src/components/PropVal.js 82.82% <0%> (ø) ⬆️
... and 56 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 b289829...9d92f81. Read the comment docs.

@@ -130,7 +130,7 @@ setDefaults({
inline: true, // Displays info inline vs click button to view
source: true, // Displays the source of story Component
propTables: [/* Components used in story */], // displays Prop Tables with this components
propTablesExclude: [], // Exclude Components from being shown in Prop Tables section
propTablesExclude: [], // Exclude Components from being shown in Prop Tables section. Pass the imported component names as values to the array.
Copy link
Member

Choose a reason for hiding this comment

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

Component name is a string, but here the component itself (class or stateless function) should be passed

@mattferderer
Copy link
Contributor Author

Would it make more sense to say "Pass the imported component class or function to the array."

@Hypnosphi
Copy link
Member

Hypnosphi commented Dec 27, 2017

"Accepts an array of component classes or functions".

The component may be defined in the same file, so mentioning imports may be confusing.

Changed to agreed upon instructions for propTablesExclude to provide clearer documentation.
@Hypnosphi Hypnosphi merged commit 7781c54 into storybookjs:master Dec 27, 2017
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