akiran / react-slick

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

Drag on sliders fires a click event #848

Closed GiancarlosIO closed 6 years ago

GiancarlosIO commented 7 years ago

When you drag a slider with a tag "<a href="www.google.com />" slick fires a click to finish the drag. How can i prevent that?

example: https://jsfiddle.net/nexuszgt/pgoap3bf/4/

thanks.

costagolub commented 7 years ago

@GiancarlosIO You can achieve the desired effect by using beforeChangeand afterChangemethodsand assign an onClickon the anchors. Based on your sample code this should be working and will not force links to open on dragging. You can also use ES6 classes instead and bind to thisinstead of a variable. https://jsfiddle.net/eckrawhy/

GiancarlosIO commented 7 years ago

@costagolub your "hack" works but i think slick-lib should be fix that because it's a behavior we don't expect

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

boltcoder commented 6 years ago

has this been fixed?

amabunny commented 6 years ago

has this been fixed?

Its not fixed on my 0.23.1 version

fullstackzach commented 5 years ago

Here's my solution that makes use of clientX and clientY attributes of the click and mousedown events and react component state to solve this issue.

Basically it's comparing the mouse position at mouse down and click and if either X or Y is different, then it will preventDefault() (default browser behavior being to go to the link)

class Carousel extends React.Component {
  constructor (props) {
    super(props)
    this.state = {
      clientXonMouseDown: null,
      clientYonMouseDown: null
    }
    this.settings = {
      dots: true,
      arrows: false,
      swipeToSlide: true,
      infinite: false,
    ...
    }
  }

  handleOnMouseDown (e) {
    this.setState({
      clientXonMouseDown: e.clientX,
      clientYonMouseDown: e.clientY
    })
    e.preventDefault() // stops weird link dragging effect
  }

  handleOnClick (e) {
    e.stopPropagation()
    if (this.state.clientXonMouseDown !== e.clientX || 
        this.state.clientYonMouseDown !== e.clientY) {
      // prevent link click if the element was dragged
      e.preventDefault()
    }
  }

  render () {
    const { items } = this.props
    return (
      <Slider
        {...this.settings} >
        {items.map((item, index) => (
          <a key={index} href={item.link} target='_blank'
            onMouseDown={e => this.handleOnMouseDown(e)}
            onClick={e => this.handleOnClick(e)}
            >
           Link innerText
          </a>
        ))}
      </Slider>)
  }
}

export default Carousel
akashmdkml commented 5 years ago

Here's my solution that makes use of clientX and clientY attributes of the click and mousedown events and react component state to solve this issue.

Basically it's comparing the mouse position at mouse down and click and if either X or Y is different, then it will preventDefault() (default browser behavior being to go to the link)

class Carousel extends React.Component {
  constructor (props) {
    super(props)
    this.state = {
      clientXonMouseDown: null,
      clientYonMouseDown: null
    }
    this.settings = {
      dots: true,
      arrows: false,
      swipeToSlide: true,
      infinite: false,
    ...
    }
  }

  handleOnMouseDown (e) {
    this.setState({
      clientXonMouseDown: e.clientX,
      clientYonMouseDown: e.clientY
    })
    e.preventDefault() // stops weird link dragging effect
  }

  handleOnClick (e) {
    e.stopPropagation()
    if (this.state.clientXonMouseDown !== e.clientX || 
        this.state.clientYonMouseDown !== e.clientY) {
      // prevent link click if the element was dragged
      e.preventDefault()
    }
  }

  render () {
    const { items } = this.props
    return (
      <Slider
        {...this.settings} >
        {items.map((item, index) => (
          <a key={index} href={item.link} target='_blank'
            onMouseDown={e => this.handleOnMouseDown(e)}
            onClick={e => this.handleOnClick(e)}
            >
           Link innerText
          </a>
        ))}
      </Slider>)
  }
}

export default Carousel

if we use this logic, it blocks focusOnSelect event which is called on onClick, and the swiping wont be smooth.. that mean it wont fire swipe event too...experience is like a forced swipe .. @fullstackzach

