JedWatson / react-select

The Select Component for React.js
https://react-select.com/
MIT License
27.57k stars 4.12k forks source link

onChange returns the selected value, not the complete event #1631

Closed OsamaShabrez closed 4 years ago

OsamaShabrez commented 7 years ago

onChange simply returns the new value and label and not the whole event.

I understand we can use the hidden input field to fire an onChange but this does not happen automatically and while we can manually fire an onChange for hidden input field, we can also wrap an event around the Select. Here is how I am doing it following #520:

<Select
  name="form-field-name"
  value={val}
  options={options}
  onChange={(val)=> {onChangeFunc({target: { name:'revision_id', value: val.value }})}}
/>

Creating an object myself and passing it to my onChange handler. Is this the right way to do it or you can suggest a better way around it?

jwarykowski commented 7 years ago

👍 I've found that can cause issues with redux-form's field component, it would be great to get the original event passed as the second argument on the onChange callback.

OsamaShabrez commented 7 years ago

Currently there is no way to get the original event, at least I didn't found one and there no mention of original event in docs.

browne0 commented 7 years ago

Would love to see this feature implemented, you can do so much from getting the original event!

kellenmace commented 7 years ago

Another workaround: you can use this to access the name of the select (or any of its other attributes). Example:

class SomeComponent extends React.Component {

  onChangeFunc(optionSelected) {
    const name = this.name;
    const value = optionSelected.value;
    const label = optionSelected.label;
  }

  render() {
    return(
      <Select
        name="form-field-name"
        value={val}
        options={options}
        onChange={this.onChangeFunc}
      />
    )
  }

}
fonsleenaars commented 7 years ago

I've run into this issue, passing a value to an onChange handler passed from one of redux-form's field component, as @jonathanchrisp mentioned, causes the string to be spread operated by this line: https://github.com/erikras/redux-form/blob/master/src/ConnectedField.js#L103

For now the only solution I could think of was writing a single edge case if statement in the container for this component.

I feel like it's kind of a standard using events in onX handlers, so I'm all in favor for introducing that standard here. To maintain compatibility with people using the onChange handler as is, a simple wrapper could be introduced, and exposed through the props? The proposed solution for adding the event as a secondary argument would probably be better.

carlos0202 commented 6 years ago

Any update on this issue?

This library really needs to conform to the expected behavior of input elements. That is, send the original event instead of an object containing label and value.

GuiRitter commented 6 years ago

I'm also looking for this feature, because either it's impossible right now to change the validation message when the field is required or I don't know how to do it. Here is a CodeSandbox that implements this solution to change the message, which doesn't work on Select due to the missing event.

Edit: Nevermind, I found this.

gvincentlh commented 6 years ago

Any chance of ever getting resolution on this issue?

ashnur commented 6 years ago

@gvincentlh this is just a guess, but I think this function should be changed so that it saves the event https://github.com/JedWatson/react-select/blob/master/src/Select.js#L452-L461

Then the event could be passed optionally instead of the value (there is a simpleValue option already, this could be similar I think)

ashnur commented 6 years ago

@JedWatson do you have an opinion on this that you care to share here?

kvedantmahajan commented 6 years ago

I didn't realise this until now and I feel I wasted my time a bit using in my project. This is so common that don't expect it to be not here.

ashnur commented 6 years ago

@kushalmahajan it's quite an interesting fact that react-select became the only viable/usable option for a customized select input control, despite all the drawbacks.

kumarharsh commented 6 years ago

well, @github really needs to fix their autocomplete 😂

a good alternative which I've been following is downshift, but it's more barebones rather than a plug-and-play kind of approach.

cyrilf commented 6 years ago

This is what I came up with: https://gist.github.com/cyrilf/109fc97a22cd080abcc5068a30976696 Not perfect and a bit hacky, but it solved my use-case where I really needed an event

Hope it helps :v:

danyonsatterlee commented 6 years ago

Has anyone taken a look at Material UI's example for React Select and their onChange function? They recommend using React Select for an auto-complete example. It helped me solve the issue of an on change event. This is their sandbox https://codesandbox.io/s/5kvmpwpo9k . For easy reference this is their onChange example and their Select component. I hope it gets someone else started on a solution that works in their project.

