Knobs addon: new knob type button
#2004
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2004 +/- ##
=========================================
+ Coverage 21.34% 21.4% +0.05%
=========================================
Files 262 263 +1
Lines 5767 5793 +26
Branches 701 694 -7
=========================================
+ Hits 1231 1240 +9
- Misses 4002 4015 +13
- Partials 534 538 +4
Continue to review full report at Codecov.
|
|
Code LGTM. Still not sure this thing belongs to addon-knobs semantically |
|
Ok, who decides how to move forward then? @danielduan? I can see where you're coming from, because it's functionally a bit different than inputs—but semantically it makes sense to me as a way to control or trigger behaviour in a story. |
|
@ndelangen @shilman thoughts? I definitely see why there's a need for this, but maybe this isn't a "knob"? perhaps it should be its own addon? |
|
|
||
| const label = 'Do Something'; | ||
| const handler = () => doSomething('foobar'); | ||
| button(label, handler); |
There was a problem hiding this comment.
The way this is used, is quite different from how the other knobs are used. This confuses me, and I'd assume other as well. Having a fully 'working' real-world example would help a lot understanding this.
There was a problem hiding this comment.
Looking at the documentation as of today, I don't get nothin' 😆. How is this supposed to be used ? 🤔
| @@ -110,6 +111,8 @@ storiesOf('Button', module) | |||
| const salutation = nice ? 'Nice to meet you!' : 'Leave me alone!'; | |||
| const dateOptions = { year: 'numeric', month: 'long', day: 'numeric' }; | |||
|
|
|||
| button('Arbitrary action', action('You clicked it!')); | |||
There was a problem hiding this comment.
This example is rather meaningless right? calling action from a button inside the knobs panel, won't be useful, or am I missing something here?
Could you add an example that would help understand the true use-case of this change? 👍
There was a problem hiding this comment.
Totally reasonable
There was a problem hiding this comment.
So I made an example in the CRA for a case where i've normally included a button inside the story itself to trigger something like loading some external data through a render callback component, or generating a set of random data. These are the kind of use cases I'd have in mind for something like this.
I haven't thought too hard about it, there could be better ways to do it.
If you think that's not a good example, or not within knobs paradigm—totally fine. Don't have my heart set on it :)
| } | ||
|
|
||
| render() { | ||
| button('Load the items', () => this.loadItems()); |
There was a problem hiding this comment.
Shouldn't this be in the constructor?
There was a problem hiding this comment.
Very clear example by the way, Thanks!
There was a problem hiding this comment.
HMR kills it when it is in the constructor. The button stays present in the knob panel but the instance of the example component is new, so the actual component tied to the knob was unmounted. I think there'd need to be modifications to the knobManager to override existing knobs of this type, instead of just returning the existing instance
There was a problem hiding this comment.
Ok, if this is the best solution, that's fine.
Can you take a look at why the CI is disapproving of this PR?
|
Great job @derrickpelletier ! Thank you! |
Issue: #1914
What I did
buttonexample tocra-kitchen-sinkandvue-kitchen-sinkwith-knobs examplesbuttonusage toaddons/knobs/README.mdHow to test
Is this testable with jest or storyshots?
Does this need a new example in the kitchen sink apps?
Does this need an update to the documentation?