Web-Medias commented 5 years ago

A simple way to bypass click triggering when swipping has already been writted by VeronQ...

But the internal bubbling and native events manager of the lib tiggers the click after the mouseup/touchend. So the way is to delay a bit the class removing.

                // Adds a class when swipping...
                $('.slick-slider').off('swipe').on('swipe', function(event, slick, direction){
                    $('.slick-slider').addClass('swipping');
                });
                // Remove tha class after swipe was done, AND delayed of ~100 ms
                $('.slick-slider').off('afterChange').on('afterChange', function(event, slick, currentSlide, nextSlide){
                    setTimeout( function(){ $('.slick-slider').removeClass('swipping'); } , 110 );
                });
                // Your event on the item when the user clicks on.
                $('.slick-slider .gp-item .item').off().on('click', function(e){
                    e.preventDefault();
                    if( $('.slick-slider').hasClass('swipping') )return;

                    // Your onclick code here...
                });
ZhekaVasil commented 5 years ago

There is another approach. We could use onClickCapture, onMouseUpCapture, onMouseDownCapture events to prevent click propagation during swiping:

See details here: https://github.com/akiran/react-slick/issues/604

gavin310 commented 4 years ago

I'd just like to add something that helped me (not a solution, sorry). There's a setting to disable dragging (draggable: false), but I didn't want to do that because I want mobile users to be able to drag. What I didn't understand is that draggable is only for the mouse. Setting draggable: false still allows touchscreen users to swipe the slider, and this click-after-dragging issue doesn't happen with touchscreens. For desktop users I don't care if they cannot drag the slider with the mouse since I have arrows, so just setting draggable: false prevents this click issue for desktop users, and mobile users can still swipe the carousel without any issues.

akifarhan commented 4 years ago

Here's my solution that makes use of clientX and clientY attributes of the click and mousedown events and react component state to solve this issue.

Basically it's comparing the mouse position at mouse down and click and if either X or Y is different, then it will preventDefault() (default browser behavior being to go to the link)

class Carousel extends React.Component {
  constructor (props) {
    super(props)
    this.state = {
      clientXonMouseDown: null,
      clientYonMouseDown: null
    }
    this.settings = {
      dots: true,
      arrows: false,
      swipeToSlide: true,
      infinite: false,
    ...
    }
  }

  handleOnMouseDown (e) {
    this.setState({
      clientXonMouseDown: e.clientX,
      clientYonMouseDown: e.clientY
    })
    e.preventDefault() // stops weird link dragging effect
  }

  handleOnClick (e) {
    e.stopPropagation()
    if (this.state.clientXonMouseDown !== e.clientX || 
        this.state.clientYonMouseDown !== e.clientY) {
      // prevent link click if the element was dragged
      e.preventDefault()
    }
  }

  render () {
    const { items } = this.props
    return (
      <Slider
        {...this.settings} >
        {items.map((item, index) => (
          <a key={index} href={item.link} target='_blank'
            onMouseDown={e => this.handleOnMouseDown(e)}
            onClick={e => this.handleOnClick(e)}
            >
           Link innerText
          </a>
        ))}
      </Slider>)
  }
}

export default Carousel

I've tried this solution. It works fine for the dragging part. I can drag the items. But when I clicked, there's nothing happened. So I check on what happen in the handleOnClick(), I found that the value of those states are undefined as it didnt mutated immediately after setState in handleOnMouseDown(). Thus it kept on preventDefault. Any idea how to mutate the state immediately or I missed something?

B-Esmaili commented 4 years ago

any update on this?

fnhipster commented 4 years ago

I thought I'd give my two cents and share how I fixed this issue using a React Functional component with Hooks. The key here is to capture the click event using onClickCapture for the slider children (https://javascript.info/bubbling-and-capturing)

import React, { useState, useCallback } from 'react'
import Slick from 'react-slick'

