FormidableLabs / nuka-carousel

Small, fast, and accessibility-first React carousel library with an easily customizable UI and behavior to fit your brand and site.
https://commerce.nearform.com/open-source/nuka-carousel
Other
3.02k stars 597 forks source link

`disableEdgeSwiping` prevents successive forward swipes #942

Closed mpedersen15 closed 1 year ago

mpedersen15 commented 1 year ago

Bugs and Questions

Prerequisites

Feel free to delete this section if you have checked off all of the following.

Describe Your Environment

Describe the Problem

It seems like the disableEdgeSwiping is preventing successive swiping forward after the first swipe forward. Swiping backwards works successively however.

I've produced a minimal reproduction at this codesandbox

Expected behavior: [What you expect to happen] In a carousel with 3 or more slides, after swiping from slide 1 to slide 2, I should be able to immediately swipe from slide 2 to slide 3.

Actual behavior: [What actually happens] In a carousel with 3 or more slides, after swiping from slide 1 to slide 2, I am not able to continue swiping forward. Swiping backwards and using the dots or side "Prev"/"Next" buttons works as expected (and is the only way to navigate to slide 3).

Additional Information

The bug was NOT present in nuka-carousel v4.7.4 (the previous version I was using in my project)

Thank you all for an awesome carousel component! If I missed anything on this issue, I apologize and will fix it!

fritz-c commented 1 year ago

Thank you for creating the issue and the minimal reproduction sandbox. Indeed, this does look like a bug. We have a few yet-unreleased bugfixes that can be seen in action on the demo site.

Does this better represent the behavior you expect?

It does not match your described "expected behavior" in that it avoids showing whitespace to the right and therefore only has two locations it can scroll through to start with, but the UI is at least consistent in its treatment of where the end of the carousel is.

This change was made to be more consistent with the treatment of the slidesToScroll > 1 && slidesToShow > 1 case, where it sought to avoid showing whitespace at the end (demo of this with the existing library version, based on your sandbox).

We don't have a concrete date in mind for releasing the next version, but just let me know if this fix is what you had in mind.

mpedersen15 commented 1 year ago

Thank you for the thorough explanation @fritz-c ! I understand now that the lack of scrolling in my demo/sandbox was due to the last slide in the carousel already being visible. I do think that the demo site version (only show TWO dot indicators) is a step in the right direction! Showing more dot indicators than are scrollable (as in my sandbox) is definitely confusing.

fritz-c commented 1 year ago

This is now fixed in v5.3.0