React-slick: Slider content should not be clickable while the user is dragging the slides on Desktop.

Created on 19 Jan 2017  ·  26Comments  ·  Source: akiran/react-slick

I replicated the issue here: https://jsfiddle.net/20bumb4g/1988/

All pictures were wrapped into a link which "would" link to an external page. (google at this example)

If you start dragging any slide without changing the slide after releasing the click (no index update!) the event will be propagated down to the inner content. This makes dragging act differently and unwanted on desktop.

Please fix this issue. Btw.: your library is awesome.

queued

Most helpful comment

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

export default class Carousel extends React.Component {
  swiping = false;
  carouselRef = React.createRef();

  handleMouseDown = event => {
    console.log('handleMouseDown');
    event.preventDefault();
  };

  handleMouseUp = () => {
    console.log('handleMouseUp');
    console.log(`swiping: ${this.carouselRef.current.innerSlider.state.swiping}`);
    this.swiping = this.carouselRef.current.innerSlider.state.swiping;
  };

  handleClick = event => {
    console.log('handleClick');
    console.log(`swiping: ${this.swiping}`);
    if (this.swiping) {
      event.preventDefault();
    }
  };

  /**
   * @see React.Component.render
   * @return {JSX}
   */
  render() {
    const { children } = this.props;
    const settings = {
      arrows: true,
      slidesToShow: 2,
      slidesToScroll: 2,
      speed: 400,
      dots: true,
      infinite: false,
      variableWidth: true,
      nextArrow: <ArrowButton next />,
      prevArrow: <ArrowButton prev />,
      dotsClass: 'slick-dots',
      responsive: [
        {
          breakpoint: 672,
          settings: {
            arrows: false,
            dots: false,
            slidesToShow: 1,
            slidesToScroll: 1,
            swipeToSlide: true,
          },
        },
      ],
    };
    return (
      <div className={classes.carousel__wrapper}>
        <Slider className={cx(classes.Slick)} ref={this.carouselRef} {...settings}>
          {React.Children.map(children, child => (
            <div onClickCapture={this.handleClick} onMouseUpCapture={this.handleMouseUp} onMouseDownCapture={this.handleMouseDown}>
              {child}
            </div>
          ))}
        </Slider>
      </div>
    );
  }
}

All 26 comments

@akiran Please solve this problem

Maybe you could pass the event through to the beforeChange callback so that we can call stopPropagation on it?

@adamshone Good idea, but I found that still doesn't work as the beforeChange accepts only two parameters currentSlide and nextSlide.

Sorry, by "you" I was referring to the project authors, I thought they could add the event as a third param to beforeChange.

Having had a very quick scan of the code I think unfortunately it's not that simple, it looks like it's using some animationEnd callbacks rather than listening to any mouse event that could be passed on to the user :/

I tried using the beforeChange and afterChange event handlers to toggle a flag this.setState({ignoreClicks: true/false}), and pass state.ignoreClicks to the carousel items as a prop.

Unfortunately triggering a render from the beforeChange event seems to break the carousel, it cuts the animation off and seems to reset some internal state so that afterChange is never fired.

So... I have a workaround but it's very hacky - you can use it if you're desperate:

import Carousel from 'react-slick';

class MyCarousel extends PureComponent {
  midDrag = false;

  toggleMidDrag = () => {
    this.midDrag = !this.midDrag;
  }

  render() {
    return (
      <Carousel beforeChange={this.toggleMidDrag} afterChange={this.toggleMidDrag}>
        <Item key="0" parentCarousel={this} />
        <Item key="1" parentCarousel={this} />
        ...
      />
    );
}

class Item extends PureComponent {
  onClick = () => {
    if (this.props.parentCarousel.midDrag) {
      // ignore clicks
      return;
    }

    // ...your usual click handling code
  }

  render() { ... }
}

@akiran please, this is critical. I think the easier solution is to stop the event propagation on dragEnd in the event handler mixin.

@akiran any update's on this issue ?

@akiran this feature is critically necessary, please update it!

@fdc-viktor-luft did you found a solution or another library? thanks!

@skumblue I decided to avoid any bugs on cost of comfort. Therefore I disabled Drag-&-Drop on Desktop by setting "draggable" to "false". Hint: The property "draggable" influences only Desktop behaviour.

In case that this issues gets cleanly solved, I will consider an update.

You can try something like this:

class Carousel extends Component {
  constructor(props) {
    super(props);
    this.state = {isDragging: false};
  }

