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 seems backwards #799

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

select knob seems backwards #799

shilman opened this issue Apr 15, 2017 · 13 comments

Comments

@shilman
Copy link
Member

shilman commented Apr 15, 2017

Issue by lifehackett
Tuesday Nov 01, 2016 at 20:18 GMT
Originally opened as storybook-eol/storybook-addon-knobs#71


Every time I use the select knob object syntax I write it incorrectly, but it also is more limiting. IMO it makes more sense that the dropdown would show the keys and on selection pass the value.

Additionally, the current setup restricts what can be passed as the prop value to strings or numbers because object keys are more restricted. i.e. you can't have a function or an object in the key position, but you could in the value.

I realize this would be a breaking change.

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by webOS101
Wednesday Dec 07, 2016 at 20:48 GMT


Add to this that you cannot set null as a key, either. I went through some of our stories and found cases where people expected that they could use 'null': null inside the select. Needless to say, it doesn't work that way.

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

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


YES. Please fix this. It makes 0 sense why it was done this way. It is completely backwards.

@shilman
Copy link
Member Author

shilman commented Apr 15, 2017

Comment by ndbroadbent
Tuesday Feb 21, 2017 at 09:06 GMT


Oh yeah, I was just wondering why my numbers were all getting converted to strings. I was using this:

select('Size', { 4: 4, 6: 6, 9: 9 }, 4)

This definitely feels backwards. The main reason is because when you use this syntax, your keys must be strings, but the values can be any type.

EDIT: Actually, values are serialized in the URL so they will be strings anyway. I think I just need to use parseInt.

@ndelangen
Copy link
Member

Yup solid case here, Anyone wants to contribute here? @ndbroadbent @bluetidepro @webOS101 @lifehackett

Sounds like it won't be too complex to do. Though it's likely a breaking change.

@stale
Copy link

stale bot commented Nov 14, 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 Nov 14, 2017
@SEAPUNK
Copy link

SEAPUNK commented Nov 14, 2017

bump

@stale stale bot removed the inactive label Nov 14, 2017
@ndelangen
Copy link
Member

Added a comment on the associated PR:
#1745

psimyn added a commit that referenced this issue Dec 29, 2017
fixes #799
add selectV2 for backwards compatibility
psimyn added a commit that referenced this issue Dec 29, 2017
fixes #799
add selectV2 for backwards compatibility
@stale
Copy link

stale bot commented Dec 30, 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 30, 2017
@SEAPUNK
Copy link

SEAPUNK commented Dec 31, 2017

bump

@stale stale bot removed the inactive label Dec 31, 2017
@ndelangen
Copy link
Member

Should be merged/solved pretty quick:
#1745 (comment)

@lifehackett
Copy link

Awesome. Glad to be able to make use of this. I think this issue can be closed.

@ndelangen
Copy link
Member

I'll close it when the PR is merged 👍

@Hypnosphi
Copy link
Member

Released as 3.4.0-alpha.7

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