handleChange = name => value => {
   this.setState({
      [name]: value,
    });
  };

  <Select
       classes={classes}
       styles={selectStyles}
       options={suggestions}
       components={components}
       value={this.state.single}
       onChange={this.handleChange('single')}
       placeholder="Search a country (start with a)"
       />

I wasn't familiar with multiple arrows in a function. This is a curried function and this had a nice explanation https://stackoverflow.com/questions/32782922/what-do-multiple-arrow-functions-mean-in-javascript

bflorat commented 6 years ago

I made it work by encapsulting the onChange function with another function that pass the Select name :

 onOptionChange = (selectName,selectedOption) => {
         //do something with selectName and selectedOption
    };
 <Select options=...
              name={selectName}
               value=...
               onChange={e => this.onOptionChange(selectName.id,e)}
   />}
ashnur commented 6 years ago

Nice hack. Why not just use vanillajs then?

bflorat commented 6 years ago

It's the method documented by the react.js team : https://reactjs.org/docs/handling-events.html#passing-arguments-to-event-handlers

ashnur commented 6 years ago
onOptionChange = (selectName,selectedOption) => {
    //do something with selectName and selectedOption
};
 <Select options=...
              name={selectName}
               value=...
               onChange={e => this.onOptionChange(selectName.id,e)}
   />}

no, I mean, the variable that seems to be a string, selectName, is actually an object with an .id property.. sure, I could just pass the whole state there, but that doesn't mean I get my original event object unless I somehow recreate a live reference to some dom element, which you seem to be doing here

escott88 commented 5 years ago

The method mentioned by bflorat returns the mouse click event on the react-select rendered div if doing a selection. It definitely would be nice to see this implemented, but can work around it for now. (edited to clarify).

Davidmycodeguy commented 5 years ago

Wow This is not implemented? Accept the PR?

Im worried about backwards compatibility. Others have wrote custom logic because the wired problems and it will break thier hacky solution to this problem.

Mileshosky commented 5 years ago

Encapsulation and normalization is great, but please expose the default browser implementation for those that need it. Should be a minimum when building components around browser UI elements.

markoj3s commented 5 years ago

https://react-select.com/upgrade-guide Seems like there is now a second argument actionMeta for onChange. name is accessible from there.

oshell commented 5 years ago

https://react-select.com/upgrade-guide Seems like there is now a second argument actionMeta for onChange. name is accessible from there.

this should be documented. is it yet? Should be in the minimal example on the readme

ashnur commented 5 years ago

https://react-select.com/upgrade-guide Seems like there is now a second argument actionMeta for onChange. name is accessible from there.

this should be documented. is it yet? Should be in the minimal example on the readme

I don't see how this solves anything related to this issue.

bobort commented 5 years ago

It seems to make the whole issue even worse, @ashnur . If the implementation is ever corrected, now developers who used that second argument will have to refactor their code even more. I expected so much more from this project. I'm just now diving into ReactJS, and I thought I was missing some major concept. Nope. The developers of this particular package just didn't follow standard procedures.

ashnur commented 5 years ago

@bobort have you tried https://github.com/paypal/downshift ? :)

kvedantmahajan commented 5 years ago

Well, the person replying to every comment could have stopped all this by just giving an example that it's okay to return the value "only" instead of even, as a result of handleChange.

If we want to get event, we can easily do so by wrapping the select in a div and let the event bubble. Therefore parents' div can have a handleChange instead and use event.currentTarget.value to retrieve the value.

I remember even I commented on it a while back just to know my statement was incorrect but he seems to reply to everyone in same fashion as he did to my comment. Are you really a representative of this package?

ashnur commented 5 years ago

If we want to get event, we can easily do so by wrapping the select in a div and let the event bubble. Therefore parents' div can have a handleChange instead and use event.currentTarget.value to retrieve the value.

I am not sure who are you talking to but the "we" in this case doesn't apply to me. I don't want to add additional markup to deal with inadequate APIs. Throwing code at problems only leads to problems down the road.

kvedantmahajan commented 5 years ago

So, what was the design process for not passing the event but only the value. I'm curious? In case if some one want to capture the [e.target.name] how is he supposed to handle that if not wrapping up with parent div.

Also, what problems do you think this simple wrap will contain. Let's say I build a React component like this. Why it would be a problem if library authors haven't provided that option? Thoughts!

