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

Using insertRowsAtIndexPaths instead of reloadData causes issues with contentOffset #31

Closed
intrepidmatt opened this issue Mar 12, 2016 · 7 comments

Comments

@intrepidmatt
Copy link

If you are updating your tableView using insertRowsAtIndexPaths rather than reloadData, the tableView doesn't calculate its contentSize before the infinite scroll handler adjusts the contentInset to remove the activity indicator. This causes the contentOffset property to be incorrectly adjusted upward by an amount equal to the height of the activity indicator.

I got around it in my own code by forcing the table view to update its contentSize after inserting rows:

tableView.contentSize = tableView.sizeThatFits(CGSize(width: tableView.bounds.size.width, height: CGFloat.max))

I'm not sure if there is a way to detect if the tableView needs to update its contentSize; if there is, it might be worth putting this logic inside a conditional in pb_stopAnimatingInfiniteScrollWithCompletion

@pronebird
Copy link
Owner

Hi,

Thanks for trying to understand how it works. UITableView has a lot of different internal states and it gets even more complex when using self-sizing cells.

I put lots of efforts in making it work with batch updates in this branch:

https://github.com/pronebird/UIScrollView-InfiniteScroll/tree/fix-dynamic-tableview-updates

I have tried to workaround the timing issue by using CATransaction in endUpdates to finish infinite scroll animations. You can see that I even have to skip one run loop because UITableView recalculates content size on next run loop. It's not perfect per se.

https://github.com/pronebird/UIScrollView-InfiniteScroll/blob/fix-dynamic-tableview-updates/Classes/UITableView%2BInfiniteScroll.m#L45

However for this to work developer would have to use custom endUpdates which is not optimal.

I'll play around with sizeThatFits, thanks for the idea!

@intrepidmatt
Copy link
Author

sizeThatFits appears to cause the tableView to make the necessary calls to its dataSource to determine the new size. Using that to explicitly set contentSize just prior to calling finishInfiniteScroll() seemed to resolve the issue (in my admittedly limited case).

Here's my full code if it helps:

    /**
     Set up the infinite scroll handler to use the `viewModel`'s paginator.
     */
    func setupInfiniteScroll() {
        searchTableView.infiniteScrollIndicatorStyle = .White
        searchTableView.addInfiniteScrollWithHandler() { scrollView in
            let startIndexPaths = self.allIndexPaths()
            let startSections = self.viewModel.search.dates.count
            self.viewModel.nextPage() {
                let endIndexPaths = self.allIndexPaths()
                let endSections = self.viewModel.search.dates.count
                let newIndexPaths = Array(endIndexPaths.subtract(startIndexPaths))
                let newSections = NSIndexSet(indexesInRange: NSRange(startSections..<endSections))

                // Use insertSections and insertRowsAtIndexPaths so we don't reload/reanimate
                // the existing cells.
                self.searchTableView.beginUpdates()
                self.searchTableView.insertSections(newSections, withRowAnimation: .None)
                self.searchTableView.insertRowsAtIndexPaths(newIndexPaths, withRowAnimation: .None)
                self.searchTableView.endUpdates()

                // Force the table view to update its contentSize; if we don't do this,
                // finishInfiniteScroll() will adjust contentInsets and cause contentOffset
                // to be off by an amount equal to the height of the activity indicator.
                // See https://github.com/pronebird/UIScrollView-InfiniteScroll/issues/31
                self.searchTableView.contentSize = self.searchTableView.sizeThatFits(CGSize(width: self.searchTableView.width, height: CGFloat.max))
                self.searchTableView.finishInfiniteScroll()
            }
        }
    }

@pronebird
Copy link
Owner

Sorry it takes that long to get any feedback from me, I may be able to spare an hour or two to take a look at this issue over weekend. I really really appreciate your efforts on discovering the potential fix for dynamic updates in table views.

@pronebird
Copy link
Owner

@intrepidmatt
Copy link
Author

I had turned off github notifications by accident -- will check this out tonight! Sorry for delay.

@pronebird
Copy link
Owner

I just realized that it's been a while and I entirely forgot to merge this. Will do now as it seems to work on iOS 9.3 for both self-sizing and fixed height cells.

@pronebird
Copy link
Owner

Released with 0.9.0. It has issues with self-sizing cells while table view is decelerating or user is messing around with scroll – animations look really chaotic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants