Prevent scroll prop

[stefvhuynh]

• Renames preventDefaultTouchmoveEvent to preventScrollOnSwipe
• Makes all touch handlers passive by default except for touchmove when preventScrollOnSwipe is true
• Removes and reattaches touch handlers when preventScrollOnSwipe value changes (fixes the main example)
• Updates README

TODO before merge

• Currently, this PR sets passive: true on touchstart and touchend. We need to decide what these should be and if we should introduce an eventOptions object to allow users to set these themselves.
• I was not able to run tests locally. Jest would just hang when I execute the command, so test need to be updated before this merges.

[fivethreeo]

Any progress on this?

[hartzis]

Any progress on this?

It's on my mind 😸 . It will be a major version change so i'm seeing what else we can/should get in like #267

[jdesherlia]

@hartzis I am running into the need for this in our implementation, and the example given here is exactly what I am trying to accomplish. Scrollable list of swipeable items that expose additional options, a la a mobile to do list or something similar.

Looking at the documentation you referenced for use-gesture, it seems that in conjunction with setting touch-action: pan-y; on the swipeable element, they have an option to specify the axis of swipe when configuring their useDrag hook. Would your solution here mimic or mitigate the need for something that? If you have a build I can test, I am happy to do so.

[bhj]

Thank you all for your work on this awesome library. There's a tangential issue I ran into that maybe this PR can address: In trying to create a generic <Swipeable> component that sets some defaults, siphons off all the possible swipe-related props and passes the rest through, e.g.:

const Swipeable = ({
  onSwiped,
  onSwipedLeft,
  onSwipedRight,
  onSwipedUp,
  onSwipedDown,
  onSwipeStart,
  onSwiping,
  onTap,
  delta = 10,
  preventDefaultTouchmoveEvent = true,
  trackTouch = true,
  trackMouse = true,
  rotationAngle = 0,
  ...props
}) => {
  ...
}

I noticed that the preventDefaultTouchmoveEvent test is only checking for the presence of the onSwiped* properties so in this case all scrolling gets prevented. It would be nicer if it checked whether their values are actually defined, I think?

[hartzis]

I noticed that the preventDefaultTouchmoveEvent test is only checking for the presence of the onSwiped* properties so in this case all scrolling gets prevented. It would be nicer if it checked whether their values are actually defined, I think?

@bhj thanks for the feedback, curious what you mean by if it checked whether their values are actually defined, the current presence check sounds similar. Do you think you could code up a small example?

[bhj]

@bhj thanks for the feedback, curious what you mean by if it checked whether their values are actually defined, the current presence check sounds similar.

Given my example Swipeable HoC, say it eventually passes all possible swipe-related props to useSwipeable. If I only give Swipeable an onSwipedLeft prop, then most of the config properties useSwipeable receives have the value undefined. Yet all scrolling will still be prevented, because in only checks for the presence of a property, and returns true even if its value is undefined.

So, in this conditional:

if (props.onSwiping || props.onSwiped || `onSwiped${dir}` in props)

The third check is doing something very different from the first two. I think it should check whether the properties' values are actually defined, like the first two, rather than simply whether or not the properties exist.

[hartzis]

So, in this conditional:

if (props.onSwiping || props.onSwiped || `onSwiped${dir}` in props)

The third check is doing something very different from the first two. I think it should check whether the properties' values are actually defined, like the first two, rather than simply whether or not the properties exist.

yup, lol. welp that's been hiding in there for a while 😬

@bhj Thank you for pointing that out, apologies for not fully grasping what you were getting at before.

We'll get this fixed in the next version for sure 👍

[github-actions[bot]]

size-limit report 📦

Path	Size
dist/react-swipeable.js	1.11 KB (0%)
dist/react-swipeable.umd.js	1.12 KB (0%)
dist/react-swipeable.module.js	1.14 KB (0%)