ashnur commented 5 years ago

So, what was the design process for not passing the event but only the value. I'm curious? In case if some one want to capture the [e.target.name] how is he supposed to handle that if not wrapping up with parent div.

Dear @kushalmahajan I have the same exact questions :)

Also, what problems do you think this simple wrap will contain. Let's say I build a React component like this. Why it would be a problem if library authors haven't provided that option? Thoughts!

It's not about problems. I mean, problems can be stated with additional markup, especially if someone wants the code to follow specific semantics. But the bigger question is, why would a library push its users to such hacks?

oshell commented 5 years ago

https://react-select.com/upgrade-guide Seems like there is now a second argument actionMeta for onChange. name is accessible from there.

this should be documented. is it yet? Should be in the minimal example on the readme

I don't see how this solves anything related to this issue.

https://react-select.com/advanced#action-meta

The documentation is even wrong. It states that action is a string, but it is an object with the properties action, option and name. It solves the issue with getting the name of the target at least.

However, I understand your frustration. This behavior is not intuitive and even worse, not documented correctly. There are other things wrong with this library. For instance, when you define the options as value-label pairs, I would expect it would be enough to pass the value as prop to have a controlled input. But the event only gives me the value, then I have to built the object with the value-label pair again and pass it to the element. This results in a lot of boilerplate, which makes this lib basically a pain in the ass. It is a shame that there is so much work put into this and then it does not even handle the most basic stuff by just following the html standards.

ashnur commented 5 years ago

It is a shame that there is so much work put into this and then it does not even handle the most basic stuff by just following the html standards.

I wouldn't go so far as to blame people. :) Let's be honest, this is still something used for all kinds of meaningful purposes, no matter its drawbacks. There is no reason to try to question the people who I am sure had good reasons for choosing the abstractions they chose.

Rall3n commented 5 years ago

There is no reason to try to question the people who I am sure had good reasons for choosing the abstractions they chose.

They have a good reason to prefer this pattern. The call to the function bound to the onChange prop is not a result of a change event, but it is the result of a keydown, click or touch. The event has no input element as the target, therefore no name, no value.

This pattern at least provides you with the selected option as the value and (thanks to a previous update) the name of the select (provided by the name prop) in the action meta as a second argument.

osifo commented 5 years ago

Wow! I would have expected that by now some work-around would have been available for this though. Hopefully soon.

mathieutu commented 5 years ago

Actually this was already a previous issue, that was marked as closed in v2, but it's not resolved at all: https://github.com/JedWatson/react-select/issues/272

pinging @gwyneplaine on that as he's the one who closed it.

JedWatson commented 4 years ago

Sorry for not providing an official response to this earlier, I'm jumping in because it was asked on Twitter and that drew my attention to this issue (for some reason I thought we'd answered this before)

As @Rall3n said, the reason for the design of the current onChange arguments is that there is no reliable event we can pass. As there is no browser-native change event we can pass, you probably don't want to receive the underlying browser event that triggered the change (click, touch, keypress, etc) because that wouldn't be consistent with other form components and interacting with the event wouldn't necessarily have the expected effect (e.g if you were to preventDefault on it)

In fact from memory, for various conditions we don't even always have an event (or there may not just be one, because of internal debouncing).

It seems better to consistently provide a predictable API, which can be relied upon in all conditions, rather than faking an API which can't be properly implemented. And given that, it also made sense not to even make the API look like something it's not (by mimicing event.currentTarget etc).

The onChange API provides two things: the updated value, and the event meta that describes how the change happened, as well as other useful data like the name property. As far as I know this is everything we can reliably provide.

The only reason I can think of to fake the native event API shape is if you're passing a generic handler to multiple form components that include Select and native inputs... which can be solved by extending the Select component, if that's what you need:

import ReactSelect from 'react-select';
export const Select = ({ onChange, name, ...props }) => {
  const patchedOnChange = (value) => {
    onChange({ currentTarget: { value, name }});
  }
  return <ReactSelect onChange={patchedOnChange} name={name} {...props} />
}

Hope this clears up the reasoning and workaround.

ashnur commented 4 years ago

there is no reliable event we can pass

From the React docs:

Note: If you want to access the event properties in an asynchronous way, you should call event.persist() on the event, which will remove the synthetic event from the pool and allow references to the event to be retained by user code.

