davidjerleke / embla-carousel

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

slidesInView returns one too many slides #650

Closed andyford closed 6 months ago

andyford commented 6 months ago

Bug is related to

Embla Carousel version

Describe the bug

The array of returned indexes always returns 1 more than expected (for example it returns [4, 5, 6, 7, 8] when only [4, 5, 6, 7] are shown. It seems the "extra" index is always at the end of the array

CodeSandbox

- The link to a CodeSandbox that demonstrates the bug clearly.

Steps to reproduce

View console log for the array reported on init and each time the slidesInView event is fired (any time the slideshow is changed or resized)

Expected behavior

in the given example showing one slide at a time, I would expect only one slide index in the array reported by the .slidesInView() method rather than 1+1

Additional context

It seems that .slidesNotInView() "agrees" with .slidesInView() in that there's on extra slide missing from the "not in view" list. It also seems that the 'slidesInView' event fires twice and on the first firing the array reported by the .slidesInView() method is correct, but on the second firing, the array has the extra index

davidjerleke commented 6 months ago

@andyford thank you for your bug report.

Before we can determine that this is a bug or not, I would encourage you to set your own IntersectionObserver on the container parent element (emblaApi.containerNode().parentElement) and see what results you get. Because Embla is using IntersectionObserver under the hood and even though you "think" that slide shouldn't be visible, it might be included by the browser because a half pixel of that slide actually IS visible.

IntersectionObserver has an option called threshold which can be passed to Embla using the inViewThreshold option. Setting that option to anything higher than 0 (which is default) should exclude the slide you mention in your bug description.

Best, David

andyford commented 6 months ago

@davidjerleke thanks for the quick reply! I see what you mean. I tried setting the inViewThreshold option to a high number like .95 and now I see results I would expect. https://codesandbox.io/p/sandbox/embla-carousel-arrow-dots-react-forked-dh5k4j

so I get how and why it works this way, and I appreciate that the docs say "Honors the inViewThreshold option." but I think most authors would still expect the length of the slidesInView method to match the number of slidesToScroll. Perhaps all that's needed here is a bit more elaboration in the slidesInView and slidesNotInView docs to explain the relationship to inViewThreshold

davidjerleke commented 6 months ago

but I think most authors would still expect the length of the slidesInView method to match the number of slidesToScroll.

@andyford this is not always the case. You can set slidesToScroll to "auto" and Embla will group slides for you depending on how many slides fit within the viewport. So let's say your slides are 40% wide. When the carousel is mounted, slide 1, 2 and 3 will be visible (slidesInView), but Embla can only group slide 1 and 2 because 3 won't fit (3 * 40% = 120%) which means wider than the viewport. So slidesInView and slidesToScroll have to be separated. See this CodeSandbox.

Feel free to contribute and add explanations to the docs if you think stuff is missing or not explained well. I'm the only dev maintaining this project so unfortunately I don't get the chance to do everything.

Best, David