akiran / react-slick

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

After swiping on mobile device, the slide should be clicked twice to trigger link in it #1298

Open miartysh-cartera opened 6 years ago

miartysh-cartera commented 6 years ago

Mobile and tablet users are forced to tap two times to trigger link, after carousel swiping. Looks like this issue appeared after fixing this problem

lm2almeida commented 5 years ago

Any update?

fmoulton commented 5 years ago

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>
schuylr commented 5 years ago

I tried implementing the workaround but it still doesn't seem to work on a true mobile device. Is there any timeline for a fix on this?

fmoulton commented 5 years ago

@schuylr what type of mobile device does it not work on for you?

schuylr commented 5 years ago

@fmoulton latest Chrome on Android for me, and I believe my colleague is using iOS + latest Safari.

fmoulton commented 5 years ago

@schuylr I just tested it on 4 different browsers on my iOS device and works as expected. See: https://www.nerdwallet.com/search/results?q=Chase&p=0&f=%7B%7D

schuylr commented 5 years ago

@fmoulton Here's our site https://www.jewlr.com

I just checked and we're running 0.23.1 - I'm wondering if 0.23.2 will make a difference. Will report back.

schuylr commented 5 years ago

Confirmed on my development environment, running with 0.23.2 still doesn't solve the issue. Can you explain how the workaround works? Might need a little context to understand if there's something I'm missing on my own code. Here's a snippet:

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

  render() {
    let settings = {
      autoplay: false, // this.state.autoplay,
      autoplaySpeed: 5000,
      dots: !!this.props.showDots || false,
      dotsClass: `home__slider-dots  ${
        this.props.showDots === "mobileOnly" ? "is-hidden-md-up" : ""
      }`,
      infinite: true,
      nextArrow: (
        <NextArrow placement={this.props.arrowPlacement || "inside"} />
      ),
      prevArrow: (
        <PrevArrow placement={this.props.arrowPlacement || "inside"} />
      ),
      slidesToShow: this.props.slidesToShow || 1,
      slidestoScroll: 1,
      responsive: [
        {
          breakpoint: 767,
          settings: {
            slidesToShow: 1
          }
        }
      ],
      touchThreshold: 20,
      onSwipe: this.onSwipe.bind(this),
      swipeToSlide: true
    }

    const sliderRef = slider => {this.slider = slider}

    return <Slider {...settings} ref={sliderRef}>{this.props.children}</Slider>
  }
fmoulton commented 5 years ago

Your slider links work for me on iOS but I don’t have an android device for testing.

bozhouyongqi commented 5 years ago

thanks for @fmoulton,your method is worked for me.This is a bug in their code when the image was slidden manually. the following is the swipeMove callback function:

swipeMove = e => {
    let state = swipeMove(e, {
      ...this.props,
      ...this.state,
      trackRef: this.track,
      listRef: this.list,
      slideIndex: this.state.currentSlide
    });
    if (!state) return;
    if (state["swiping"]) {
      this.clickable = false;
    }
    this.setState(state);
  };

and then the following is the clickHandler:

clickHandler = e => {

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

so we should click the image twice ,then it could respond the click event.

Kevinpahlevi commented 5 years ago

@fmoulton ur solution work for me when just one slider in page , and how when in one page have 2 slider, i try it and didnt work i think cause this
const sliderRef = slider => {this.slider = slider}

(edit) i found solution for this just change variabel for different slider like : const sliderRef = slider => {this.slider1 = slider} const sliderRef = slider => {this.slider2 = slider} and this work for me using 2 slider in one page , no double tap in mobile

liro commented 5 years ago

@akiran hello, could you add this line: this.clickable = true; in swipeEnd function of inner-slider.js?

swipeEnd = e => {
    let state = swipeEnd(e, {
      ...this.props,
      ...this.state,
      trackRef: this.track,
      listRef: this.list,
      slideIndex: this.state.currentSlide
    });
    this.clickable = true;
    if (!state) return;
    let triggerSlideHandler = state["triggerSlideHandler"];
    delete state["triggerSlideHandler"];
    this.setState(state);
    if (triggerSlideHandler === undefined) return;
    this.slideHandler(triggerSlideHandler);
    if (this.props.verticalSwiping) {
      this.enableBodyScroll();
    }
  };

This would reset clickable state after swipe ends and seems everything works correctly.

zekchan commented 5 years ago

Any updates?

uberman4740 commented 5 years ago

Same issue https://codesandbox.io/s/react-slick-playground-lbbge

After swiping the tiles, clicking on a slide should change the link on the address bar. But it does not happen. It requires two taps to change the link. Please try it multiple times since the bug is random.

emotinator commented 5 years ago

Just adding a crappy work-around until this is properly fixed, but if you use createRef to your slider, you can use the afterChange to re-enable the clickable property. Ugly fix, but should hold me over til the proper fix liro suggested.

afterChange: n => { if (thisSwiper.current && thisSwiper.current.innerSlider) { thisSwiper.current.innerSlider.clickable = true } },

Malekz commented 4 years ago

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 has solved my bug. Thank you

cburgosro9303 commented 4 years ago

Hi! my english is not good, but I'm using 0.25.2 and this issue isn't fix, somebody can't help me with this. I can't modify <InnerSlider> cause project rules, What can I do?