Skip to content
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

Incorrect index given on carousel:didScrollFrom:To: delegate on loop mode #2

Open
firanto opened this issue Aug 31, 2017 · 2 comments

Comments

@firanto
Copy link

firanto commented Aug 31, 2017

When I implement func carousel(_ carousel: CarouselView, didScrollFrom from: Int, to: Int) of CarouselViewDelegate, in loop mode, whenever we jump from the edge of our collections, it gives wrong index.

Example, if we have 3 items, the index should be 0, 1, and 2. When scrolling forward, didScrollFrom:To: should return from 0 to 1, from 1 to 2 (and reach the end of collection). When we move forward one more time, the expected result should be from 2 to 0. But this call was giving result as from 2 to 3. This also happen when moving backward on index 0. The expected result should be from 0 to 2. And again, the call was giving from 0 to -1.

I know this happen because of extra offset cell. Maybe before calling that method, we should reassign the to in this manner:

to = to >= 0 ? to % numberOfCells : numberOfCells - 1

My take is, users only know the number of items they own. They won't give a damn about offset cells. Logically, they only know indexes of their items and looping should give correct index which they know. Not some index outside their collection.

@zhangbozhb
Copy link
Owner

@kurotsukikaitou Thanks. As you point out, it gives wrong index. from/to was use to indicate scroll direction. Assume cell number is 3. As none loop type, the current display cell is 3, and you scroll. At this point 'carouselScroll(from:3, to: 4)' or 'carouselScroll(from:3, to: 0)'. Maybe the later is more reasonable。And this may be more confused,if cell count is 2 and in loop mode。
Your idea sounds more reasonable in normal usage. Thanks your advise. It adopt in version 1.4.
This lib solution is not that good now, and i have an rough idea to improve it。Thanks.

@firanto
Copy link
Author

firanto commented Sep 7, 2017

Yeah. I stumble on the offset index confusion when using 'to' index to set UIPageControl selected index. Which is why I have to add that index conversion before setting page control's selected index.

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

No branches or pull requests

2 participants