Skip to content
This repository has been archived by the owner on Nov 10, 2017. It is now read-only.

Inverted key/value for select options #87

Closed
wants to merge 1 commit into from

Conversation

ndbroadbent
Copy link

In response to #71

@mastilver
Copy link

Why not fixing the issue rather that inverting the docs? It's obviously a bug

Also it doesn't fix the issue for Arrays:

select('test', [
  'item1',
  'item2',
  'item3'
], 'item1');

When the expected is:

select('test', [
  'item1',
  'item2',
  'item3'
], 0);

@@ -27,7 +27,7 @@ stories.add('with all knobs', () => {

const bold = boolean('Bold', false);
const selectedColor = color('Color', 'black');
const favoriteNumber = number('Favorite Number', 42);
const favoriteNumber = select('Favorite Number', { 'Seven': 7, 'Forty-two': 42, 'Nine thousand': 9000 }, 42);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to add a new line, keeping the simple "number" example.

@mthuret
Copy link
Contributor

mthuret commented Apr 8, 2017

@mastilver Can you explain what you've expected? It seems that this doesn't just invert the doc.

I think this syntax is more natural. But this is a breaking change. I think it's ok to go for a major version but WDYT @mnmtanish and @apexskier?

@mastilver
Copy link

mastilver commented Apr 8, 2017

To me the docs sounds right, That's the way I would want it to work. Especially when you take array in consideration

Cc @lifehackett

@apexskier
Copy link

I actually very much like this. When I first used knobs, the original syntax was pretty surprising, but this definitely requires a major version bump.

@ndelangen
Copy link
Contributor

Hey @ndbroadbent

Thank you so much for this PR! We're moving over to a mono-repo, since that makes a lot of sense for a project like this.
We would ❤️ it if you could create a PR here: https://github.com/storybooks/storybook

That'd be awesome, thanks!

@ndelangen ndelangen closed this Apr 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants