nuka-carousel icon indicating copy to clipboard operation
nuka-carousel copied to clipboard

autoplay controlled prop is conflicting with pauseOnHover

Open lerayne opened this issue 5 years ago • 2 comments

Bugs and Questions

Describe Your Environment

  • What version of nuka-carousel are you using? 4.5.5 (note: I was using v4.4.8 but I've looked in the current code and this issue's causes still remain)
  • What version of React are you using? 16.8.2
  • What browser are you using? Chrome 74
  • What machine are you on? MacBook Pro 2015

Describe the Problem

There is no safeguard in code against running setInterval in "startAutoplay" twice.

This results in a possibility to set two intervals, only one of which can be cleared, in this case at some point slider starts to switch slides twice or even more times in a row and nothing can stop it.

How to reproduce it? For example, you can have a slider with "autoplay" prop which is controlled from above AND pauseOnHover set to "true". In this case you can change the prop (startAutoplay will be ran by componentWillReceiveProps) and then trigger "mouseOut" which will run startAutoplay by handleMouseOut -> unpauseAutoplay. Voila, we have 2 intervals running at the same time.

My fix suggestion is simple:

startAutoplay() {
  if (!this.autoplayID) {
    this.autoplayID = setInterval(
      this.autoplayIterator,
      this.props.autoplayInterval
    );
  }
}

stopAutoplay() {
  if (this.autoplayID) {
    clearInterval(this.autoplayID);
    this.autoplayID = 0
  }
}

lerayne avatar May 13 '19 13:05 lerayne

hi @lerayne with autoplay and pauseOnHover set to true, I am unable to reproduce this issue. I see the slides pause correctly when hovering over the slide, and when I move the mouse away, the slides continue to scroll in proper increments. Are there more parameters in place that are causing the issue?

Thanks!

sarmeyer avatar May 13 '19 16:05 sarmeyer

This bug was pretty difficult to reproduce, it was happening once in a 10-20 interactions. Maybe because "pauseOnHover" feature is based on mouseIn/mouseOut events rather than mouseEnter/mouseLeave, so when you move the cursor over the slide, pause and unpause events happen multiple times and for this bug to happen you have to catch the right movement sequence. But that's enough to be critical bug for production. I think making a safeguard here is worth the risks.

I had breakpoints there and I'm sure no other parameters are involved. The only thing that could affect the case is that my slider was in an iframed widget, so maybe mouseIn/Out events were sometimes happening in a wrong sequence when mouse was leaving the iframe.

But my main point is that according to the code nothing prevents startAutoplay from being called twice before stopAutoplay happens. It can happen in some other manipulations coincidence.

lerayne avatar May 14 '19 11:05 lerayne

We don't use setInterval for autoplay any more, so this is unlikely to come up.

fritz-c avatar Aug 18 '22 18:08 fritz-c