MaherKSantina / MSPeekCollectionViewDelegateImplementation

A custom paging behavior that peeks the previous and next items in a collection view
https://medium.com/@maher.santina90/how-to-create-cells-that-peek-on-the-sides-like-ios-11-app-store-ef4bb54c0c7d
MIT License
356 stars 32 forks source link

Consider volecity of the EndDragging. Act more like Apple style. #52

Closed N1nomiya closed 4 years ago

MaherKSantina commented 4 years ago

Hello @imzark ! Thank you so much for giving interest to this repo! And thank you for raising this PR! 🎉 I'll add my comments inline

N1nomiya commented 4 years ago

Thanks for reply! @MaherKSantina

And sorry for my uncautious code. The condition should be abs(velocity) > 0.

So my intention is when finger drag ending with no velocity (not negative velocity) , the destinationIndex may be judged by (scrollDistance / halfPageWidth).

Now in the repo, the destinationIndex is always judged by (scrollDistance / scrollThreshold), that makes scrolling to next page too easy when the velocity is zero. That means if I drag a little distance (but more than scrollThreshold) and stop my finger, the collectionView will scroll to next page.

MaherKSantina commented 4 years ago

Ohh haha thanks @imzark for the clarification! I actually just tried the app store paging and it looks like it also moves to the next page even if you slightly scroll with your finger. That being said, the scroll distance in the repo is somewhat taking the velocity into consideration because it's using the targetContentOffset. If the distance between the targetContentOffset and the current scroll is large, that means there is high velocity.

Would love your input on this!

N1nomiya commented 4 years ago

Yeah,, I'm not sure whether you touch the little difference I was saying... cause me not that good at english expression.

gifs below explain my thought.

Repo Now 1

PR 2

MaherKSantina commented 4 years ago

@imzark ohh I see what you mean, but I also tried this on the app store and here's a video of the behavior: ezgif-2-2cd24c0c4a3d

It looks like it's switching to the other page even without velocity. Is that what you're talking about?

N1nomiya commented 4 years ago

Yes! Without velocity, It not always scroll to next page.

MaherKSantina commented 4 years ago

So you're saying that you don't want it to scroll all the time to the next page if there is no velocity? Wouldn't increasing the scrollThreshold fix this?

N1nomiya commented 4 years ago

Thanks again for your reply! And ya you got my point~ 😁

Increasing scrollThreshold would make it more difficulty to scroll if there is a velocity.

So I mean may be another way is needed to decide whether scroll to next page when there is no velocity. That is something like scrollDistance / halfPageWidth, just like the App Store's behavior.

MaherKSantina commented 4 years ago

Ohhh I think now I understand fully what you're saying! Yeah that's a good idea! I was thinking maybe having a velocityThreshold similar to the scroll threshold and that would dictate whether we need to move to the next page or not based on the threshold. What do you think?

N1nomiya commented 4 years ago

YEAH! That would be a better solution I think.

MaherKSantina commented 4 years ago

Awesome! Do you think you can update this PR to reflect what we talked about? 👼

N1nomiya commented 4 years ago

Sure, I'll give a try

N1nomiya commented 4 years ago

Hello @MaherKSantina ! I've update the PR, would you please test it? 😁

MaherKSantina commented 4 years ago

Awesome @imzark! 🎉 Thank you very much! I'll take a look at today if I have time after work! Really appreciate the effort 🎖

MaherKSantina commented 4 years ago

Hello @imzark ! Apologies for the delay but I've been very busy at work 😢 I'll try to take a look at it as soon as I can! Thank you for your patience

N1nomiya commented 4 years ago

Yeah it's ok, take your time. Just a little change btw

MaherKSantina commented 4 years ago

Hey @imzark ! I just wanted to let you know that I completely re-developed the library using a custom collection view layout and a separate paging class https://github.com/MaherKSantina/MSPeekCollectionViewDelegateImplementation/releases/tag/3.0.0. The reason is that people are having a lot of issues with the current implementation. So hopefully the new version is better!

I tried to incorporate your logic in this PR in the new version, but I'm not 100% sure I got it right. Do you mind taking a look at the new logic and letting me know if it's working the way you want it to?

In the meantime I'll close this PR because the logic and location of code has changed, but if you found out that there's something that can be enhanced please feel free to create a new PR

Much appreciated buddy ❤️

N1nomiya commented 4 years ago

Awesome! 👏 I would be glad to try the new version and let you know if meeting any problem. Thanks for the excellent repo. 👍

MaherKSantina commented 4 years ago

@imzark yes please let me know and thank you so much for your support! 🎉