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

select knob for optional prop #805

Closed
shilman opened this issue Apr 15, 2017 · 7 comments
Closed

select knob for optional prop #805

shilman opened this issue Apr 15, 2017 · 7 comments

Comments

@shilman
Copy link
Member

shilman commented Apr 15, 2017

Issue by alex-bezek
Thursday Feb 02, 2017 at 02:20 GMT
Originally opened as storybook-eol/storybook-addon-knobs#85


I am trying to implement a story book for a react component with a number of optional props that PropTypes.oneOf(['option1', 'option2', 'option3'])

I would like to be able to configure this knob so by default it doesn't pass in the prop. Is this possible and I'm missing an explanation?

Thanks!

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by bluetidepro
Wednesday Feb 08, 2017 at 18:48 GMT


I don't know if there is a more graceful way to do this @alex-bezek, but the way I have done this in the past is by doing:

{select('BLAH Prop', {
	Value1: 'Name for Value 1 in Select',
	Value2: 'Name for Value 2 in Select',
	'': 'Null',
}, '')}

And then that way by default it passes '' (you could make this null if needed) and that's basically the same as passing no prop.

Hope that helps some.

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by Djspaceg
Tuesday Feb 28, 2017 at 01:52 GMT


An alternative that I do occasionally is this:

    propName={select('propName', [null, 'option1', 'option2', 'option3'])}

Knobs interprets this null as {'': 'null'} so your component gets handed a propName with an empty string as its value.

Your propTypes can then look like this if you want to make Storybook Knobs happy:

    PropTypes.oneOf(['option1', 'option2', 'option3', ''])

It would be great if knobs could interpret having a literal null, supplied via an Array, as a literal null for a prop, by either excluding the prop altogether from the component or sending null as the value. Obviously, this isn't as straightforward if you need to supply a hash to the knob since null can't be a key, but it would still be nice for an Array.

@matteocng
Copy link

matteocng commented Jun 19, 2017

I've tried to find a way to make prop-types errors go away not just when the initial value of a PropTypes.oneOf knob is undefined (that is easily achievable), but also when the user selects a value and then selects the empty/placeholder <option></option> again. Currently, as noted before, false prop values (SEE: onChange) are passed as an empty string to the React component. Example:

stories/Button.js

Button.propTypes = {
  fooString: PropTypes.oneOf([
    'one',
    'two',
    'three',
  ]),
  fooNumber: PropTypes.oneOf([
    0,
    30,
    60,
    90,
  ]),
}

stories/index.js

const fooStringValues = {
  'null': 'Select a fooString',
  one: 'one',
  two: 'two',
  three: 'three',
}

or like this:

const fooStringValues = [null, 'one', 'two', 'three']
<Button fooString={select('fooString', fooStringValues) } />

works perfectly when storybook is loaded, but selecting a value and then the empty value again ... yields a prop-types error:

Warning: Failed prop type: Invalid prop fooString of value '' supplied to Button, expected one of ["one","two","three"].

Warning: Failed prop type: Invalid prop fooString of value 'null' supplied to Button, expected one of ["one","two","three"].

a. Let the user take care of this.

In this case we could add some examples to the documentation.

  • Option 1: Change the PropType to allow an empty string and handle it in the component:
Button.propTypes = {
  fooString: PropTypes.oneOf([
    '',
    'one',
    'two',
    'three',
   ])
}

Drawbacks: this doesn't sound optimal at all, unless the '' is actually handled by the component and not just ignored.

  • Option 2: detect a false value returned by select and replace it with undefined for every prop of your component that doesn't accept ``:
<Button fooString={select('fooString', fooStringValues) || undefined } />

Drawbacks: this requires changing code in all of one's stories.

Note: this workaround can also be used to correctly pass "number" props, that otherwise are passed as strings and trigger prop-types errors (unless there is some other way to do this):

const selectNumber = (key, values, defaultValue) => {
  const ret = select(key, values, defaultValue)
  return (ret !== '0' && !ret) ? undefined : parseInt(ret, 10)
}
...
<Button fooNumber={selectNumber('fooNumber', fooNumberValues)} />

b. Modify @storybook/addon-knobs.

diff --git a/addons/knobs/src/components/types/Select.js b/addons/knobs/src/components/types/Select.js
index a0a5366a..35bc7c02 100644
--- a/addons/knobs/src/components/types/Select.js
+++ b/addons/knobs/src/components/types/Select.js
@@ -47,7 +47,7 @@ class SelectType extends React.Component {
         }}
         style={styles}
         value={knob.value}
-        onChange={e => onChange(e.target.value)}
+        // some logic here
       >
         {this._options(knob.options)}
       </select>

We would have to add some some logic in @storybook/addon-knobs (probably in the Select.js component) to interpret a null (array) and a literal null (hash) as "send undefined to onChange" (= the prop wouldn't be passed to the React component), as @Djspaceg suggested.

Any thoughts?

@ndelangen
Copy link
Member

Related: #799

I'm open to option b actually, the easier we can make the usage of knobs, the better.

Are you open to making a PR on this @matteocng ?

@stale
Copy link

stale bot commented Dec 19, 2017

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks!

@stale stale bot added the inactive label Dec 19, 2017
@stale
Copy link

stale bot commented Jan 3, 2018

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

@oldwin
Copy link

oldwin commented Mar 22, 2018

Hi there,
I have workaround. I just use next code to pass 'undefined' (optional value) to component from Storybook.

Just use next code snippet in your stories

<Component
  myProp={select('myProp', [null, 'value1', 'value2']) || undefined}
/>

It will pass undefined to Component without any prop types warnings or errors.

Component.propTypes = {
   myProp: PropTypes.oneOf(['value1', 'value2']),
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants