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

fix onChange with placeholder on IE #5004

Closed
wants to merge 6 commits into from

Conversation

yiminghe
Copy link
Contributor

@yiminghe
Copy link
Contributor Author

btw: how to test on ie10?

grunt test --debug does not work any more

I have to grunt build and test build/react.js on my example page.

@afc163
Copy link

afc163 commented Sep 30, 2015

👍

@sophiebits
Copy link
Collaborator

At least some of this code would need to go in ChangeEventPlugin, because the event is already going to bubble by the time _handleChange is invoked in ReactDOMInput. _currentValue should also go within the ReactDOMInput-specific _wrapperState object; you shouldn't need to add anything to every ReactDOMComponent. I think it's hard/impossible to get the ReactDOMInput instance from ChangeEventPlugin though.

We don't run our tests in browsers any more. It wasn't something we used often and it got lost in one of our change to extract common modules to https://github.com/facebook/fbjs.

@yiminghe
Copy link
Contributor Author

yiminghe commented Oct 1, 2015

ok, have moved _currentValue to _wrapperState and prevent false change from bubbling (fortunately input can not has any child).

ChangeEventPlugin is already very complex, I think change in ReactDOMInput is easy and effective.

@sophiebits
Copy link
Collaborator

#4051 should have fixed this.

@sophiebits sophiebits closed this Dec 16, 2015
afc163 referenced this pull request in ant-design/ant-design Feb 23, 2016
yiminghe referenced this pull request in ant-design/ant-design Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants