From 619b835710c71ce795588baa504e83e198a3f852 Mon Sep 17 00:00:00 2001 From: "Qinghua Hong(qhhonx)" Date: Sat, 25 May 2019 12:44:39 -0700 Subject: [PATCH] Fix scrollToObject: inconsistency bug when scrolling to bottom/right with content inset (#1284) Summary: The implementation of ```Objective-C -[IGListAdapter scrollToObject:supplementaryKinds:scrollDirection:scrollPosition:animated:] ``` has little inconsistencies where the `UICollectionViewScrollPositionLeft` and `UICollectionViewScrollPositionTop` considering the content inset left/top of the collection view was being applied to the final offset. However, the `UICollectionViewScrollPositionRight` and `UICollectionViewScrollPositionBottom` ignoring the content inset right/bottom of the collection view was being applied to the final offset. The different result is that scrolling to the most `Left/Top`, the first section content always be visible by considering its content inset while scrolling to the most `Right/Bottom`, the last section content always be fully displayed but not be fully visible by considering its content inset. ## Changes in this pull request Issue fixed: # ### Checklist - [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) Pull Request resolved: https://github.com/Instagram/IGListKit/pull/1284 Reviewed By: lorixx Differential Revision: D13590416 Pulled By: lorixx fbshipit-source-id: ae52be9e5ba25b50c7a0ad768a4af728347523e2 --- CHANGELOG.md | 2 ++ Source/IGListAdapter.m | 4 ++-- Tests/IGListAdapterTests.m | 42 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4c79553..d774bfaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag ### Fixes +- Fixed bug with `-[IGListAdapter scrollToObject:supplementaryKinds:scrollDirection:scrollPosition:animated:]` where the content inset(bottom/right) of the collection view was incorrectly being applied to the final offset and was inconsistent with the content inset(top/left) of the collection view being applied. [Qinghua Hong](https://github.com/xohozu) [(#1284)](https://github.com/Instagram/IGListKit/pull/1284) + - Fixed crash when the data source is nil before calling `-[IGListAdapterUpdater performUpdateWithCollectionViewBlock:fromObjects:toObjectsBlock:animated:objectTransitionBlock:completion:]`. [Zhisheng Huang](https://github.com/lorixx) (tbd) - Experimental fix to get the `UICollectionView` for batch updating immediately before applying the update. [Ryan Nystrom](https://github.com/rnystrom) (tbd) diff --git a/Source/IGListAdapter.m b/Source/IGListAdapter.m index cc47e508..d13de0dc 100644 --- a/Source/IGListAdapter.m +++ b/Source/IGListAdapter.m @@ -264,7 +264,7 @@ case UICollectionViewScrollDirectionHorizontal: { switch (scrollPosition) { case UICollectionViewScrollPositionRight: - contentOffset.x = offsetMax - collectionViewWidth - contentInset.left; + contentOffset.x = offsetMax - collectionViewWidth + contentInset.right; break; case UICollectionViewScrollPositionCenteredHorizontally: { const CGFloat insets = (contentInset.left - contentInset.right) / 2.0; @@ -288,7 +288,7 @@ case UICollectionViewScrollDirectionVertical: { switch (scrollPosition) { case UICollectionViewScrollPositionBottom: - contentOffset.y = offsetMax - collectionViewHeight; + contentOffset.y = offsetMax - collectionViewHeight + contentInset.bottom; break; case UICollectionViewScrollPositionCenteredVertically: { const CGFloat insets = (contentInset.top - contentInset.bottom) / 2.0; diff --git a/Tests/IGListAdapterTests.m b/Tests/IGListAdapterTests.m index 104c8082..30ced65a 100644 --- a/Tests/IGListAdapterTests.m +++ b/Tests/IGListAdapterTests.m @@ -736,6 +736,12 @@ self.dataSource.objects = @[@100]; [self.adapter reloadDataWithCompletion:nil]; + if (@available(iOS 11.0, *)) { + self.collectionView.contentInsetAdjustmentBehavior = UIScrollViewContentInsetAdjustmentNever; + } else { + // Fallback on earlier versions + } + // no insets self.collectionView.contentInset = UIEdgeInsetsMake(0, 0, 0, 0); [self.collectionView layoutIfNeeded]; @@ -751,13 +757,45 @@ self.collectionView.contentInset = UIEdgeInsetsMake(0, 0, 100, 0); [self.collectionView layoutIfNeeded]; [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 900); + IGAssertEqualPoint([self.collectionView contentOffset], 0, 900 + 100); // top 50, bottom 100 self.collectionView.contentInset = UIEdgeInsetsMake(50, 0, 100, 0); [self.collectionView layoutIfNeeded]; [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 900); + IGAssertEqualPoint([self.collectionView contentOffset], 0, 900 + 100); +} + +- (void)test_whenScrollHorizontalToRight_withContentInsets_thatRightFlushWithCollectionViewBounds { + self.dataSource.objects = @[@100]; + [self.adapter reloadDataWithCompletion:nil]; + + UICollectionViewFlowLayout *layout = (UICollectionViewFlowLayout *)self.collectionView.collectionViewLayout; + layout.scrollDirection = UICollectionViewScrollDirectionHorizontal; + [layout invalidateLayout]; + + // no insets + self.collectionView.contentInset = UIEdgeInsetsMake(0, 0, 0, 0); + [self.collectionView layoutIfNeeded]; + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionRight animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 900, 0); + + // left 100 + self.collectionView.contentInset = UIEdgeInsetsMake(0, 100, 0, 0); + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionRight animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 900, 0); + + // right 100 + self.collectionView.contentInset = UIEdgeInsetsMake(0, 0, 0, 100); + [self.collectionView layoutIfNeeded]; + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionRight animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 900 + 100, 0); + + // left 50, right 100 + self.collectionView.contentInset = UIEdgeInsetsMake(0, 50, 0, 100); + [self.collectionView layoutIfNeeded]; + [self.adapter scrollToObject:@100 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionRight animated:NO]; + IGAssertEqualPoint([self.collectionView contentOffset], 900 + 100, 0); } - (void)test_whenScrollHorizontallyToItem {