akiran / react-slick

React carousel component
http://react-slick.neostack.com/
MIT License
11.76k stars 2.11k forks source link

Disable window scrolling while swiping horizontally [MOBILE] #1240

Open ninosaurus opened 6 years ago

ninosaurus commented 6 years ago

When you start scrolling on mobile(Safari && Chrome from iOS) it is able to scroll vertically and thats bad UX. Is there any workarounds for that case. My settings: let settings = { className: this.props.className || '', infinite: true, lazyLoad: true, speed: 200, arrows: this.props.arrows, swipeToSlide: true, touchMove: true, nextArrow: <NextArrow/>, prevArrow: <PrevArrow/>, afterChange: this.props.afterChange, beforeChange: this.props.beforeChange, };

ZibbyKarel commented 6 years ago

yep. I'm having a same issue :(

ninosaurus commented 6 years ago

Is there any updates on this?

dhamberlin commented 6 years ago

Having the same issue. If I had to guess, this is related to the fact that calling preventDefault on react synthetic events does NOT prevent scrolling, unlike with native events, but I haven't yet messed with the source to confirm.

I'll update if I figure it out. Would love an update if anyone has found a good workaround.

ninosaurus commented 6 years ago

Here is my solution, it works fine for me inner-slider.js -> componentDidMount() window.addEventListener('touchmove', this.preventDefault, {passive: false}); inner-slider.js -> componentWillUnmount() window.removeEventListener('touchmove', this.preventDefault, {passive: false});

preventDefault = (e) => { if(this.state.swiping) { e.preventDefault(); e.returnValue = false; return false; } };

ninosaurus commented 6 years ago

You can read more about passive events here -> LINK

yunyong commented 6 years ago

@ninosaurus Hello, I'm also having the same issue. I want to use your code and have a question. How can you change this.state.swiping ? Is there any listener during or before swiping??

yunyong commented 6 years ago

How about this?

componentDidMount(){
    window.addEventListener('touchstart', this.touchStart);
    window.addEventListener('touchmove', this.preventTouch, {passive: false});
}

componentWillUnmount(){
    window.removeEventListener('touchstart', this.touchStart);
    window.removeEventListener('touchmove', this.preventTouch, {passive: false});
}

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

preventTouch(e){
    const minValue = 5; // threshold

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

    // Vertical scrolling does not work when you start swiping horizontally.
    if(Math.abs(this.clientX) > minValue){ 
        e.preventDefault();
        e.returnValue = false;
        return false;
    }
}
vincentaudebert commented 6 years ago

Code above from @yunyong seems to work fine. Thanks

Fishlz commented 5 years ago

Hello, I used your plug-in on the mobile side, but in the page to add "overflow:auto" to add the scroll bar, scroll bar content can not up and down, how to do?

jeancaldeira97 commented 5 years ago

UP! It works for me, thank you! @yunyong

iliquifysnacks commented 5 years ago

Sorry for the stupid question, but where would this solution go? I use gatsbyjs and call the slider like this: https://pastebin.com/ZSY2gsEv

mattcolman commented 5 years ago

I've tried calling e.preventDefault(); inside of innerSliderUtils.swipeMove function and it doesn't work. But I'm surprised, looking at @yunyong 's code it seems calling e.preventDefault ontouchmove is the trick...any ideas?

MungeParty commented 5 years ago

It appears to be applied to a container component. It's watching the touch delta on the window itself and disabling the default window response to mouse movement (i.e. scrolling the body) if the x position of the mouse is a certain distance from the start position. This does mean however you can re-enable scrolling if the touch returns to the initial position. The fix for this would be to accumulate delta or flip a toggle once you're blocking scrolling that you toggle back on touch release.

iDVB commented 5 years ago

@yunyong We've got your solution working, however it seems like it sometimes interferes with the vertical scrolling of the page on mobile? What we have, seems to be rather unreliable and I'm wondering if it's how we implemented it vs the solution itself?

this is the code we have in our stateless functional component...

  const [firstClientX, setFirstClientX] = useState();
  const [firstClientY, setFirstClientY] = useState();
  const [clientX, setClientX] = useState();

  useEffect(() => {
    const touchStart = e => {
      setFirstClientX(e.touches[0].clientX);
      setFirstClientY(e.touches[0].clientY);
    };

    const preventTouch = e => {
      const minValue = 5; // threshold

      setClientX(e.touches[0].clientX - firstClientX);

      // Vertical scrolling does not work when you start swiping horizontally.
      if (Math.abs(clientX) > minValue) {
        e.preventDefault();
        e.returnValue = false;
        return false;
      }
    };

    window.addEventListener('touchstart', touchStart);
    window.addEventListener('touchmove', preventTouch, { passive: false });
    return () => {
      window.removeEventListener('touchstart', touchStart);
      window.removeEventListener('touchmove', preventTouch, {
        passive: false,
      });
    };
  }, [clientX, firstClientX, firstClientY]);
