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: Allow text and number to take undefined values #10101

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

davidcsally
Copy link
Contributor

@davidcsally davidcsally commented Mar 10, 2020

This allows developers to pass optional props to text and number knobs
in storybook. A common pattern for this is when a React component has
an optional string or number prop.

Issue:

text() and number() knobs do not allow undefined values, which become many optional props when writing React components with typescript.

What I did

Update the types for each of these props to allow the value to be optional. I first tried changing value to optional inside KnobControlConfig (src/components/types/types.ts), but this threw errors in the Date knob, and I didn't want to dive into that.

How to test

This already works in knobs today when using typescript - if you pass undefined to a text() or number() knob, you will see a typescript warning, however, if you add a //@ts-ignore, you will notice that the knobs functions as expected with no value provided.

Here is a very simple code example:

This component (SimpleText) takes an optional value prop, which can be a string or a number. To represent this in knobs, you can pass undefined as the value (or you may omit it as I have done in the number example). However, value is a required property for both the number and text functions, which will generally throw typescript errors during compilation.

import React from 'react'
import { text, number } from '@storybook/addon-knobs'

interface Value {
  value?: string | number;
}

const SimpleText: React.FC<Value> = ({ value }) => <div>{value}</div>

export const emptyDiv = () => (
  <SimpleText />
)

export const textDiv = () => (
  <SimpleText value={text('Text for div: ', undefined)} />
)

export const numberDiv = () => (
  <SimpleText value={number('Number for div: ')} />
)

export default {
  component: SimpleText,
  title: 'Example|TextDiv'
}

This allows developers to pass optional props to text and number knobs
in storybook. A common pattern for this is when a React component has
an optional string or number prop.
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 added this to the 5.3.x milestone Mar 11, 2020
@davidcsally
Copy link
Contributor Author

Hey @shilman! Just wondering if I should rebase this so it is current with master.

@shilman shilman changed the title Allow text() and number() knobs to take undefined values Addon-knobs: Allow text and number to take undefined values Apr 1, 2020
@shilman shilman merged commit c5bd44f into storybookjs:next Apr 1, 2020
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