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

switch onChange to onInput because of IE11 react bug #261

Closed
wants to merge 1 commit into from

Conversation

cuth
Copy link

@cuth cuth commented Oct 5, 2016

The reason for this PR is because of this React Bug: facebook/react#7027

Instead of waiting for React v16, we are editing this component to use onInput instead of onChange to see if that helps IE11.

This breaks all the tests and just replacing "onChange" with "onInput" didn't fix the tests.

@cuth cuth closed this Oct 5, 2016
@cuth
Copy link
Author

cuth commented Oct 5, 2016

So sorry, my bad, I only wanted to do this locally!

@jbandi
Copy link

jbandi commented Oct 10, 2016

I would be interested if this resulted in an acceptable workaround until React 16 is released?

@cuth
Copy link
Author

cuth commented Oct 10, 2016

So far, so good

@tannerjt
Copy link

tannerjt commented Dec 8, 2016

I was able to implement the change (onChange => onInput), however the problem persists. Is this workaround actually binding the method to the onInput event, or just changing the name of the property that is passed in? I am thinking the workaround would need to be done on react-autowhatever, since that is the component that creates the input element. My fingers are crosses for react 16 to come out soon.

@jbandi
Copy link

jbandi commented Dec 8, 2016

We are using this implementation of react-autosuggest in our project:
https://github.com/postlight/react-autosuggest

The bug described in #262 is fixed in that fork of react-autosuggest.

@tannerjt
Copy link

tannerjt commented Dec 8, 2016

Thanks @jbandi. It would appear it isn't merged into the master branch, correct? I'm assuming the correct branch with the edits is the fix-onchange-to-oninput? I thought I had tried this and was still getting errors in IE11. I'll clone your fork and try again.

@tannerjt
Copy link

The fix isn't working for me. Are you testing in IE11? I think your changes (here) are only biding a new function called onInput that is secretly still biding to the native onChange event. I'll probably wait until react 16 comes out, since this seems to be a problem with React

@jbandi
Copy link

jbandi commented Dec 27, 2016

I am not sure if we are talking about the same problem.

In our project we had the following problem:
#262

The above problem is fixed with the following fork of react-autosuggest:
https://github.com/postlight/react-autosuggest

We are testing in IE11.

I did not really look at the implementation of the fix. But as seen in the bug report here:
facebook/react#7027

... the underlying problem seems to come from "wrong" timing or the OnChange Event, which is a synthetic event of React. Therefore switching to the OnInput Event could well be the fix...

@JacksonKearl
Copy link

For a version with @cuth 's fix, which also passes the tests: see my branch.

Or npm install react-autosuggest-ie11-compatible. This is a direct drop in replacement for moroshko's version; there's no need to mess around with changing onInput or onChange in the invocation.

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

Successfully merging this pull request may close these issues.

4 participants