From 1d926d9a9749d412291ce1e8fff685a0c383267d Mon Sep 17 00:00:00 2001 From: "Chen (Bert) Li" Date: Thu, 29 Jun 2017 08:58:17 -0700 Subject: [PATCH] Fix over-scrolling issue in IGListAdapter scrollToObject: method Summary: UIScrollView can over-scroll, if content offset is too small or too large. Providing methods to help calculate safe content offsets, given the desired values. Use the safe content offsets in IGListAdapter. Reviewed By: rnystrom Differential Revision: D5312473 fbshipit-source-id: d882948940ce08c48f5ff0ab8d997591f62915a8 --- Source/IGListAdapter.m | 14 ++++++++++++-- Tests/IGListAdapterTests.m | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Source/IGListAdapter.m b/Source/IGListAdapter.m index 11f7600d..d1a86dc4 100644 --- a/Source/IGListAdapter.m +++ b/Source/IGListAdapter.m @@ -220,7 +220,7 @@ const UIEdgeInsets contentInset = collectionView.contentInset; CGPoint contentOffset = collectionView.contentOffset; switch (scrollDirection) { - case UICollectionViewScrollDirectionHorizontal: + case UICollectionViewScrollDirectionHorizontal: { switch (scrollPosition) { case UICollectionViewScrollPositionRight: contentOffset.x = offsetMax - collectionViewWidth - contentInset.left; @@ -238,8 +238,13 @@ contentOffset.x = offsetMin - contentInset.left; break; } + const CGFloat maxOffsetX = collectionView.contentSize.width - collectionView.frame.size.width + collectionView.contentInset.right; + const CGFloat minOffsetX = -collectionView.contentInset.left; + contentOffset.x = MIN(contentOffset.x, maxOffsetX); + contentOffset.x = MAX(contentOffset.x, minOffsetX); break; - case UICollectionViewScrollDirectionVertical: + } + case UICollectionViewScrollDirectionVertical: { switch (scrollPosition) { case UICollectionViewScrollPositionBottom: contentOffset.y = offsetMax - collectionViewHeight - contentInset.top; @@ -257,7 +262,12 @@ contentOffset.y = offsetMin - contentInset.top; break; } + const CGFloat maxOffsetY = collectionView.contentSize.height - collectionView.frame.size.height + collectionView.contentInset.bottom; + const CGFloat minOffsetY = -collectionView.contentInset.top; + contentOffset.y = MIN(contentOffset.y, maxOffsetY); + contentOffset.y = MAX(contentOffset.y, minOffsetY); break; + } } [collectionView setContentOffset:contentOffset animated:animated]; diff --git a/Tests/IGListAdapterTests.m b/Tests/IGListAdapterTests.m index 3a4491d6..71b6e73f 100644 --- a/Tests/IGListAdapterTests.m +++ b/Tests/IGListAdapterTests.m @@ -627,7 +627,8 @@ [self.adapter scrollToObject:@3 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; IGAssertEqualPoint([self.collectionView contentOffset], 0, 30); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 150); + // 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); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionBottom animated:NO]; @@ -649,7 +650,8 @@ [self.adapter scrollToObject:@3 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionNone animated:NO]; IGAssertEqualPoint([self.collectionView contentOffset], 30, 0); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionNone animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 150, 0); + // 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); [self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal scrollPosition:UICollectionViewScrollPositionRight animated:NO]; @@ -676,7 +678,8 @@ [self.adapter scrollToObject:@1 supplementaryKinds:@[UICollectionElementKindSectionHeader] scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); [self.adapter scrollToObject:@2 supplementaryKinds:@[UICollectionElementKindSectionHeader] scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 20); + // Content height smaller than collection view height, won't scroll + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); } - (void)test_whenScrollToItem_thatSupplementarySourceSupportsHeaderAndFooter { @@ -698,7 +701,8 @@ [self.adapter scrollToObject:@1 supplementaryKinds:@[UICollectionElementKindSectionHeader, UICollectionElementKindSectionFooter] scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); [self.adapter scrollToObject:@2 supplementaryKinds:@[UICollectionElementKindSectionHeader, UICollectionElementKindSectionFooter] scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 30); + // Content height smaller than collection view height, won't scroll + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); } - (void)test_whenScrollVerticallyToItem_thatFeedIsEmpty { @@ -719,9 +723,11 @@ [self.adapter scrollToObject:@5 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); [self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 10); + // Content height is smaller than collection view height, can't scroll + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); [self.adapter scrollToObject:@5 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical scrollPosition:UICollectionViewScrollPositionNone animated:NO]; - IGAssertEqualPoint([self.collectionView contentOffset], 0, 10); + // Content height is smaller than collection view height, can't scroll + IGAssertEqualPoint([self.collectionView contentOffset], 0, 0); } - (void)test_whenQueryingIndexPath_withOOBSectionController_thatNilReturned {