osnysantos commented 5 years ago

Looks like with @yunyong solution it prevent the page to scroll horizontally inside elements with overflow: scroll. Have you experienced this @yunyong?

xavi-tristancho commented 5 years ago

following the code by @yunyong I've come up with a solution that doesn't break the window horizontal scroll by adding the addEventListener to a div containing the react-slick component instead of the window object.

Also @iDVB I don't think it's a good idea to save the firstClientX, firstClientY, clientX into the state as this values do not affect the component render at all, furthermore, the setX calls are slower than just assigning the new values to normal variables defined outside the component given that the touchstart and touchmove are called about 10 times by one simple little scroll.

let firstClientX, clientX;

const preventTouch = e => {
  const minValue = 5; // threshold

  clientX = e.touches[0].clientX - firstClientX;

  // Vertical scrolling does not work when you start swiping horizontally.
  if (Math.abs(clientX) > minValue) {
    e.preventDefault();
    e.returnValue = false;

    return false;
  }
};

const touchStart = e => {
  firstClientX = e.touches[0].clientX;
};

const Slider = ({ children, ...props }) => {
  let containerRef = createRef();

  useEffect(() => {
    if (containerRef.current) {
      containerRef.current.addEventListener("touchstart", touchStart);
      containerRef.current.addEventListener("touchmove", preventTouch, {
        passive: false
      });
    }

    return () => {
      if (containerRef.current) {
        containerRef.current.removeEventListener("touchstart", touchStart);
        containerRef.current.removeEventListener("touchmove", preventTouch, {
          passive: false
        });
      }
    };
  });

  return (
    <div ref={containerRef}>
      <ReactSlick {...settings} {...props}>
        {children}
      </ReactSlick>
    </div>
  );
};
shaneallantaylor commented 5 years ago

Following up on the wonderful solutions from @yunyong , @iDVB , and @xavitb3 - I found that our implementation impacted UX in a negative way. Specifically, if a users touchmove events passed the threshold it would have two consequences:

  1. Vertical scrolling would be stopped (Good! We want this!)
  2. The event.defaultPrevented would be set to true (This turns out to be not desirable)

Why is the second consequence not desirable? Well, users reported the following experience to us:

Upon inspecting the event object upon touchstart, I noticed that all touchstart events that immediately proceeded instances of touchmove the effectively prevented vertical scrolling still had event.defaultPrevented set to true. Bummer.

Initially, it may seem like you can't expect consequence 1 without also expecting consequence 2, but my testing has led me to believe otherwise. Simply removing e.preventDefault() in the preventTouch function worked for me!

I hope this is helpful for others that bump into this issue, but please raise a red flag if you see any issues with my logic.

derwaldgeist commented 5 years ago

Also stumbled upon this issue. Any estimation if we could get a solution right in the library?

paulcpk commented 4 years ago

I solved this problem like so:

// global.css
body.prevent-scroll {
    overflow: hidden;
}

// CustomSlider.jsx
let startX = 0;

const swipeAction = (event) => {
  const { type } = event;
  const { screenX } = event.changedTouches[0];
  const threshold = 20;

  if (type === 'touchstart') {
    startX = screenX;
  } else if (type === 'touchmove') {
    if (screenX > startX + threshold || screenX < startX - threshold) {
      // moved more than 20px left or right
      document.body.classList.add('prevent-scroll');
    }
  } else if (type === 'touchend') {
    document.body.classList.remove('prevent-scroll');
    startX = 0;
  }
};

const CustomSlider = () => (
  <div
    className="custom-slider"
    onTouchEnd={swipeAction}
    onTouchMove={swipeAction}
    onTouchStart={swipeAction}
  >
    <Slider />
  </div>
);

No htmlRefs and no re- and deregistering of eventListeners needed.

emphaticsunshine commented 4 years ago

How about this?

componentDidMount(){
    window.addEventListener('touchstart', this.touchStart);
    window.addEventListener('touchmove', this.preventTouch, {passive: false});
}

