react-tappable icon indicating copy to clipboard operation
react-tappable copied to clipboard

Why is `return false` required?

Open ljharb opened this issue 9 years ago • 7 comments

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?

ljharb avatar Feb 12 '16 21:02 ljharb

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

nmn avatar Apr 30 '16 23:04 nmn

PRs accepted

dcousens avatar May 08 '17 01:05 dcousens

@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?

ljharb avatar May 17 '17 05:05 ljharb

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).

dcousens avatar May 17 '17 06:05 dcousens

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.

ljharb avatar May 17 '17 07:05 ljharb

@ljharb shall we close?

dcousens avatar Apr 03 '18 23:04 dcousens

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 :-)

ljharb avatar Apr 04 '18 01:04 ljharb