Skip to content

Commit

Permalink
Fix binding SC cell updates when the SC's section changes.
Browse files Browse the repository at this point in the history
Summary:
Issue fixed: #1111

As discussed in the issue's comments, this seems to be the most expedient fix if we accept that `cellForItemAtIndex:` should be allowed within update blocks. I am not familiar enough with `IGListAdapter` to know whether this is the best solution but am willing to make any changes requested!

- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes #1144

Differential Revision: D7585125

Pulled By: rnystrom

fbshipit-source-id: 29a0e68da33bb4b94c2eb5e68844e537a7b05868
  • Loading branch information
ccrazy88 authored and facebook-github-bot committed Apr 11, 2018
1 parent 784f1be commit f469c34
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag

- `-[IGListSectionController didSelectItemAtIndex:]` is now called when a `scrollViewDelegate` or `collectionViewDelegate` is set. [Ryan Nystrom](https://github.com/rnystrom) [(#1108)](https://github.com/Instagram/IGListKit/pull/1108)

- Fixed binding section controllers failing to update their cells when the section controller's section changes. [Chrisna Aing](https://github.com/ccrazy88) [(#1144)](https://github.com/Instagram/IGListKit/pull/1144)

3.2.0
-----

Expand Down
2 changes: 1 addition & 1 deletion Source/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ - (__kindof UICollectionViewCell *)cellForItemAtIndex:(NSInteger)index
return nil;
}

NSIndexPath *indexPath = [self indexPathForSectionController:sectionController index:index usePreviousIfInUpdateBlock:NO];
NSIndexPath *indexPath = [self indexPathForSectionController:sectionController index:index usePreviousIfInUpdateBlock:YES];
// prevent querying the collection view if it isn't fully reloaded yet for the current data set
if (indexPath != nil
&& indexPath.section < [self.collectionView numberOfSections]) {
Expand Down
66 changes: 66 additions & 0 deletions Tests/IGListBindingSectionControllerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,72 @@ - (void)test_whenUpdating_withViewModelMovesAndReloads_thatCellUpdatedAndInstanc
[self waitForExpectationsWithTimeout:30 handler:nil];
}

- (void)test_whenUpdating_withViewModelDeletionsMovesAndReloads_thatCellUpdatedAndInstanceSame {
NSArray *startingSection0 = @[
@"0",
@"1",
[[IGTestObject alloc] initWithKey:@0 value:@"2"]
];
NSArray *startingSection1 = @[
@"a",
@"b",
[[IGTestObject alloc] initWithKey:@1 value:@"c"],
[[IGTestObject alloc] initWithKey:@2 value:@"d"],
];
[self setupWithObjects:@[
[[IGTestDiffingObject alloc] initWithKey:@0 objects:startingSection0],
[[IGTestDiffingObject alloc] initWithKey:@1 objects:startingSection1]
]];

XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 3);
XCTAssertEqual([self.collectionView numberOfItemsInSection:1], 4);

IGTestStringBindableCell *cell10 = [self cellAtSection:1 item:0];
IGTestStringBindableCell *cell11 = [self cellAtSection:1 item:1];
IGTestCell *cell12 = [self cellAtSection:1 item:2];
IGTestCell *cell13 = [self cellAtSection:1 item:3];

XCTAssertEqualObjects(cell10.label.text, @"a");
XCTAssertEqualObjects(cell11.label.text, @"b");
XCTAssertEqualObjects(cell12.label.text, @"c");
XCTAssertEqualObjects(cell13.label.text, @"d");

// Moves:
// - Delete section 0.
// - Modify section 1 in several ways:
NSArray *newSection1 = @[
[[IGTestObject alloc] initWithKey:@1 value:@"e"], // Index: 2 -> 0, Value: "c" -> "e"
@"b", // No change.
@"a", // Index: 0 -> 2
[[IGTestObject alloc] initWithKey:@2 value:@"f"], // Value: "d" -> "f"
];
self.dataSource.objects = @[
[[IGTestDiffingObject alloc] initWithKey:@1 objects:newSection1]
];

XCTestExpectation *expectation = [self expectationWithDescription:NSStringFromSelector(_cmd)];
[self.adapter performUpdatesAnimated:YES completion:^(BOOL finished) {
IGTestCell *batchedCell00 = [self cellAtSection:0 item:0];
IGTestStringBindableCell *batchedCell01 = [self cellAtSection:0 item:1];
IGTestStringBindableCell *batchedCell02 = [self cellAtSection:0 item:2];
IGTestCell *batchedCell03 = [self cellAtSection:0 item:3];

XCTAssertEqualObjects(batchedCell00.label.text, @"e");
XCTAssertEqualObjects(batchedCell01.label.text, @"b");
XCTAssertEqualObjects(batchedCell02.label.text, @"a");
XCTAssertEqualObjects(batchedCell03.label.text, @"f");

XCTAssertEqual(cell10, batchedCell02);
XCTAssertEqual(cell11, batchedCell01);
XCTAssertEqual(cell12, batchedCell00);
XCTAssertEqual(cell13, batchedCell03);

[expectation fulfill];
}];

[self waitForExpectationsWithTimeout:30 handler:nil];
}

- (void)test_whenUpdatingManually_with2Updates_thatBothCompletionBlocksCalled {
[self setupWithObjects:@[
[[IGTestDiffingObject alloc] initWithKey:@1 objects:@[@7, @"seven"]],
Expand Down

0 comments on commit f469c34

Please sign in to comment.