giulio92 / GLTableCollectionView

Netflix and App Store like UITableView with UICollectionView, written in pure Swift 4.2
MIT License
705 stars 37 forks source link

Jumping to end of Content #30

Closed mmdock closed 6 years ago

mmdock commented 6 years ago

I had an issue with GLIndexedCollectionViewFlowLayout where when content offset is 0, if I were to bounce the collection (pull it to have a negative content offset on x) the offset would jump to the very end of the loaded content in the collection.

I added this:

if offsetCorrection != .greatestFiniteMagnitude {
    return CGPoint(x: proposedContentOffset.x + offsetCorrection, y: 0)
} else {
    return CGPoint(x: proposedContentOffset.x , y: 0)
}

to the final return in:

func targetContentOffset(forProposedContentOffset proposedContentOffset: , withScrollingVelocity velocity: )

Probably not the best way to handle this behavior, since I am not 100% sure why that number needs to be so high

I just wanted to bring some attention to this, as I wasn't sure if it was intended behavior to scroll to the end on a bounce at the beginning of the collection like that.

giulio92 commented 6 years ago

Hello @mmdock, thank you for your feedback!

I have tried to place your code inside targetContentOffset(forProposedContentOffset proposedContentOffset: , withScrollingVelocity velocity: ) func to see how the code flows in one of the two if branches but as far as I can test, with regular back and forth scrolling, the only if branch that gets executed is:

return CGPoint(x: proposedContentOffset.x + offsetCorrection, y: 0)

How should I test GLIndexedCollectionViewFlowLayout to get to the other branch that you suggested?

One more question: With content offset do you refer to proposedContentOffset?

mmdock commented 6 years ago

@giulio92 I was finding it hard to put into words so I just upload two little vids(as gifs) with an explanation

ezgif com-video-to-gif

In the above clip, you will see that when I drag my collection to the right when there are no more cells to the left, so the code return CGPoint(x: proposedContentOffset.x + offsetCorrection, y: 0) has a really large value as x causing my collection to go immediately to the final loaded cell in the collection.

The aforementioned if statement solved this issue.

Here is the new resulting behavior: ezgif com-video-to-gif 1

Answer: The content offset I was referring to was the content offset of the collection that happens in the first clip above. It starts at 0, when on the first cell, and I "bounce" the collection when I drag to the right trying to go to a cell on the left that doesn't exist

giulio92 commented 6 years ago

@mmdock thank your for your further explanation!

I have managed to reproduce the issue and I have found the cause of it. The bug happens when the proposedContentOffset .x or .y values are equal to 0, causing the

targetContentOffset(forProposedContentOffset proposedContentOffset: , withScrollingVelocity velocity: )

func return to be:

return CGPoint(x: proposedContentOffset.x + offsetCorrection, y: 0)

(where offsetCorrection is .greatestFiniteMagnitude)

To fix this wrong behavior we should check if proposedContentOffset .x/.y values are greater than 0 and, if so, simply return

I will open a new Pull Request to fix this issue, with you as a credit!

giulio92 commented 6 years ago

Pull request opened and merged on master, new version released: 1.81

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.