brainhubeu / react-carousel

A pure extendable React carousel, powered by Brainhub (craftsmen who ❤️ JS)
https://brainhub.eu/
MIT License
1.08k stars 164 forks source link

Disable vertical scroll (touchmove) when dragging #244

Open EmilEriksen opened 4 years ago

EmilEriksen commented 4 years ago

Issuehunt badges

Currently, it's quite easy to accidentally scroll vertically when dragging (at least on iOS). This delivers a sub-par UX where you feel like you have to be really careful when you're swiping through a slideshow (it feels fragile). It should be easy to notice if you compare it do something like Zalando on iOS (or maybe any touch screen device). I was completely unable to "accidentally" scroll vertically while dragging their slider. I'm not sure how it's implemented but I'm guessing vertical touchmove is being disabled when dragging starts.


IssueHunt Summary ### Backers (Total: $0.00) #### [Become a backer now!](https://issuehunt.io/r/brainhubeu/react-carousel/issues/244) #### [Or submit a pull request to get the deposits!](https://issuehunt.io/r/brainhubeu/react-carousel/issues/244) ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/brainhubeu/react-carousel/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.
shlaing commented 4 years ago

@EmilEriksen , You need to bind two events "touchstart" and "touchmove".

shlaing commented 4 years ago

@EmilEriksen , Disable horizontal swiping while you are doing vertical swipe. This is how I solved it. However, this library should automatically handle this and It will be cool. @piotr-s-brainhub

componentDidMount() {

    this.nv.addEventListener("touchstart", this.handleNvEnter);
    this.nv.addEventListener("touchmove", this.handleNvMove);
}

componentWillUnmount() {
    this.nv.removeEventListener("touchstart", this.handleNvEnter);
    this.nv.removeEventListener("touchmove", this.handleNvMove);
}

handleNvEnter (event) {
    this.firstClientX = event.touches[0].clientX;
    this.firstClientY = event.touches[0].clientY;
}

handleNvMove (event) {

    const minValue = 10; // threshold
    const maxValue = -10;

    this.clientX = event.touches[0].clientX - this.firstClientX;
    this.clientY = event.touches[0].clientY - this.firstClientY;

    if((this.clientX > minValue || this.clientX < maxValue) && event.cancelable){
        //console.log("Window Scroll Should Stop Here")
        event.preventDefault()
    }
}
issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $15.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $1.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $1.00 to this issue.


piotr-s-brainhub commented 4 years ago

@shlaing

Thanks for your suggestion.

You're more than welcome to open a PR.

You could earn some money because it's funded on IssueHunt.

shlaing commented 4 years ago

Yes @piotr-s-brainhub , I can contribute that.

piotr-s-brainhub commented 4 years ago

@shlaing

That's great so we're waiting for a PR.

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $15.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $1.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $1.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $18.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $10.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $8.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $5.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $10.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $1.00 to this issue.


piotr-s-brainhub commented 4 years ago

@shlaing

Are you going to work on this?

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $20.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $5.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has funded $2.00 to this issue.


issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $18.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $10.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $8.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $5.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $10.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $1.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $20.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $5.00) See it on IssueHunt

issuehunt-oss[bot] commented 4 years ago

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $2.00) See it on IssueHunt

yannbf commented 4 years ago

For anyone stumbling upon this issue, you can add this to your page's css and should be enough:

  .BrainhubCarousel__container {
    touch-action: pan-x; 
  }

@piotr-s-brainhub I'm not sure if this is a desirable fix to do in this lib directly, given that it does not allow users to start scrolling vertically if they touch the carousel container. It works nicely though. :)

eggachecat commented 3 years ago

here's another workaround for functional components. I found comparing the aspect of deltaY aspect to deltaX is more accurate in my case


  const touchRef = useRef({
    startX: 0,
    startY: 0
  });

  // const [realTimeMsg, setRealTimeMsg] = useState("")

  return <div style={{ width: "100%" }} onTouchStart={(e) => {
    touchRef.current.startX = e.touches[0].clientX
    touchRef.current.startY = e.touches[0].clientY
  }} onTouchMove={(e) => {
    const deltaY = Math.abs(
      touchRef.current.startY - e.touches[0].clientY
    )
    const deltaX = Math.abs(
      touchRef.current.startX - e.touches[0].clientX
    )
    // setRealTimeMsg(`${deltaY}: ${deltaX}`)
    if (deltaY / deltaX > 1) {
      e.stopPropagation()
    }
    // e.preventDefault()
  }}>
    {/* {realTimeMsg} */}
    <Carousel
      plugins={slides.length > 1 ? [
        // 'infinite',
        // 'arrows',
        'fastSwipe'
      ] : []}
      value={value}
      slides={slides}
      onChange={setValue}
      draggable={slides.length > 1}
    />
    {slides.length > 1 ? <Dots value={value} onChange={setValue} number={slides.length} /> : <div style={{ height: "25px" }}>&nbsp;</div>}
  </div>
tonygustafsson commented 3 years ago

For anyone stumbling upon this issue, you can add this to your page's css and should be enough:

  .BrainhubCarousel__container {
    touch-action: pan-x; 
  }

@piotr-s-brainhub I'm not sure if this is a desirable fix to do in this lib directly, given that it does not allow users to start scrolling vertically if they touch the carousel container. It works nicely though. :)

That's the best one without a doubt! :D Thanks man... solved it for me. Couldn't ask for less code.