export const Slider = ({children, ...props}) => {
  const [dragging, setDragging] = useState(false)

    const handleBeforeChange = useCallback(() => {
        console.log('handleBeforeChange')
        setDragging(true)
    }, [setDragging])

    const handleAfterChange = useCallback(() => {
        console.log('handleAfterChange')
        setDragging(false)
    }, [setDragging])

    const handleOnItemClick = useCallback(
        e => {
            console.log('handleOnItemClick')
            if (dragging) e.stopPropagation()
        },
        [dragging]
    ) 

   return (
      <Slick 
          beforeChange={handleBeforeChange}
          afterChange={handleAfterChange}
          {...props}
      >
         {React.Children.map(children, child => (
             <div onClickCapture={handleOnItemClick}>{child}</div>
         ))}
      </Slick>
  )
} 
MantasMikal commented 4 years ago

@fnhipster Thanks for sharing! Unfortunately, that did not work for me. When do you call handleAfterChange and handleBeforeChange?

fnhipster commented 4 years ago

@MantasMikal my mistake, I missed the handleAfterChange and handleBeforerChange props. I edited my comment https://github.com/akiran/react-slick/issues/848#issuecomment-597185462. I hope that works for you.

bill-bishop-code commented 4 years ago

@fnhipster that worked for me! Excellent way of solving the issue. Only issue is missing bracket on {handleBeforeChange <--- missing bracket here

Thanks!

philipp-wienes commented 4 years ago

You can just add .slick-slide img { pointer-events: none; } to your css :)

fnhipster commented 4 years ago

You can just add .slick-slide img { pointer-events: none; } to your css :)

That might work only if you don't have any clickable event in your slider. I.e., any link or button would also not be clickable.

arhoy commented 4 years ago

I feel like this is a pretty common problem. Any fix on this? My code is not working after trying the above mentioned

import React, { useState, useCallback } from 'react';
import styled from '@emotion/styled';
import Slider from 'react-slick';
import { NoStyleLink } from '../../resusableStyles/Links/LinkStyles';

const Property = styled(NoStyleLink)`
  width: 20rem;
  height: 20rem;
  & img {
    width: 100%;
    height: 100%;
    object-fit: cover;
  }
`;

export const PropertyGallery = ({ properties }) => {
  const [dragging, setDragging] = useState(false);

  const handleBeforeChange = useCallback(() => {
    console.log('handleBeforeChange');
    setDragging(true);
  }, [setDragging]);

  const handleAfterChange = useCallback(() => {
    console.log('handleAfterChange');
    setDragging(false);
  }, [setDragging]);

  const handleOnItemClick = useCallback(
    (e) => {
      console.log('handleOnItemClick');
      if (dragging) e.stopPropagation();
    },
    [dragging],
  );
  const settings = {
    dots: true,
    infinite: true,
    speed: 1000,
    autoplaySpeed: 5000,
    fadeIn: false,
    autoplay: true,
    pauseOnHover: true,
    slidesToShow: 4,
    slidesToScroll: 1,
    rows: 1,

    responsive: [
      {
        breakpoint: 1000,
        settings: {
          slidesToShow: 3,
          slidesToScroll: 1,
          infinite: true,

          rows: 1,
        },
      },
      {
        breakpoint: 600,
        settings: {
          slidesToShow: 2,
          slidesToScroll: 1,
        },
      },
      {
        breakpoint: 400,
        settings: {
          slidesToShow: 1,
          slidesToScroll: 1,
        },
      },
    ],
  };

  console.log('properties are');
  return (
    <Slider
      beforeChange={handleBeforeChange}
      afterChange={handleAfterChange}
      {...settings}
    >
      {properties.slice(0, 5).map((property, i) => (
        <Property onClickCapture={handleOnItemClick} key={i}>
          <img src={property.acf.mainimage} alt={property.acf.title} />
        </Property>
      ))}
    </Slider>
  );
};
dkadrios commented 4 years ago

