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

Why is return false required? #74

Open
ljharb opened this issue Feb 12, 2016 · 7 comments
Open

Why is return false required? #74

ljharb opened this issue Feb 12, 2016 · 7 comments

Comments

@ljharb
Copy link

ljharb commented Feb 12, 2016

Per https://github.com/JedWatson/react-tappable#native-events, it seems like you're recommending using return false from an event handler to prevent Tappable from handling the event.

However, it's a common best practice (and one my company enforces in its own codebase) to never return false from an event handler, and to always only explicitly call preventDefault/stopPropagation/stopImmediatePropagation on the event object.

Would it be possible to make Tappable respect preventDefault on the event object such that return false isn't required?

@nmn
Copy link
Contributor

nmn commented Apr 30, 2016

I don't see a problem with doing it. @JedWatson ?

@dcousens
Copy link
Collaborator

dcousens commented May 8, 2017

PRs accepted

@ljharb
Copy link
Author

ljharb commented May 17, 2017

@dcousens yes thanks, but given that that's the default on Github that's not particularly helpful :-) could you perhaps point me to the part of the code I should start looking at?

@dcousens
Copy link
Collaborator

dcousens commented May 17, 2017

https://github.com/JedWatson/react-tappable/blob/master/src/TappableMixin.js#L68 - however I'll need a test case to understand, as at this stage, I don't understand why that advice is there unless you're somehow handling onTouchStart yourself (outside of this module).

@ljharb
Copy link
Author

ljharb commented May 17, 2017

Thanks! For one, capturing all touch events that bubble up to document, for logging purposes. return false stops propagation, which is almost never necessary, and kills this use case.

@dcousens
Copy link
Collaborator

dcousens commented Apr 3, 2018

@ljharb shall we close?

@ljharb
Copy link
Author

ljharb commented Apr 4, 2018

I’d prefer it remain open until someone (myself, maintainers, or someone else) can fix it. That the issue is old doesn’t make it any less of an issue :-)

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

No branches or pull requests

3 participants