From f79516ee66e185586d9a04c280b09bf5575d4d64 Mon Sep 17 00:00:00 2001 From: Gulam Moledina Date: Tue, 22 Aug 2017 13:25:56 -0700 Subject: [PATCH] Proper support vertical and bottom positions in IGListAdapter scrollToObject: API Summary: I simply wrote failing tests for `scrollToObject` API for the `ScrollPosition` parameter and then fixed the code. If we agree that these tests are correct, I don't mind suggesting a fix for it. I think it's the `indexPath`'s row that is hardcoded to `0` that needs to be changed depending on the `scrollPosition` desired. (Done!) - [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 https://github.com/Instagram/IGListKit/pull/861 Differential Revision: D5678607 Pulled By: rnystrom fbshipit-source-id: ab902b58fe70667ef6133cb05569a2223a668243 --- CHANGELOG.md | 3 +++ Source/IGListAdapter.m | 20 ++++++++++++++------ Tests/IGListAdapterTests.m | 33 +++++++++++++++++++++++++++++---- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 653a2d08..2a3fc43c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,11 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag - Return correct `-[IGListAdapter visibleSectionControllers]` when section has no items, but has supplementary views. [Mani Ghasemlou](https://github.com/manicakes) [(#643)](https://github.com/Instagram/IGListKit/issues/643) - Call `[CATransaction commit]` before calling completion block in IGListAdapterUpdater to prevent animation issues. [Maxime Ollivier](https://github.com/maxoll) (tbd) + - Fix `scrollToObject:supplementaryKinds:...` not scrolling when section is empty but does have supplymentary views +- Better support for non-top positions in `scrollToObject:` API. [Gulam Moledina](https://github.com/gmoledina) [(#861)](https://github.com/Instagram/IGListKit/pull/861) + ### Enhancements - Added `-[IGListSectionController didDeselectItemAtIndex:]` API to support default `UICollectionView` cell deselection. [Ryan Nystrom](https://github.com/rnystrom) (tbd) diff --git a/Source/IGListAdapter.m b/Source/IGListAdapter.m index 4e46a0cd..30469ebf 100644 --- a/Source/IGListAdapter.m +++ b/Source/IGListAdapter.m @@ -185,19 +185,27 @@ [collectionView setNeedsLayout]; [collectionView layoutIfNeeded]; - NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:section]; - + NSIndexPath *indexPathFirstElement = [NSIndexPath indexPathForItem:0 inSection:section]; + // collect the layout attributes for the cell and supplementary views for the first index // this will break if there are supplementary views beyond item 0 - NSArray *attributes = nil; - + NSMutableArray *attributes = nil; + const NSInteger numberOfItems = [collectionView numberOfItemsInSection:section]; if (numberOfItems > 0) { - attributes = [self layoutAttributesForIndexPath:indexPath supplementaryKinds:supplementaryKinds]; + attributes = [self layoutAttributesForIndexPath:indexPathFirstElement supplementaryKinds:supplementaryKinds].mutableCopy; + + if (numberOfItems > 1) { + NSIndexPath *indexPathLastElement = [NSIndexPath indexPathForItem:(numberOfItems - 1) inSection:section]; + UICollectionViewLayoutAttributes *lastElementattributes = [self layoutAttributesForIndexPath:indexPathLastElement supplementaryKinds:supplementaryKinds].firstObject; + if (lastElementattributes != nil) { + [attributes addObject:lastElementattributes]; + } + } } else { NSMutableArray *supplementaryAttributes = [NSMutableArray new]; for (NSString* supplementaryKind in supplementaryKinds) { - UICollectionViewLayoutAttributes *supplementaryAttribute = [layout layoutAttributesForSupplementaryViewOfKind:supplementaryKind atIndexPath:indexPath]; + UICollectionViewLayoutAttributes *supplementaryAttribute = [layout layoutAttributesForSupplementaryViewOfKind:supplementaryKind atIndexPath:indexPathFirstElement]; if (supplementaryAttribute != nil) { [supplementaryAttributes addObject: supplementaryAttribute]; } diff --git a/Tests/IGListAdapterTests.m b/Tests/IGListAdapterTests.m index c93dde53..e37821ff 100644 --- a/Tests/IGListAdapterTests.m +++ b/Tests/IGListAdapterTests.m @@ -647,9 +647,9 @@ // Content height minus collection view height is 110, can't scroll more than that IGAssertEqualPoint([self.collectionView contentOffset], 0, 110); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionCenteredVertically animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 105); + IGAssertEqualPoint([self.collectionView contentOffset], 0, 110); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 60); + IGAssertEqualPoint([self.collectionView contentOffset], 0, 110); } - (void)test_whenScrollVerticallyToItemInASectionWithNoCellsAndNoSupplymentaryView { @@ -689,6 +689,31 @@ IGAssertEqualPoint([self.collectionView contentOffset], 0, 20); } +- (void)test_whenScrollVerticallyToItemWithPositionning { + self.dataSource.objects = @[@1, @100, @200]; + [self.adapter reloadDataWithCompletion:nil]; + [self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); + [self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionTop animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); + [self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionCenteredVertically animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); + [self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); + + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 10); + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionTop animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 10); + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionCenteredVertically animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 460); + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, 910); + + [self.adapter scrollToObject:@200 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 0, self.collectionView.contentSize.height - self.collectionView.frame.size.height); +} + - (void)test_whenScrollHorizontallyToItem { // # of items for each object == [item integerValue], so @2 has 2 items (cells) IGListTestAdapterHorizontalDataSource *dataSource = [[IGListTestAdapterHorizontalDataSource alloc] init]; @@ -707,9 +732,9 @@ // Content width minus collection view width is 110, can't scroll more than that IGAssertEqualPoint([self.collectionView contentOffset], 110, 0); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionCenteredHorizontally animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 105, 0); + IGAssertEqualPoint([self.collectionView contentOffset], 110, 0); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionRight animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 60, 0); + IGAssertEqualPoint([self.collectionView contentOffset], 110, 0); self.layout.scrollDirection = UICollectionViewScrollDirectionVertical; self.adapter.dataSource = self.dataSource; }