diff --git a/CHANGELOG.md b/CHANGELOG.md index fae013ec..38741145 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,13 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag This release closes the [2.1.0 milestone](https://github.com/Instagram/IGListKit/milestone/2). ### Enhancements + - Disables `prefetchEnabled` by default on `IGListCollectionView`. [Sven Bacia (#323)](https://github.com/Instagram/IGListKit/pull/323) +### Fixes + +- Avoid `UICollectionView` crashes when queueing a reload and insert/delete on the same item as well as reloading an item in a section that is animating. [Ryan Nystrom](https://github.com/rnystrom) [(#325)](https://github.com/Instagram/IGListKit/pull/325) + 2.0.0 ----- diff --git a/Source/IGListAdapter.m b/Source/IGListAdapter.m index 168284d4..5bda8c1f 100644 --- a/Source/IGListAdapter.m +++ b/Source/IGListAdapter.m @@ -893,8 +893,29 @@ return; } - NSArray *indexPaths = [self indexPathsFromSectionController:sectionController indexes:indexes adjustForUpdateBlock:YES]; - [self.updatingDelegate reloadItemsInCollectionView:collectionView indexPaths:indexPaths]; + if (self.isInUpdateBlock) { + /** + UICollectionView is not designed to support -reloadSections: or -reloadItemsAtIndexPaths: during batch updates. + Internally it appears to convert these operations to a delete+insert. However the transformation is too simple + in that it doesn't account for the item's section being moved (naturally or explicitly) and can queue animation + collisions. + + If you have an object at section 2 with 4 items and attempt to reload item at index 1, you would create an + NSIndexPath at section: 2, item: 1. Within -performBatchUpdates:, UICollectionView converts this to a delete + and insert at the same NSIndexPath. + + If a section were inserted at position 2, the original section 2 has naturally shifted to section 3. However, + the insert NSIndexPath is section: 2, item: 1. Now the UICollectionView has a section animation at section 2, + as well as an item insert animation at section: 2, item: 1, and it will throw an exception. + + IGListAdapter tracks the before/after mapping of section controllers to make precise NSIndexPath conversions. + */ + [self deleteInSectionController:sectionController atIndexes:indexes]; + [self insertInSectionController:sectionController atIndexes:indexes]; + } else { + NSArray *indexPaths = [self indexPathsFromSectionController:sectionController indexes:indexes adjustForUpdateBlock:YES]; + [self.updatingDelegate reloadItemsInCollectionView:collectionView indexPaths:indexPaths]; + } } - (void)insertInSectionController:(IGListSectionController *)sectionController atIndexes:(NSIndexSet *)indexes { diff --git a/Tests/IGListAdapterE2ETests.m b/Tests/IGListAdapterE2ETests.m index 3499af06..89cfd7f2 100644 --- a/Tests/IGListAdapterE2ETests.m +++ b/Tests/IGListAdapterE2ETests.m @@ -1055,4 +1055,85 @@ [self waitForExpectationsWithTimeout:15 handler:nil]; } +- (void)test_whenReloadingItems_withDeleteAndInsertCollision_thatUpdateCanBeApplied { + [self setupWithObjects:@[ + genTestObject(@1, @1), + genTestObject(@2, @5), + genTestObject(@3, @1), + ]]; + + IGListSectionController *section = [self.adapter sectionControllerForObject:self.dataSource.objects[1]]; + + XCTestExpectation *expectation = genExpectation; + [section.collectionContext performBatchAnimated:NO updates:^{ + [section.collectionContext deleteInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 2)]]; + [section.collectionContext insertInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 2)]]; + [section.collectionContext reloadInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 4)]]; + } completion:^(BOOL finished) { + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:15 handler:nil]; +} + +- (void)test_whenReloadingItems_withSectionInsertedInFront_thatUpdateCanBeApplied { + [self setupWithObjects:@[ + genTestObject(@1, @1), + genTestObject(@2, @5), + genTestObject(@3, @1), + ]]; + + IGListSectionController *section = [self.adapter sectionControllerForObject:self.dataSource.objects[1]]; + + XCTestExpectation *expectation1 = genExpectation; + [section.collectionContext performBatchAnimated:NO updates:^{ + [section.collectionContext reloadInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 4)]]; + } completion:^(BOOL finished) { + [expectation1 fulfill]; + }]; + + self.dataSource.objects = @[ + genTestObject(@1, @1), + genTestObject(@4, @1), // insert to shift object @2 + genTestObject(@2, @5), + genTestObject(@3, @1), + ]; + + XCTestExpectation *expectation2 = genExpectation; + [self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) { + [expectation2 fulfill]; + }]; + + [self waitForExpectationsWithTimeout:15 handler:nil]; +} + +- (void)test_whenReloadingItems_withSectionDeletedInFront_thatUpdateCanBeApplied { + [self setupWithObjects:@[ + genTestObject(@1, @1), + genTestObject(@2, @5), + genTestObject(@3, @1), + ]]; + + IGListSectionController *section = [self.adapter sectionControllerForObject:self.dataSource.objects[1]]; + + XCTestExpectation *expectation1 = genExpectation; + [section.collectionContext performBatchAnimated:NO updates:^{ + [section.collectionContext reloadInSectionController:section atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(1, 4)]]; + } completion:^(BOOL finished) { + [expectation1 fulfill]; + }]; + + self.dataSource.objects = @[ + genTestObject(@2, @5), + genTestObject(@3, @1), + ]; + + XCTestExpectation *expectation2 = genExpectation; + [self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) { + [expectation2 fulfill]; + }]; + + [self waitForExpectationsWithTimeout:15 handler:nil]; +} + @end