-
Notifications
You must be signed in to change notification settings - Fork 149
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
Preventdefault touch ios fix2 #131
Conversation
const getTouchHandlerOption = props => { | ||
if (props.touchHandlerOption) return props.touchHandlerOption | ||
return props.preventDefaultTouchmoveEvent | ||
? { passive: false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventListener
options no longer needed since this pr attaches touch listeners to the swipe container via ref
callback
function getHandlers(set, props) { | ||
const onStart = event => { | ||
// if more than a single touch don't track, for now... | ||
if (event.touches && event.touches.length > 1) return | ||
|
||
set(() => { | ||
set(state => { | ||
if (state.props.trackMouse) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comment here about why we attach mouse tracking to document
// setup mouse listeners on document to track swipe if swipe leaves container
src/index.js
Outdated
document.removeEventListener(touchEnd, onUp, touchHandlerOption) | ||
} | ||
set(state => { | ||
if (state.props.trackMouse) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove set
in stop
since it only removes event listeners which is safe
if (state.el === el) return state | ||
if (state.el && state.el !== el) cleanUp(state.el) | ||
if (state.props.trackTouch) { | ||
if (el && el.addEventListener) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment all these if
's
This solved my issue. @hartzis thank you very much for your help and especially for creating an alpha release of this feature 🎉 I'm going to close my PR |
@JiiB Thank you for your help in testing and pushing to get this issue fixed! It is fun to go on deep dives like this to truly understand how this stuff works. I got the tests updated tonight, should be able to get the docs updated and |
[email protected]
After more digging and exploration this mimics
v4
by using aref
callback to then set thetouchstart
andtouchmove
event listeners before any touch/swipe starts being tracked.This solution is ultimately a work around for these issues:
{ passive: false }
option set, which is needed to callpreventDefault
touchmove
be attached before a touch event startspassive
option defaults tofalse
when attaching the touch listeners to the swiping container via theref
, chrome/firefox and possibly iosdocument
thepassive
option defaults totrue
TODO:
touchHandlerOption
andpassive
event listener notesuseSwipeable
's returnedhandlers
have a callbackref
nowinnerRef
prop for<Swipeable />
can only be a callback(el) => this.swipeable = el;