  shouldComponentUpdate(nextProps, nextState) {
    return this.state.isDragging === nextState.isDragging
  }

  onClickFunction = (e) => {
    if (this.state.isDragging) {
      e.preventDefault()
    }
  };

  render() {
    const settings = {
      ... // the rest of the settings
      beforeChange: () => this.setState({isDragging: true}),
      afterChange: () => this.setState({isDragging: false})
    };

    return (
      <div>
        <Slider {...settings}>
            <div><a href="#" onClick={this.onClickFunction}>one</a></div>
            <div><a href="#" onClick={this.onClickFunction}>two</a></div>
            <div><a href="#" onClick={this.onClickFunction}>three</a></div>
        </Slider>
      </div>
    )
  }
}

I have implemented a similar fix to the one above for this issue. Works perfectly.

I did something similar to @trumko but I set isDragging as a property on the class rather than in state so changing it never causes a re-render. shouldComponentUpdate was causing some weird glitching still

It seems to be almost perfect with these hacks, although the slides need to slide completely off the screen, as in entire slide width, in order to prevent default click. If you just slide it around slightly, it will click anyway, since beforeChange and afterChange don't event trigger that way 😞

The referenced commit should solve the mentioned problem. The changes will be released soon.

@laveesingh

Can this feature (from 7918201) be release as of now?

Looks like this commit partly breaks carousel functionality on mobile devices.
because of

clickHandler = e => {
  if (this.clickable === false) {
    e.stopPropagation()
    e.preventDefault()
  }
  this.clickable = true
}

now after swiping user must tap 2 times on slider to trigger link.

The bug still can be reproduced if I finish dragging on the slide.
This happens because clickHandler fires after swipeMove is finished.
Mac OS, Chrome 69

Is there any update on this issue?

This can be fixed if you don't mind changing some internals of <InnerSlider>

onSwipe() {
    if (this.slider) {
      this.slider.innerSlider.clickable = true;
    }
  }

settings = {
    onSwipe: this.onSwipe.bind(this),
    swipeToSlide: true,
  };

// in render()
const sliderRef = slider => {this.slider = slider;};

// in return
<Slider {...settings} ref={sliderRef}>{this.slides}</Slider>

This can be fixed if you don't mind changing some internals of <InnerSlider>

onSwipe() {
    if (this.slider) {
      this.slider.innerSlider.clickable = true;
    }
  }

settings = {
    onSwipe: this.onSwipe.bind(this),
    swipeToSlide: true,
  };

// in render()
const sliderRef = slider => {this.slider = slider;};

// in return
<Slider {...settings} ref={sliderRef}>{this.slides}</Slider>

This doesn't work for me. this.slider.innerSlider is undefined and it's still triggering the onClick on the slides

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

export default class Carousel extends React.Component {
  swiping = false;
  carouselRef = React.createRef();

  handleMouseDown = event => {
    console.log('handleMouseDown');
    event.preventDefault();
  };

  handleMouseUp = () => {
    console.log('handleMouseUp');
    console.log(`swiping: ${this.carouselRef.current.innerSlider.state.swiping}`);
    this.swiping = this.carouselRef.current.innerSlider.state.swiping;
  };

  handleClick = event => {
    console.log('handleClick');
    console.log(`swiping: ${this.swiping}`);
    if (this.swiping) {
      event.preventDefault();
    }
  };

  /**
   * @see React.Component.render
   * @return {JSX}
   */
  render() {
    const { children } = this.props;
    const settings = {
      arrows: true,
      slidesToShow: 2,
      slidesToScroll: 2,
      speed: 400,
      dots: true,
      infinite: false,
      variableWidth: true,
      nextArrow: <ArrowButton next />,
      prevArrow: <ArrowButton prev />,
      dotsClass: 'slick-dots',
      responsive: [
        {
          breakpoint: 672,
          settings: {
            arrows: false,
            dots: false,
            slidesToShow: 1,
            slidesToScroll: 1,
            swipeToSlide: true,
          },
        },
      ],
    };
    return (
      <div className={classes.carousel__wrapper}>
        <Slider className={cx(classes.Slick)} ref={this.carouselRef} {...settings}>
          {React.Children.map(children, child => (
            <div onClickCapture={this.handleClick} onMouseUpCapture={this.handleMouseUp} onMouseDownCapture={this.handleMouseDown}>
              {child}
            </div>
          ))}
        </Slider>
      </div>
    );
  }
}

You can try something like this:

class Carousel extends Component {
  constructor(props) {
    super(props);
    this.state = {isDragging: false};
  }

