-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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-knobs: : add number config options #2371
Conversation
Fix inconsistency in number API fix step,min,max attributes in number type
Codecov Report
@@ Coverage Diff @@
## master #2371 +/- ##
==========================================
- Coverage 19.92% 19.92% -0.01%
==========================================
Files 293 293
Lines 6484 6485 +1
Branches 752 755 +3
==========================================
Hits 1292 1292
- Misses 4597 4607 +10
+ Partials 595 586 -9
Continue to review full report at Codecov.
|
That's great, thanks! This fixes #1632
Actually, I think it does. Please add one here: https://github.com/storybooks/storybook/blob/master/examples/cra-kitchen-sink/src/stories/addon-knobs.stories.js You may need to run |
Thank you for this PR @lifeiscontent ! @Hypnosphi Looks like the min,max,step was already in the example? |
@ndelangen we need the same but without |
@Hypnosphi how do I run the kitchen sink example? I tried the following.
but I get issues saying none of the storybook modules exist. |
@Hypnosphi updated! |
@@ -47,7 +47,7 @@ class AsyncItemLoader extends React.Component { | |||
|
|||
storiesOf('Addon Knobs.withKnobs', module) | |||
.addDecorator(withKnobs) | |||
.add('tweaks static values', () => { | |||
.add('tweaks static values via range', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example includes a lot of other knobs, not only the range input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s add min/max/step to dollars
below instead of duplicating the example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run yarn jest -u
after that
@@ -47,7 +47,7 @@ class AsyncItemLoader extends React.Component { | |||
|
|||
storiesOf('Addon Knobs.withKnobs', module) | |||
.addDecorator(withKnobs) | |||
.add('tweaks static values', () => { | |||
.add('tweaks static values via range', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s add min/max/step to dollars
below instead of duplicating the example
@Hypnosphi updated. I ran jest, but it seems there are tests failing that are unrelated. |
Ok looks like it passes as is. I'll be to test the example manually in a few hours |
Fix inconsistency in number API
fix step,min,max attributes in number type
Issue:
What I did
Fixed inconsistency in number type.
How to test
when setting the options of a number, the step,min,max attributes now get applied.
Is this testable with jest or storyshots? Yes, but I didn't write a test.
Does this need a new example in the kitchen sink apps? No.
Does this need an update to the documentation? Not really, but I could add an example.
If your answer is yes to any of these, please make sure to include it in your PR.