there is no browser-native change event we can pass

The HTML element called select does have such an event, being a HTML element. You could use the original or could create a new one.

If you call this a select library, that is to be used in place of that select, it is highly dishonest to say that there isn't such an event. We all used it for quite a long time, it exists. It's your implementation details that are leaking through and blocking a proper API. And I find it obnoxious that you suggest further propagating said leak of internals by way of one of, non-standard workarounds.

JedWatson commented 4 years ago

@ashnur I'm going to address your comment for the benefit of anyone else who is seeking to understand the reasoning here, but I also want to make it clear that your language and tone is not welcome or acceptable.

First of all, this isn't about retaining the synthetic event, I know how to do that - it's about getting an event in the first place.

The HTML element called select does have such an event, being a HTML element. You could use the original or could create a new one.

It's not possible to create a select component and capture an onChange event by changing its value. The event requires user interaction to fire. Faking the interaction would be significantly less predictable than choosing not to try and conform to an API that we can't correctly implement.

If you call this a select library, that is to be used in place of that select, it is highly dishonest to say that there isn't such an event.

There. Isn't. Such. An. Event. There is for native select components, and if they do what you need, I wouldn't recommend using this library.

I find it obnoxious that you suggest further propagating said leak of internals by way of one of, non-standard workarounds.

react-select is one giant non-standard workaround. It fills a gap left by the HTML spec, because native select controls are insufficient for a range of use cases; that hasn't changed in two decades, and this is far from the first library to deal with it (prior art includes Selectize and Select2 for jQuery, you can go further back to YUI and other frameworks if you like)

It's your implementation details that are leaking through and blocking a proper API

There's no such thing as a "proper API". This project is not pretending to be a native form control. If anything, the API has been designed to prevent implementation details (such as unpredictable source events) from leaking through.


Anyway, I'm done with this issue, hope the explanation and workaround helps people who come across this in the future, but I don't maintain this project to be called dishonest or obnoxious.

ashnur commented 4 years ago

@JedWatson

MadeByMike commented 4 years ago

I remember this one time the internet gave me close to 100 thumbs down in a GitHub issue. So I doubled-down on a being jerk to open-source maintainers and it really changed the way everyone saw things.

mdhenriksen commented 4 years ago

I know this might not be the ideal solution, but a workaround would be to set the value to be an object and extracting the values from the event and passing it to the onChange method:

value={{ "valueOne": object.id, "valueTwo": "Another value" }}

And then

onChange={(event) => onChangeMethod(event.target.value.valueOne, event.taget.value.valueTwo)} 
// ^ or you could send the whole event and extract the values in the method

Hope it's useful for somebody else!

ghost commented 3 years ago

i want to get value , not object v3:

 const handleChange =(value)=>{
    console.log(value.map(v => v.id));
  }

you will get [1,2,3,4,5]

tronganhnguyenthanh commented 3 years ago

Do we have any way to save the data in react-select? I mean that when we got the data already and we press F5 to reload it, it can't be lost.

ebonow commented 3 years ago

@tronganhnguyenthanh ,

You would need to implement a client-side storage solution if you want data to persist beyond the lifecycle scope of the application.

There's a plethora of different solutions out there for this kind of thing...

tronganhnguyenthanh commented 3 years ago

Dear Mr Eric Bonow, I would like to say a big thank you for your support and I would like to take your facebook account or any other social media you are using to take the advantage in learning new things. Thank you and best regards, Trong Anh

Vào Th 7, 19 thg 6, 2021 vào lúc 07:40 Eric Bonow < @.***> đã viết:

@tronganhnguyenthanh https://github.com/tronganhnguyenthanh ,

You would need to implement a client-side storage solution if you want data to persist beyond the lifecycle scope of the application.

There's https://usehooks.com/useLocalStorage/ a https://github.com/beautifulinteractions/ plethora https://github.com/hacknlove/usestorage of https://github.com/ugenc/useStorage different https://github.com/alex-cory/use-react-storage solutions https://github.com/capacitor-community/react-hooks out there for this kind of thing...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JedWatson/react-select/issues/1631#issuecomment-864331381, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHLDLXCYRCKAEXTTT3Y34N3TTPRQFANCNFSM4DFPOY6Q .