  shouldComponentUpdate(nextProps, nextState) {
    return this.state.isDragging === nextState.isDragging
  }

  onClickFunction = (e) => {
    if (this.state.isDragging) {
      e.preventDefault()
    }
  };

  render() {
    const settings = {
      ... // the rest of the settings
      beforeChange: () => this.setState({isDragging: true}),
      afterChange: () => this.setState({isDragging: false})
    };

    return (
      <div>
        <Slider {...settings}>
            <div><a href="#" onClick={this.onClickFunction}>one</a></div>
            <div><a href="#" onClick={this.onClickFunction}>two</a></div>
            <div><a href="#" onClick={this.onClickFunction}>three</a></div>
        </Slider>
      </div>
    )
  }
}

yes,you are right,thank you very much.

您可以尝试如下操作:

class Carousel extends Component {
  constructor(props) {
    super(props);
    this.state = {isDragging: false};
  }

  shouldComponentUpdate(nextProps, nextState) {
    return this.state.isDragging === nextState.isDragging
  }

  onClickFunction = (e) => {
    if (this.state.isDragging) {
      e.preventDefault()
    }
  };

  render() {
    const settings = {
      ... // the rest of the settings
      beforeChange: () => this.setState({isDragging: true}),
      afterChange: () => this.setState({isDragging: false})
    };

    return (
      <div>
        <Slider {...settings}>
            <div><a href="#" onClick={this.onClickFunction}>one</a></div>
            <div><a href="#" onClick={this.onClickFunction}>two</a></div>
            <div><a href="#" onClick={this.onClickFunction}>three</a></div>
        </Slider>
      </div>
    )
  }
}

是的,你是对的,非常感谢。

no.when drag too much time,it's out of work

I do still have the same problem. Any help is much appreciated.

`import React, { Component } from 'react';
import PropTypes from 'prop-types';
import SlickSlider from 'react-slick';
import _throttle from 'lodash.throttle';
import _isEmpty from 'lodash.isempty';
import key from 'weak-key';

// components
import SlideImage from './SlideImage';
import SlideVideo from './SlideVideo';
import useTracking from '../../../utils/hooks/useTracking';
import { sliderClick } from './trackingActions';

class Slider extends Component {
constructor(props) {
super(props);

this.slider = null;
this.handleResize = _throttle(this.handleResize.bind(this), 50);
this.handlePlay = this.handlePlay.bind(this);
this.handleCTAButtonClick = this.handleCTAButtonClick.bind(this);

this.state = {
  activePlayer: null,
};

}

componentDidMount() {
if (typeof window !== 'undefined') {
window.addEventListener('resize', this.handleResize);
}
}

componentDidUpdate(prevProps) {
const { language, content } = this.props;

if (language !== prevProps.language) {
  // update slick slider after language change to get first slide and the right text color
  if (this.slider) {
    const items = content.items ? content.items : content;
    this.setSliderTextColor(items[0].textColor);
  }
}

}

componentWillUnmount() {
if (typeof window !== 'undefined') {
window.removeEventListener('resize', this.handleResize);
}
}

handleCTAButtonClick(text) {
const track = useTracking();
track.trackEvent(sliderClick(text));
}

/**

  • set the slider color class regarding textColor attribute
    *
  • @param textColor
    */
    setSliderTextColor(textColor) {
    if (textColor === 'white') {
    this.sliderContainer.classList.add('slider-container--color-white');
    } else {
    this.sliderContainer.classList.remove('slider-container--color-white');
    }
    }

handleResize() {
const playerNode = document.querySelector('.mi-content');

if (playerNode) {
  const videoContentWidth = playerNode.offsetWidth;

  Array.from(document.querySelectorAll('.mi-controls')).forEach((element) => {
    // eslint-disable-next-line no-param-reassign
    element.style.width = `${videoContentWidth}px`;
  });
}

}

handlePlay(player) {
const { slideDuration } = this.props;

this.setState({
  activePlayer: player,
});

this.handleResize();

if (slideDuration > 0 && this.slider) {
  this.slider.slickPause();
}

}

handleStop() {
const { slideDuration } = this.props;

if (slideDuration > 0 && this.slider) {
  this.slider.slickPlay();
}

}

renderSlides() {
const { catalogName, sliderType, sliderContext, ...content } = this.props;
const items = content.items ? content.items : content;

return items.map((slide, index) => {
  if (slide.video && slide.video.videoId && sliderType !== 'hero-tile') {
    return (
      <SlideVideo
        key={`${key(slide)}-video`}
        slide={slide}
        onPlay={this.handlePlay}
        onPause={this.handleStop}
        catalogName={catalogName}
        index={index}
        type={sliderType}
        context={sliderContext}
        sliderOnClickCTA={this.handleCTAButtonClick}
      />
    );
  }
  return (
    <SlideImage
      key={`${key(slide)}-image`}
      slide={slide}
      catalogName={catalogName}
      index={index}
      type={sliderType}
      context={sliderContext}
      sliderOnClickCTA={this.handleCTAButtonClick}
    />
  );
});

}

renderSlider() {
const { activePlayer } = this.state;
const { sliderType, slideDuration, ...content } = this.props;
const items = content.items ? content.items : content;

const slickSettings = {
  dots: true,
  arrows: sliderType !== 'hero-tile',
  beforeChange: (currentSlide, nextSlide) => {
    if (activePlayer) {
      activePlayer.pause();
    }

    const nextSlideItem = items[nextSlide];

    // should not be triggered on language change because nextSlide is not the right one
    if (this.sliderContainer) {
      this.setSliderTextColor(nextSlideItem.textColor);
    }
  },
  responsive: [
    {
      breakpoint: 992, // Breakpoint "large"
      settings: {
        arrows: false,
      },
    },
  ],
};

if (slideDuration > 0) {
  slickSettings.autoplay = true;
  slickSettings.autoplaySpeed = slideDuration;
}

return (
  <SlickSlider
    ref={(slider) => {
      this.slider = slider;
    }}
    {...slickSettings}
  >
    {this.renderSlides()}
  </SlickSlider>
);

}

renderSingleSlide() {
return (





{this.renderSlides()}





);
}

render() {
const { sliderType, items } = this.props;
if (!items || _isEmpty(items)) {
return null;
}

let styleName = '';
if (items[0] && Object.prototype.hasOwnProperty.call(items[0], 'textColor')) {
  styleName = items[0].textColor === 'white' ? ' slider-container--color-white' : '';
}

const slider = (
  <div
    className={`slider-container slider-container--${sliderType}${styleName}`}
    ref={(sliderContainer) => {
      this.sliderContainer = sliderContainer;
    }}
  >
    {items.length === 1 && this.renderSingleSlide()}
    {items.length > 1 && this.renderSlider()}
  </div>
);

return slider;

}
}

Slider.propTypes = {
language: PropTypes.string,
content: PropTypes.objectOf(PropTypes.any),
sliderType: PropTypes.string,
sliderContext: PropTypes.string,
catalogName: PropTypes.string,
slideDuration: PropTypes.number,
items: PropTypes.arrayOf(PropTypes.object),
};

Slider.defaultProps = {
language: null,
content: null,
items: null,
sliderType: 'small', // or homepage or hero-tile
catalogName: 'st_heroSlider',
sliderContext: 'default',
slideDuration: 6000,
};

export default Slider;
`

It's been a long time ago, that I reported this issue. It's pretty sad, that the problem still exists. I recently decided to try it on my own. You can have a look at my library, which I already used many times, but it is much less customizable.

Differences:

  • Dragging can be disabled but works otherwise on all device types the similar
  • Clicking works only when not dragging
  • the package is much smaller (1.8 kB gzipped instead of 14.5 kB)
  • peer dependency on react > 16.8 (because hook based)
  • TypeScript support included

Here's a demo

And here you can find further information

But let me summarize: Maybe there are better solutions already out there :v:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ramyatrouny picture ramyatrouny  ·  3Comments

darkalor picture darkalor  ·  4Comments

rohitgoyal7 picture rohitgoyal7  ·  3Comments

quarklemotion picture quarklemotion  ·  4Comments

laveesingh picture laveesingh  ·  3Comments