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-knobs: Fix select options types to allow string[] and null #7356

Merged

Conversation

emilio-martinez
Copy link
Contributor

Issue: #7348

What I did

Fixed select(...) knob types to allow for:

  • Values of type string, string[] and null.
  • Options passed as string[] in addition to objects.

This better aligns with the Knobs Addons select docs, i.e.:

const options = {
  Red: 'red',
  Blue: 'blue',
  Yellow: 'yellow',
  Rainbow: ['red', 'orange', 'etc'],
  None: null,
};

...

You can also provide options as an array like this: ['red', 'blue', 'yellow']

@vercel
Copy link

vercel bot commented Jul 9, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-emilio-martinez-fix-7348-addon-knobs-select.storybook.now.sh

@@ -3,17 +3,15 @@ import PropTypes from 'prop-types';

import { Form } from '@storybook/components';

type SelectTypeKnobValue = string;
export type SelectTypeKnobValue = string | string[] | null;
Copy link
Member

Choose a reason for hiding this comment

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

Are select() knobs always strings? Is select(1, 2, 3, 4) not possible?

Also, if you allow nulll.. is the intention to pass in a null value as knob, like that select(null, 'string')?
If yes, this could be useful too: select(null, undefined, 'string')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points! You're right, technically numbers should be fine because they'd pass the strict eq. comparison. I had added null because it was in the example, and similarly undefined should work.

Updated. Let me know your thoughts!

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

@shilman shilman changed the title Fix addon-knobs to allow string[] and null as select() options Addon-knobs: Fix types to allow string[] and null as select() options Jul 10, 2019
@shilman shilman changed the title Addon-knobs: Fix types to allow string[] and null as select() options Addon-knobs: Fix select options types to allow string[] and null Jul 10, 2019
@shilman shilman added this to the 5.2.0 milestone Jul 10, 2019
@shilman shilman merged commit 5f6ce01 into storybookjs:next Jul 10, 2019
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.

None yet

3 participants