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

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

Closed
OsamaShabrez opened this issue Mar 28, 2017 · 46 comments
Closed

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

OsamaShabrez opened this issue Mar 28, 2017 · 46 comments

Comments

@OsamaShabrez
Copy link

OsamaShabrez commented Mar 28, 2017

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
Copy link

jwarykowski commented Apr 13, 2017

👍 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
Copy link
Author

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
Copy link

browne0 commented Jun 7, 2017

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

@kellenmace
Copy link

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
Copy link

fonsleenaars commented Oct 2, 2017

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
Copy link

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
Copy link

GuiRitter commented May 17, 2018

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
Copy link

gvincentlh commented Jun 6, 2018

Any chance of ever getting resolution on this issue?

@ashnur
Copy link

ashnur commented Jun 11, 2018

@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
Copy link

ashnur commented Jun 12, 2018

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

@kvedantmahajan
Copy link

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
Copy link

ashnur commented Jul 12, 2018

@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
Copy link
Contributor

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
Copy link

cyrilf commented Aug 30, 2018

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 ✌️

@danyonsatterlee
Copy link

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
Copy link

bflorat commented Sep 20, 2018

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
Copy link

ashnur commented Sep 20, 2018

Nice hack. Why not just use vanillajs then?

@bflorat
Copy link

bflorat commented Sep 20, 2018

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

@ashnur
Copy link

ashnur commented Sep 20, 2018

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
Copy link

escott88 commented Nov 15, 2018

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
Copy link

Davidmycodeguy commented Dec 13, 2018

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
Copy link

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
Copy link

markoj3s commented Jan 8, 2019

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

@oshell
Copy link

oshell commented Jan 23, 2019

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
Copy link

ashnur commented Jan 23, 2019

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
Copy link

bobort commented Jan 28, 2019

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
Copy link

ashnur commented Jan 28, 2019

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

@kvedantmahajan
Copy link

kvedantmahajan commented Jan 29, 2019

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
Copy link

ashnur commented Jan 30, 2019

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
Copy link

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
Copy link

ashnur commented Jan 30, 2019

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
Copy link

oshell commented Jan 31, 2019

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
Copy link

ashnur commented Feb 1, 2019

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
Copy link
Collaborator

Rall3n commented Feb 7, 2019

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
Copy link

osifo commented Feb 9, 2019

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

francocorreasosa pushed a commit to auth0/cosmos that referenced this issue Feb 22, 2019
## Issues
- No way to access the entire `onChange` event: JedWatson/react-select#1631 so we're "simulating it" https://github.com/auth0/cosmos/pull/1512/files#diff-92e315380bbd9b642236e410bd014289R147 
- It does not seem to output a real `<select>` component so:
  - We cannot use our current automation attributes.
  - We cannot add a custom id to the select because there is no select.

API Compatibility: **No breaking changes**. 

In this PR I include some "adapters" for react-select that may slow down the performance of the component, we could make our API closer to react-select's one in order to remove this adapters and improve performance.
@mathieutu
Copy link

mathieutu commented Jul 31, 2019

Actually this was already a previous issue, that was marked as closed in v2, but it's not resolved at all:
#272

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

@JedWatson
Copy link
Owner

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
Copy link

ashnur commented Nov 20, 2019

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
Copy link
Owner

@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
Copy link

ashnur commented Nov 20, 2019

@JedWatson

  • thank for clarifying, even if it took you 2.5 years.
  • for the record: I called your actions what they are, I haven't called you anything. If you can't make this distinction, that's not on me.

@MadeByMike
Copy link

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.

landitus pushed a commit to auth0/cosmos that referenced this issue Jan 17, 2020
## Issues
- No way to access the entire `onChange` event: JedWatson/react-select#1631 so we're "simulating it" https://github.com/auth0/cosmos/pull/1512/files#diff-92e315380bbd9b642236e410bd014289R147 
- It does not seem to output a real `<select>` component so:
  - We cannot use our current automation attributes.
  - We cannot add a custom id to the select because there is no select.

API Compatibility: **No breaking changes**. 

In this PR I include some "adapters" for react-select that may slow down the performance of the component, we could make our API closer to react-select's one in order to remove this adapters and improve performance.
@mdhenriksen
Copy link

mdhenriksen commented Aug 17, 2020

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
Copy link

ghost commented Oct 20, 2020

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
Copy link

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
Copy link
Collaborator

ebonow commented Jun 19, 2021

@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
Copy link

tronganhnguyenthanh commented Jun 19, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests