Wrong active dots calculation

[pierzchalatomasz]

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior:

1. Create a carousel with 5 items
2. Pass following props:

<Carousel
  ...
  dots
  slidesPerPage={2}
  slidesPerScroll={2}
>

3. Keep changing active slides
4. Notice the order of active dots (it's repeatable 1, 3, 5, 2, 4)

Expected behavior Probably active dots should be calculated differently for this kind of situation.

Screenshots

Desktop (please complete the following information):

• OS: [macOS 10.14.6]
• Browser [Chrome]
• Version [79.0.3945.88]

---

IssueHunt Summary

Backers (Total: $0.00)

Become a backer now!

Or submit a pull request to get the deposits!

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.

[issuehunt-oss[bot]]

@piotr-s-brainhub has funded $5.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[jamsinclair]

Note for others, the infinite and arrows props are vital for the reproduction.

<Carousel
  arrows={true}
  dots={true}
  infinite={true}
  slidesPerPage={2}
  slidesPerScroll={2}
  value={...}
  slides={...}
  onChange={...}
/>

The issue is really "How to represent dots when the carousel is infinite" 🤔

The current implementation seems correct, the "active" slide is the most lefthand and the active sequence 1, 3, 5, 2, 4 would be expected.

@pierzchalatomasz How would you expect this interaction to be represented?

[pierzchalatomasz]

Hi @jamsinclair,

In this situation we have 5 scenes (I mean slides visible at one time) in the following order (where [1, 2] meaning scene with slides 1 and 2 etc.):

1. [1, 2]
2. [3, 4]
3. [5, 1]
4. [2, 3]
5. [4, 5]

I would expect that dots are calculated based on the index of scene instead of the first item in a current scene i.e. dots should be activated in the following order: 1 => 2 => 3 => 4 => 5 instead of 1 => 3 => 5 => 2 => 4.

[jamsinclair]

@pierzchalatomasz Thanks for the great explanation 👌 . Interesting problem 🤔.

[piotr-s-brainhub]

@pierzchalatomasz After merging https://github.com/brainhubeu/react-carousel/pull/177, the order is the following:

1: [1, 2]
2: [2, 3]
3: [3, 4]
4: [4, 5]
5: [5]

So I would change only the end to be:

5: [5, 1]

What do you think?

[Lukasz-pluszczewski]

@piotr-s-brainhub 5: [5, 1] instead of 5: [5, empty-space] makes sense if infinite is on. But the problem @pierzchalatomasz described is that when I have slidesPerPage={2}, slidesPerScroll={2} and 6 "slides", the actual number of pages is 3, so some users expect to have 3 dots. The issue here is that we always use the slide index and not page index. The reason for this is that we can have slidesPerPage={2} but slidesPerScroll={3}. What number of "pages" are there then? Not 3, because after clicking right arrow you end up on "page" 2,5. You can also have slidesPerPage={2} and controlled carousel, and pass the index 1 or 3 to it. You would be on a half page then.

I would leave the indexing as is, and the users just need to implement dots by themselves to have the behaviour @pierzchalatomasz suggested.

[piotr-s-brainhub]

@Lukasz-pluszczewski do you mean this situation or I have some incorrect params?

<Carousel
  slidesPerPage={2}
  slidesPerScroll={2}
  infinite
  dots
>
 <div>foo-1</div>
 <div>foo-2</div>
 <div>foo-3</div>
 <div>foo-4</div>
 <div>foo-5</div>
</Carousel>

for 5 slides:

for 6 slides:

[issuehunt-oss[bot]]

@piotr-s-brainhub has cancelled funding for this issue.(Cancelled amount: $5.00) See it on IssueHunt

[humbak]

I think that we should divide the amount of dots by slidesPerScroll by default. We may also add props to dots to control it.

[humbak]

I did a research and indeed this is an important bug.

We should correct the dots' behavior. How it should work:

1. number of dots should be equal to the number of slides divided by slidesPerScroll property and rounded up
2. if we have the rest from division. On the action (e.g. clicking the arrow, drag the slide) on the last slides, we should scroll only the remaining ones.
3. if we have the rest from dividing and infinite mode we should behave in the same way like in point 2

[felipenmoura]

So...this is a major problem, I get it (and agree).

Any update on that? This is pretty important stuff... Any workaround that makes it work?

The problem really seems to happen only when there's the combination of arrows, dots and infinite.