Skip to content

feature: infinite scroll works with swiping as well as buttons #204

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

Merged
merged 1 commit into from
Nov 13, 2019

Conversation

mariohoyos92
Copy link
Contributor

@mariohoyos92 mariohoyos92 commented Nov 12, 2019

What:

There was a recent addition to allow infinite scrolling with the buttons and with autoplay, but this isn't currently happening when a user swipes at the start or end of the carousel. This PR changes that.

Why:

When a user is swiping through pictures and hits the end of the carousel, it is cumbersome to have to swipe back through all of the photos instead of continuing to swipe forward.

How:

Modifying the computeCurrentSlide to account for when infinite is set to true.

Checklist:

  • Documentation added/updated
  • Typescript definitions updated
  • Tests added and passing
  • Ready to be merged

Love what the team is doing with this! Thank you for the great package!

Nov-12-2019 15-16-56

@bcarroll22
Copy link
Contributor

This looks good to me, thanks @mariohoyos92!

@tim-steele or @mrbinky3000 would you like to take a look at this before we merge it?

@allcontributors please add @mariohoyos92 for code

@allcontributors
Copy link
Contributor

@bcarroll22

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

@mrbinky3000
Copy link
Collaborator

mrbinky3000 commented Nov 13, 2019

How does everyone feel about this as a user experience? Swipe right and it jumps all the way back to the beginning of the slide show? Feels like a bug. I would like this better if I could turn this on/off for swipe. My gut says someone will complain about this in the future.

@tim-steele tim-steele merged commit a9d8c39 into express-labs:master Nov 13, 2019
@tim-steele
Copy link
Contributor

Thanks @mariohoyos92 - we published [email protected] with the update.

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

Successfully merging this pull request may close these issues.

4 participants