The NumericSelect.js demo file is broken when selecting the multi-select checkbox

[elkarouh]

Thanks for using react-select!

If you are reporting an error please include a test case that demonstrates the issue you're reporting!
This is very helpful to maintainers in order to help us see the issue you're seeing.

Here is a Plunker you can fork that has react-select loaded and supports JSX syntax:
https://plnkr.co/edit/HTmtER9AMNcPoWhXV707?p=preview

You may also find the online Babel tool quite helpful if you wish to use ES6/ES7 syntax not yet supported by the browser you are using.

[agirton]

Hi @elkarouh thank you for reporting. I can confirm this is in fact broken. Would you like to submit a PR to fix it? 😄

[elkarouh]

Hi, I am in the learning phase. I don't know how i would fix it. I don't even know what a PR is ! Sorry not being able to help.

…

On Thu, Jun 22, 2017 at 7:01 PM Adam Girton ***@***.***> wrote: Hi @elkarouh <https://github.com/elkarouh> thank you for reporting. I can confirm this is in fact broken. Would you like to submit a PR to fix it? 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1810 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC5mAi3YEsOiNeZAEdsDbM6I-7j9oqNeks5sGp3lgaJpZM4OB_le> .

[agirton]

Oh ok, no problem! Would you like some guidance on where to look to fix it?

[eric-ships]

Hi @agirton I am looking to contribute to open-source and wouldn't minding taking a look.

It seems like I need to fix something in NumericSelect.js.

If you guys don't mind, can I be assigned to this?

[agirton]

Hi @ericliu121187 that would be great! It's possible there is an underlying issue with the combination the example uses: simpleValue, multi select and numbers. I will remove the help wanted label since you would like to take this on.

[eric-ships]

Yup! The following seemed to fix it.

// NumericSelect.js, line 42
onChange(value) {
  this.setState({
    value: this.state.multi ? value.split(',').map(Number) : value
  });
console.log('Numeric Select value changed to', value);
},

Concerns:

1. I don't have permission to push to origin. How should I proceed?
2. Is there a particular branch naming convention and PR template that you would prefer?
3. It seems like there is no test for this file, right? Should I make one?
4. Also, example/dist/app.js keeps getting updated as well. Should that folder be included in the .gitignore?
5. The build does not work with node v.8. I'm not exactly sure what is the most recent non-breaking version the build supports, but it might be worthwhile to mention in the README.md. (I ended using v4.2.1.)

[audiolion]

@ericliu121187 see https://github.com/JedWatson/react-select/blob/master/CONTRIBUTING.md

[agirton]

Hi @ericliu121187 if the NumericSelect has this issue. It's very possible that there is an underlying issue.

It looks there is a PR with a fix for this #1600. Also found a duplicate issue #1476. So going to close out this in favor of that ticket.

My apologies for not finding this sooner and wasting your time 😞 .

[eric-ships]

@agirton There doesn't seem to be any underlying issue. It's just that simpleValue returns a string and select expects an array when multiValue is selected.

No problem! Is there another issue I can help with?

@audiolion Ah, thanks!

[agirton]

@ericliu121187 we have quite a long list of issues 😄 . At this time nothing stands out to me as good intro tickets. But feel free to go through the list and see if there are any issues that might be easy to take on that doesn't already have a PR open.