search

davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
5.76k stars 174 forks source link

[Bug]: canScrollPrev() and canScrollNext() struggle with dragFree: true #938

Closed kbachl closed 2 weeks ago

kbachl commented 1 month ago

Which variants of Embla Carousel are you using?

Steps to reproduce

See this Sandbox, generated from the Embla Generator with dragFree, 33% and Group Slides: https://codesandbox.io/s/jy5xck?file=/index.html If you open it up in Firefox or Chrome and drag exactly 1 slide, the "Next" button gets deactivated, yet we only see slides 2 to 4. If we go to Slide 5 and scroll back slowly to see slides 2 to 4 it happens with the prev. It seems that if the remaining slides are lower than the slides you usually scroll (here 3) it gets greyed out, yet there are hidden slides left that can only be reached by drag then.

Expected Behavior

Prev and Next need to be active till they are at the END or the START slides, e.g. index = 0 or index = maxIndex

Additional Context

I have also tried...

What browsers are you seeing the problem on?

No response

Version

8.1.6

CodeSandbox

https://codesandbox.io/s/jy5xck

Before submitting

davidjerleke commented 1 month ago

Hi @kbachl,

Thanks for your bug report. This is by design and I'm going to try to explain why. When you group slides in your CodeSandbox example, Embla treats every group of slides as if it were a single slide. In your case:

slide-1 slide-2

Your setup has two scroll snaps because you group your slides:

The canScrollPrev() and canScrollNext() methods are purely based on scroll snap index and not if the carousel has scrolled to any end or not, meaning fully to the left or right end. Nor is it based on what slides are visible or not. Any time you scroll the carousel with drag free, canScrollPrev() and canScrollNext() will check if the current carousel position is closest to snap A or B. If the position where the carousel stops after dragging is closest to snap B, canScrollPrev() will return true. If it's closest to snap A, canScrollNext() will return true.

Of course, one could argue that canScrollPrev() and canScrollNext() should be purely based on if the carousel has scrolled to any end or not. But in that case, it's a feature/change request. Not a bug. I could create a poll for devs to vote about this change, but the problem I have is that I've created similar polls about features before, and after a year has gone by, I've only gathered like 35 votes or something. It's not much if we were to make a decision based on that.

With that said, you can use the emblaApi.internalEngine() to create your own canScrollPrev() and canScrollNext() methods to achieve what you want.

Best, David

davidjerleke commented 1 month ago

@kbachl have you read my comment?

kbachl commented 1 month ago

Hi @davidjerleke, sorry for beeing late :)

I understand what you mean and why you dont see it as a bug, yet I'm still a bit troubled because it's - simply spoken - code generated by your carousel generator that is not working as one would expect it.

I haven't had time to dig into emblaApi.internalEngine() since I usually try to stay miles away from "internal" parts of libraries, but I at least would like to have this issue added to the documentation. In my point of view code that gets generated by a promoted generator has to work like the end-user expects it.

And thanks for taking your time on helping me here and I really must admit that the current embla carousel is way better than the "keen-slider" it has replaced. Very smoth and with the way it gets integrated exactly 0 CLS.

davidjerleke commented 2 weeks ago

@kbachl thanks for your response.

I understand what you mean and why you dont see it as a bug, yet I'm still a bit troubled because it's - simply spoken - code generated by your carousel generator that is not working as one would expect it.

When you say "is not working as one would expect it" we should be clear that it's how you expect it to work and this might not apply to everyone else. The current behavior is by design and expected. It's not a bug. As I explained in my previous comment, the canScrollPrev() and canScrollNext() methods are purely based on scroll snap index and not if the carousel has scrolled to any end or not, meaning fully to the left or right end.

Embla has been around for a long time, even before keen-slider and this is the first time someone wants to change the current behavior. I can't change the carousel behavior based on a single developers opinion but I can promise this: If a lot of devs come here and give your issue thumbs up and complain about this, I will consider changing the internal behavior. But until then, I've put together a canScrollBackward() and canScrollForward() function in this CodeSandbox that does what you want.

Best, David