JedWatson / react-tappable

Tappable component for React
http://jedwatson.github.io/react-tappable/
MIT License
863 stars 90 forks source link

Fixes a nasty bug that breaks preventDefault #49

Closed nmn closed 9 years ago

nmn commented 9 years ago

calling preventDefault on the event received in onTap is currently broken on iOS. This fixes that.

nmn commented 9 years ago

The reason this breaks is because of setState. The onTap is only being fired after the setState completes. I'm not sure this complete necessary.

Otherwise, there may be a way to just mutate state and call forceUpdate instead to make it synchronous. I'll look into it so that there is not need for such a bad hack.

JedWatson commented 9 years ago

@nmn there's been a series of updates and PRs from @slorber that fix the issue and remove this hack, see #39 #47 and #51. Let us know if you see any issues with the current implementation.

nmn commented 9 years ago

Fair enough. Didn't notice this. I guess I'll have to take care to update my refactor to smaller modules.

slorber commented 9 years ago

@nmm this should not impact you much because all SyntheticEvent passed to the onTap callback will be sync, and will be able to call preventDefault (unless you use setTimeout/debounce before preventDefault, which does not make sense anyway. Check also http://stackoverflow.com/a/28046731/82609).

nmn commented 9 years ago

@slorber I get that. I have a separate PR that majorly refactors all of React-Tappable into smaller modules. I was just saying that I need to be careful to not have a regression.