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: #1284 Reviewed By: lorixx Differential Revision: D13590416 Pulled By: lorixx fbshipit-source-id: ae52be9e5ba25b50c7a0ad768a4af728347523e2
Instagram · May 25, 2019 · 619b835 · 619b835
1 parent 7551573
commit 619b835
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 4 deletions.
diff --git 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
@@ -264,7 +264,7 @@ - (void)scrollToObject:(id)object
         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 @@ - (void)scrollToObject:(id)object
         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
@@ -736,6 +736,12 @@ - (void)test_whenScrollVerticallyToBottom_withContentInsets_thatBottomFlushWithC
     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 @@ - (void)test_whenScrollVerticallyToBottom_withContentInsets_thatBottomFlushWithC
     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 {