performBatchUpdates method doesn't perform batch updates

[vldalx]

RxTableViewSectionedAnimatedDataSource performs batch updates not as a single batch but separately each update from the batch.

The problem is https://github.com/RxSwiftCommunity/RxDataSources/blob/5458733cb4d2a1ca6a457804921a5fac4b72fca2/Sources/RxDataSources/RxTableViewSectionedAnimatedDataSource.swift#L102-L106 https://github.com/RxSwiftCommunity/RxDataSources/blob/5458733cb4d2a1ca6a457804921a5fac4b72fca2/Sources/RxDataSources/UI%2BSectionedViewType.swift#L60-L64 So it means each difference is handled in a separate UITableView.performBatchUpdates(_:completion:) invoke.

The same problem goes for UICollectionView.

The code above should be like:

if #available(iOS 11.0, *) {
    tableView.performBatchUpdates({
        for difference in differences {
            dataSource.setSections(difference.finalSections) // not sure about this line
            _performBatchUpdates(tableView, changes: difference, animationConfiguration: self.animationConfiguration)
        }
    }, completion: nil)
} else {
    tableView.beginUpdates()
    for difference in differences {
         dataSource.setSections(difference.finalSections) // not sure about this line
         _performBatchUpdates(tableView, changes: difference, animationConfiguration: self.animationConfiguration)
    }
    tableView.endUpdates()
}

[vldalx]

The issue with the current implementation is shown in the yoking GIF. If TableView is scrolled to the bottom and an item in the datasource is replaced with a new one, then TableView will lost its offset. The app in the GIF replaces an item in the datasource if the item has been selected.

Compare it with the fixed performBatchUpdates in the next GIF.

[mashe]

@vldalx hey, have you tried your code with examples provided in the project? As far as i got, Diff calculations are fast, but the price is you have to do make separate performBatchUpdates for each difference.

[hooliooo]

What are the implications of changing it to this? This suggestion fixed a UI error I was having where I had collapsible section headers. The underlying data was correct but the UI showed duplicated sections.

This change fixed that.

By the time the UITableView/UICollectionView is updated, the diffs were already computed. AFAIK, there should be little to no difference in terms of speed right? @kzaher

[vldalx]

@mashe I haven't tried my code with the examples. I created the pretty simple project and recorded the difference (see the attached GIFs)

[kzaher]

Potential solutions:

• ignoring UIScrollView inset for the time updates are made.
• caanimationgroup maybe
• finding an algorithm that splits the updates in different groups.

Unfortunately I don't have time to try these solutions. Maybe somebody could and post a result here.

[mashe]

@hooliooo there are cases when change (old sections -> new sections) could not be performed in one performBatchUpdates block, otherwise there is a crash: 'NSInternalInconsistencyException attempt to delete and reload the same index path' or something similar. That is the reason why change is separated in three diffs that could be performed one by one.

[hooliooo]

I see. I subclassed Rx*ViewSectionedAnimatedDataSource to do wrap the for loop in a single performBatchUpdate and it works for how I use RxDataSources.

At first glance it seems that error you mentioned only happens if a section changes/gets deleted but its items doesn't so the diff deletes the section but keeps its items to be used by another section. Maybe that's how it would happen?, another possiblility could be the items you are diffing aren't hashed correctly therefore aren' truly unique?

I only really use sections if I want to pin the section to the top as the user scrolls down. If the UI doesn't require that "pin header" functionality, I "fake" the section.

Maybe the upcoming Swift 5.1 collection diffing will make this problem go away for apps that use 5.1