I like @fnhipster's approach but found that occasionally beforeChange would fire when first mounted but without afterChange also firing. This seems to be related to the issue where initialSlide is not honored sometimes in infinite mode. Probably a deeper timing issue, but at any rate, it meant that sometimes no clicks would register until a swipe event occurred.

I'm getting around that by using the swipe event directly, which as far as I've seen always fires before the child's click event.

import React, { useCallback, useState } from 'react'
import PropTypes from 'prop-types'
import Slick from 'react-slick'

export default ({ children, ...rest }) => {
  const [swiped, setSwiped] = useState(false)

  const handleSwiped = useCallback(() => {
    setSwiped(true)
  }, [setSwiped])

  const handleOnItemClick = useCallback(
    (e) => {
      if (swiped) {
        e.stopPropagation()
        e.preventDefault()
        setSwiped(false)
      }
    },
    [swiped],
  )

  return (
    <Slick
      onSwipe={handleSwiped}
      {...rest}
    >
      {React.Children.map(children, child => (
        <div onClickCapture={handleOnItemClick}>{child}</div>
      ))}
    </Slick>
  )
}

HTH

coeing commented 4 years ago

@fnhipster Thanks for your solution! I'm a bit sad that there is no built-in way to prevent clicks on the slider content when the user dragged it (should be possible, shouldn't it?), so I built my own drag detection:

import React, { PropsWithChildren, Ref, useState } from "react";
import Slider, { Settings as SliderProps } from "react-slick";

export type DefaultSliderProps = SliderProps;

/**
 * Threshold from which mouse movement with pressed mouse button
 * is considered a drag instead of a click.
 */
const MoveDragThreshold = 10;

function useDragDetection(): {
  handleMouseDown: () => void;
  dragging: boolean;
} {
  const [mouseDown, setMouseDown] = useState(false);
  const [dragging, setDragging] = useState(false);

  useEffect(() => {
    let mouseMove = 0;

    function handleMouseUp(): void {
      setMouseDown(false);
    }

    function handleMouseMove(e: MouseEvent): void {
      mouseMove += Math.abs(e.movementX) + Math.abs(e.movementY);
      setDragging(mouseMove > MoveDragThreshold);
    }

    if (mouseDown) {
      document.addEventListener("mouseup", handleMouseUp);
      document.addEventListener("mousemove", handleMouseMove);
    }

    return () => {
      document.removeEventListener("mouseup", handleMouseUp);
      document.removeEventListener("mousemove", handleMouseMove);
    };
  }, [mouseDown]);

  function handleMouseDown(): void {
    setMouseDown(true);
    setDragging(false);
  }

  return {
    handleMouseDown,
    dragging,
  };
}

export function DefaultSlider(
  props: PropsWithChildren<DefaultSliderProps>
): React.ReactElement {
  const { children, ...sliderProps } = props;

  const {
    handleMouseDown,
    dragging,
  } = useDragDetection();

  function handleChildClick(
    e: React.MouseEvent<HTMLDivElement, MouseEvent>
  ): void {
    if (dragging) {
      e.preventDefault();
    }
  }

  return (
    <Slider {...sliderProps}>
      {React.Children.map(children, (child) => (
        <div
          onMouseDownCapture={handleMouseDown}
          onClickCapture={handleChildClick}
        >
          {child}
        </div>
      ))}
    </Slider>
  );
}

The solution from @fnhipster was already quite good, but I didn't like that it still triggered the click when the user only dragged a bit without changing the to another slide. So I introduced the MoveDragThreshold to distinguish between a click and a drag. It might be useful even outside the sliders. Hope this helps some fellow devs! :)

alirezazbr commented 3 years ago

clientYonMouseDown

Here's my solution that makes use of clientX and clientY attributes of the click and mousedown events and react component state to solve this issue.

Basically it's comparing the mouse position at mouse down and click and if either X or Y is different, then it will preventDefault() (default browser behavior being to go to the link)

class Carousel extends React.Component {
  constructor (props) {
    super(props)
    this.state = {
      clientXonMouseDown: null,
      clientYonMouseDown: null
    }
    this.settings = {
      dots: true,
      arrows: false,
      swipeToSlide: true,
      infinite: false,
    ...
    }
  }

  handleOnMouseDown (e) {
    this.setState({
      clientXonMouseDown: e.clientX,
      clientYonMouseDown: e.clientY
    })
    e.preventDefault() // stops weird link dragging effect
  }

  handleOnClick (e) {
    e.stopPropagation()
    if (this.state.clientXonMouseDown !== e.clientX || 
        this.state.clientYonMouseDown !== e.clientY) {
      // prevent link click if the element was dragged
      e.preventDefault()
    }
  }

  render () {
    const { items } = this.props
    return (
      <Slider
        {...this.settings} >
        {items.map((item, index) => (
          <a key={index} href={item.link} target='_blank'
            onMouseDown={e => this.handleOnMouseDown(e)}
            onClick={e => this.handleOnClick(e)}
            >
           Link innerText
          </a>
        ))}
      </Slider>)
  }
}

export default Carousel

Thanks, it works like a charm

jbn1981 commented 2 years ago

I'd just like to add something that helped me (not a solution, sorry). There's a setting to disable dragging (draggable: false), but I didn't want to do that because I want mobile users to be able to drag. What I didn't understand is that draggable is only for the mouse. Setting draggable: false still allows touchscreen users to swipe the slider, and this click-after-dragging issue doesn't happen with touchscreens. For desktop users I don't care if they cannot drag the slider with the mouse since I have arrows, so just setting draggable: false prevents this click issue for desktop users, and mobile users can still swipe the carousel without any issues.

Well, this approach fits 100 % my needs, so I guess it really IS a solution. Thanks a lot!

mrezaslk commented 2 years ago

Here's my solution that makes use of clientX and clientY attributes of the click and mousedown events and react component state to solve this issue.

Basically it's comparing the mouse position at mouse down and click and if either X or Y is different, then it will preventDefault() (default browser behavior being to go to the link)

class Carousel extends React.Component {
  constructor (props) {
    super(props)
    this.state = {
      clientXonMouseDown: null,
      clientYonMouseDown: null
    }
    this.settings = {
      dots: true,
      arrows: false,
      swipeToSlide: true,
      infinite: false,
    ...
    }
  }

  handleOnMouseDown (e) {
    this.setState({
      clientXonMouseDown: e.clientX,
      clientYonMouseDown: e.clientY
    })
    e.preventDefault() // stops weird link dragging effect
  }

  handleOnClick (e) {
    e.stopPropagation()
    if (this.state.clientXonMouseDown !== e.clientX || 
        this.state.clientYonMouseDown !== e.clientY) {
      // prevent link click if the element was dragged
      e.preventDefault()
    }
  }

  render () {
    const { items } = this.props
    return (
      <Slider
        {...this.settings} >
        {items.map((item, index) => (
          <a key={index} href={item.link} target='_blank'
            onMouseDown={e => this.handleOnMouseDown(e)}
            onClick={e => this.handleOnClick(e)}
            >
           Link innerText
          </a>
        ))}
      </Slider>)
  }
}

export default Carousel

thanks you its worked

CongCong-1228 commented 2 years ago

href={v.url ? v.url : "#"} target={this.checkJunp(v)} onClick={() => { return false }} it works well for me

Digirolamo commented 1 year ago

Is this the same issue as #604 ? If so, I just posed my solution there, which is a TS drop-in replacement for the Slider, if anyone is interested: https://github.com/akiran/react-slick/issues/604#issuecomment-1364202362

Lakshay-art commented 1 year ago

href={v.url ? v.url : "#"} target={this.checkJunp(v)} onClick={() => { return false }} it works well for me

what to if i want to trigger a function instead?

raubreak commented 7 months ago
$('body').on('swipe', '.slick-slider', function(event, slick, direction){
    event.stopPropagation();
});