Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: autoplay interval bug #253

Merged
merged 3 commits into from
Mar 17, 2020
Merged

fix: autoplay interval bug #253

merged 3 commits into from
Mar 17, 2020

Conversation

ZLevine
Copy link
Contributor

@ZLevine ZLevine commented Mar 16, 2020

What:

Fixes #252

Automatically pause "autoplay" (isPlaying) when the user changes slides using one of these HOC control components:

  • <ButtonNext />
  • <ButtonBack />
  • <ButtonFirst />
  • <ButtonLast />
  • <Dot />

This mirrors the existing behavior where autoplay is paused when the user interacts with the Slider (i.e. drags on mobile).

Why:

Currently, the isPlaying interval timer doesn't reset when the user visits a slide using one of these buttons. The end result (as defined in #252) is that a user could progress to a slide and then immediately be progressed to another slide. I don't see why the user would EVER want this to happen.

How:

Set isPlaying: false in the CarouselProvider state in the onClick handler for each of the aforementioned HOCs.

Checklist:

  • Documentation added/updated (N/A)
  • Typescript definitions updated (N/A)
  • Tests added and passing
  • Ready to be merged

I meant to create a bugfix/* branch in my repo to merge in but accidentally committed to Master instead; NBD.

@tim-steele
Copy link
Contributor

@ZLevine just so I understand from what I am reading: If a user clicks the back, next, or dot button - the timer isn't reset, it just pauses autoplay?

@ZLevine
Copy link
Contributor Author

ZLevine commented Mar 17, 2020

Correct—this mirrors the existing behavior where it pauses autoplay when a user swipes to a new slide on mobile or otherwise interacts with the slider.

I think this makes sense because, if the user is using one of the navigation arrows or clicking individual slides, they probably want to read and control the slider without it progressing on them.

@tim-steele
Copy link
Contributor

@allcontributors please add @ZLevine for code

@allcontributors
Copy link
Contributor

@tim-steele

I've put up a pull request to add @ZLevine! 🎉

@tim-steele tim-steele merged commit 0e57068 into express-labs:master Mar 17, 2020
@bcarroll22
Copy link
Contributor

🎉 This PR is included in version 1.26.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Andrew-Jang
Copy link

Andrew-Jang commented May 8, 2023

I think this merge resulted a minor bug #314 . Is there a workaround?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Autoplay doesn't pause (or reset interval) when slides change via a button click
4 participants