Comments (11)
It would be nice. Thank you for this library. 👍
from mspeekcollectionviewdelegateimplementation.
There is, as a result, another bug: If we try to scroll to the next item and then click on the item while scrolling, it returns us to the previous item.
I fixed this by changing MSCollectionViewPaging.collectionViewWillEndDragging
horizontal case:
public func collectionViewWillEndDragging(scrollDirection: UICollectionView.ScrollDirection, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer<CGPoint>) {
switch scrollDirection {
case .horizontal:
if velocity.equalTo(.zero) {
targetContentOffset.pointee = CGPoint(x: currentContentOffset, y: 0)
} else {
targetContentOffset.pointee = CGPoint(x: getNewTargetOffset(startingOffset: currentContentOffset, velocity: velocity.x, targetOffset: targetContentOffset.pointee.x), y: targetContentOffset.pointee.y)
currentContentOffset = targetContentOffset.pointee.x
}
case .vertical:
targetContentOffset.pointee = CGPoint(x: targetContentOffset.pointee.x, y: getNewTargetOffset(startingOffset: currentContentOffset, velocity: velocity.y, targetOffset: targetContentOffset.pointee.y))
currentContentOffset = targetContentOffset.pointee.y
default:
assertionFailure("Not Implemented")
}
}
It's not the perfect solution, but it works better now.
from mspeekcollectionviewdelegateimplementation.
Thanks @TimGrell so much for raising this bug and spending time to find a solution!
I'll take a look at this after work and see how we can fix this
Cheers!
from mspeekcollectionviewdelegateimplementation.
Again @TimGrell thanks for the solution! This looks like it fixes some of the edge cases but I just realized that I don't use scroll threshold at all 😆 I'm using a velocity threshold only (When you scroll faster than the threshold velocity, it moves to another cell). I'm going to make an update to introduce the scroll threshold and hopefully it will fix this issue
from mspeekcollectionviewdelegateimplementation.
Are there any updates? I tried the proposed solution which fixed the snap back, but made it so slowly pulling the collection and releasing snaps back to whatever was current before the scroll.
I made the following change instead:
public func indexForItemAtPoint(point: CGPoint) -> Int {
// at 375 * 200, peeking = 20, spacing = 20, item length = 137.5
// at index = 2, point is 315
// at index = 4, point is 630
// at index = 6, point is 945
let pointOffset: CGFloat
switch scrollDirection {
case .horizontal:
pointOffset = point.x
case .vertical:
pointOffset = point.y
default:
assertionFailure("Not implemented")
return 0
}
return min(max(0, Int(round(pointOffset / (itemLength(axis: .main) + spacingLength)))), numberOfItems)
}
This seems to fix both issues, but introduces a new issue where a short fast drag from left to right snaps the collection to the next cell on the right strangely. It must have something to do with velocity but I have not worked it out yet.
Thank you for the library, hopefully we can find a solution that works for all cases.
EDIT: Not sure why but the delta calculated during a short fast swipe was causing the issue.
Changed the case where current and target index are the same to the following and everything is working:
case let (x, y, v) where x == y && v > scrollThreshold:
delta = Int(velocity * 100)
offset = 1
It is definitely not the cleanest solution but just making delta related to the velocity in this instance fixed the odd scrolling behavior with a quick swipe. I will keep checking back for a better solution.
from mspeekcollectionviewdelegateimplementation.
Thank you so much @Errortype520 for spending time and raising this issue! I also appreciate you supplying a potential solution for this! <3
As for the solution, I'm not sure if rounding is a long term solution for this. It seems like it's just a patch and doing something like this might introduce other edge cases, and I can see that you're aware of that. That also applies to the velocity change
But you're definitely looking in the right place! getNewTargetOffset
is a monster of a function and there's too much stuff going on. Right now it's a bit difficult to change the logic and make sure that all previous features are working properly. So I'm currently adding some unit tests to this function so that any changes we introduce will not affect previous logic.
Maybe we can figure a way to prioritize the scroll threshold when the velocity is 0, this way we don't jump to a neighboring cell if the user didn't swipe.
If you're interested I can push a change to the repo that includes the required tests and maybe we can investigate this thing together how about that?
Cheers!
from mspeekcollectionviewdelegateimplementation.
@MaherKSantina I added rounding because it seemed to prefer the previous cell position even if next cell was closer to center when dragging stopped.
Looking at the func indexForItemAtPoint(point: CGPoint) -> Int
function more, it looks like it takes the (item size + spacing) and the offset to find the index.
What about the initial peek?
It looks like index is calculated off of the left edge of the item. Shouldn't the index also be adjusted to consider the center of the item?
from mspeekcollectionviewdelegateimplementation.
Thanks @Errortype520 for your comment! I'm sorry it takes me time to reply to you because we're in different timezones
The indexForItemAtPoint
function is just a converter from an offset to an index. It basically will behave the same way even if we use the center to get the index. Suppose we have a width of 100 and no peeking, here's now it does it now:
0 offset -> 0 index
100 offset -> 1 index
200 offset -> 2 index
...
If we want to change the offset calculation to the center, it would be something like this:
50 offset -> 0 index
150 offset -> 1 index
250 offset -> 2 index
The peeking shouldn't affect this logic, because the peeking is something related to how the cells should be displayed on the screen. The spacing affects it so that's why I'm adding the spacingLength
to mitigate this offset change
from mspeekcollectionviewdelegateimplementation.
As for the rounding fix, I was wonder what would happen if we got a whole number after doing the equation? Wouldn't the value be the same and we end up with the wrong cell?
from mspeekcollectionviewdelegateimplementation.
@Errortype520 , I've created a new branch to work on the fix (fix-scroll-issue). I've added some tests which are failing that we need to make them pass. If you have some free time feel free to try some things to make the tests pass. I'll also do the same thing and hopefully this issue will be gone forever!
from mspeekcollectionviewdelegateimplementation.
@Errortype520 you're a genius! After digging into it I found out that you were right about the rounding for the index. It makes more sense because as you said it was preferring the left cell instead of getting the nearest. I've just merged a PR that fixes the issue #57 and I'll create a new version of the library that contains the fix. Thank you so much for helping me out hunt and fix this bug!
Have a great day!
@TimGrell thanks so much for you too for raising this issue. Feel free to re-open this issue if you found out that it's not fixed yet
from mspeekcollectionviewdelegateimplementation.
Related Issues (20)
- Add support to animate peeking items while scrolling HOT 1
- Where is MSPeekImplementationDelegate now? HOT 3
- Add SPM Support HOT 2
- Alignment horizontal scroll collection view HOT 6
- Is it possible to change the view content of center one? HOT 3
- RxCollectionView only shows one cell always HOT 1
- sizeForItem delegate function is not called when peek behavior is configured HOT 6
- Use of undeclared type 'MSCollectionViewPeekingBehavior' HOT 4
- Use of undeclared type 'MSCollectionViewPeekingBehavior' HOT 1
- extra space !? HOT 4
- MSPeekImplementationDelegate returns under/over index in race conditions HOT 5
- Top and Bottom spacing of UICollectionViewCell cut HOT 2
- Crash on behavior.scrollViewWillEndDragging call in v3.1.1
- Collection view item is getting cut off HOT 8
- SPM Fail HOT 1
- Backward scroll
- Scroll bars not visible
- can't change cell size and positioning
- Mistakenly created
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mspeekcollectionviewdelegateimplementation.