componentWillUnmount(){
    window.removeEventListener('touchstart', this.touchStart);
    window.removeEventListener('touchmove', this.preventTouch, {passive: false});
}

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

preventTouch(e){
    const minValue = 5; // threshold

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

    // Vertical scrolling does not work when you start swiping horizontally.
    if(Math.abs(this.clientX) > minValue){ 
        e.preventDefault();
        e.returnValue = false;
        return false;
    }
}

following the code by @yunyong I've come up with a solution that doesn't break the window horizontal scroll by adding the addEventListener to a div containing the react-slick component instead of the window object.

Also @iDVB I don't think it's a good idea to save the firstClientX, firstClientY, clientX into the state as this values do not affect the component render at all, furthermore, the setX calls are slower than just assigning the new values to normal variables defined outside the component given that the touchstart and touchmove are called about 10 times by one simple little scroll.

let firstClientX, clientX;

const preventTouch = e => {
  const minValue = 5; // threshold

  clientX = e.touches[0].clientX - firstClientX;

  // Vertical scrolling does not work when you start swiping horizontally.
  if (Math.abs(clientX) > minValue) {
    e.preventDefault();
    e.returnValue = false;

    return false;
  }
};

const touchStart = e => {
  firstClientX = e.touches[0].clientX;
};

const Slider = ({ children, ...props }) => {
  let containerRef = createRef();

  useEffect(() => {
    if (containerRef.current) {
      containerRef.current.addEventListener("touchstart", touchStart);
      containerRef.current.addEventListener("touchmove", preventTouch, {
        passive: false
      });
    }

    return () => {
      if (containerRef.current) {
        containerRef.current.removeEventListener("touchstart", touchStart);
        containerRef.current.removeEventListener("touchmove", preventTouch, {
          passive: false
        });
      }
    };
  });

  return (
    <div ref={containerRef}>
      <ReactSlick {...settings} {...props}>
        {children}
      </ReactSlick>
    </div>
  );
};

How can you call preventDefault function when the event handler is passive. Aren't passive events not cancellable?

distillery-david-rearte commented 3 years ago

yeah.. still hacking this, here's my solution

import { disableBodyScroll, enableBodyScroll } from 'body-scroll-lock';

// Some component definition

const slider = useRef(null);

const lockVerticalScrollWhenHorizontalSwiping = (direction: 'vertical' | 'right' | 'left'): void => {
    const isHorizontal = direction !== 'vertical';
    if (isHorizontal) {
      // Will be released when the gesture finish even if the slide has no changed.
      disableBodyScroll(slider.current);
    }
  };

const releaseBodyScroll = (): void => enableBodyScroll(slider.current);

return (
    <div onTouchEnd={releaseBodyScroll}>
      <Slider
        ref={slider}
        swipeEvent={lockVerticalScrollWhenHorizontalSwiping}
        enableVerticalScroll
        {...settings}
      >
        {theslides}
      </Slider>
    </div>
  );
ioxua commented 2 years ago

Here's @xavi-tristancho's answer as a React Hook:

import { RefObject, useCallback, useEffect, useRef } from 'react'

// @see https://github.com/akiran/react-slick/issues/1240#issuecomment-513235261
export function usePreventVerticalScroll<T extends HTMLElement>(ref: RefObject<T>, dragThreshold = 5) {
  const firstClientX = useRef<number>(0)
  const clientX = useRef<number>(0)

  const preventTouch = useCallback(
    (e: TouchEvent) => {
      clientX.current = e.touches[0].clientX - firstClientX.current
      // Vertical scrolling does not work when you start swiping horizontally.
      if (Math.abs(clientX.current) > dragThreshold) {
        e.preventDefault()
        e.returnValue = false
        return false
      }

      return true
    },
    [dragThreshold],
  )

  const touchStart = useCallback((e: TouchEvent) => {
    firstClientX.current = e.touches[0].clientX
  }, [])

  useEffect(() => {
    const current = ref.current
    if (current) {
      current.addEventListener('touchstart', touchStart)
      current.addEventListener('touchmove', preventTouch, { passive: false })
    }
    return () => {
      if (current) {
        current.removeEventListener('touchstart', touchStart)
        // Had to change this line to prevent a typing error. You may not have the issue:
        // current.removeEventListener('touchmove', preventTouch, { passive: false })
        current.removeEventListener('touchmove', preventTouch)
      }
    }
  }, [preventTouch, ref, touchStart])
}