Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify end position for -scrollToObject #196

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ This release closes the [2.0.0 milestone](https://github.com/Instagram/IGListKit

- Diff result method `-resultWithUpdatedMovesAsDeleteInserts` removed and replaced with `-resultForBatchUpdates` [(b5aa5e3)](https://github.com/Instagram/IGListKit/commit/b5aa5e39002854c947e777c11ae241f67f24d19c)
- `IGListDiffable` equality method changed from `isEqual:` to `isEqualToDiffableObject:`
- `ScrollToObject` method changed from `-scrollToObject:supplementaryKinds:scrollDirection:animated` to `-scrollToObject:supplementaryKinds:scrollDirection:atScrollPosition:animated`. Added support for specify end position. [Bofei Zhu](https://github.com/zhubofei) [(#196)](https://github.com/Instagram/IGListKit/pull/196)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: Can we remove the ScrollToObject formatting and just start w/ "Scrolling method changed from"


### Enhancements

Expand All @@ -19,7 +20,7 @@ This release closes the [2.0.0 milestone](https://github.com/Instagram/IGListKit
- Added an additional initializer for `IGListSingleSectionController` to be able to support single sections created from nibs. An example can be found [here](Example/IGListKitExamples/ViewControllers/SingleSectionViewController.swift).
- Fixed `-[IGListAdapter reloadDataWithCompletion:]` not returning early when `collectionView` or `dataSource` is nil and `completion` is nil. [Ben Asher](https://github.com/benasher44) [(#51)](https://github.com/Instagram/IGListKit/pull/51)
- Added `-isFirstSection` and `-isLastSection` APIs to `IGListSectionController`
- Added support for cells created from storyboard. [Bofei Zhu](https://github.com/zhubofei) [(#92)](https://github.com/Instagram/IGListKit/pull/92)
- Added support for cells and supplementaryViews created from storyboard. [Bofei Zhu](https://github.com/zhubofei) [(#92)](https://github.com/Instagram/IGListKit/pull/92)
- Added examples for Today & iMessage extensions. [Sherlouk](https://github.com/Sherlouk) [(#112)](https://github.com/Instagram/IGListKit/pull/112)
- Added `tvOS` support. [Jesse Squires](https://github.com/jessesquires) [(#137)](https://github.com/Instagram/IGListKit/pull/137)
- Added new `-[IGListAdapter visibleObjects]` API. [Ryan Nystrom](https://github.com/rnystrom) [(386ae07)](https://github.com/Instagram/IGListKit/commit/386ae0786445c06e1eabf074a4181614332f155f)
Expand Down
2 changes: 2 additions & 0 deletions Source/IGListAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,13 @@ IGLK_SUBCLASSING_RESTRICTED
@param object The object to scroll to.
@param supplementaryKinds The types of supplementary views in the section.
@param scrollDirection A flag indicating the direction to scroll.
@param scrollPosition An option that specifies where the item should be positioned when scrolling finishes.
@param animated A flag indicating if the transition should be animated.
*/
- (void)scrollToObject:(id)object
supplementaryKinds:(nullable NSArray<NSString *> *)supplementaryKinds
scrollDirection:(UICollectionViewScrollDirection)scrollDirection
atScrollPosition:(UICollectionViewScrollPosition)scrollPosition
animated:(BOOL)animated;

/**
Expand Down
50 changes: 41 additions & 9 deletions Source/IGListAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ - (void)updateCollectionViewDelegate {
- (void)scrollToObject:(id)object
supplementaryKinds:(NSArray<NSString *> *)supplementaryKinds
scrollDirection:(UICollectionViewScrollDirection)scrollDirection
atScrollPosition:(UICollectionViewScrollPosition)scrollPosition
animated:(BOOL)animated {
IGAssertMainThread();
IGParameterAssert(object != nil);
Expand All @@ -165,33 +166,64 @@ - (void)scrollToObject:(id)object
NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:section];
NSArray *attributes = [self layoutAttributesForIndexPath:indexPath supplementaryKinds:supplementaryKinds];

CGFloat offset = 0.0;
CGFloat offsetMin = 0.0;
CGFloat offsetMax = 0.0;
for (UICollectionViewLayoutAttributes *attribute in attributes) {
const CGRect frame = attribute.frame;
CGFloat origin;
CGFloat originMin;
CGFloat endMax;
switch (scrollDirection) {
case UICollectionViewScrollDirectionHorizontal:
origin = CGRectGetMinX(frame);
originMin = CGRectGetMinX(frame);
endMax = CGRectGetMaxX(frame);
break;
case UICollectionViewScrollDirectionVertical:
origin = CGRectGetMinY(frame);
originMin = CGRectGetMinY(frame);
endMax = CGRectGetMaxY(frame);
break;
}

// find the minimum origin value of all the layout attributes
if (attribute == attributes.firstObject || origin < offset) {
offset = origin;
if (attribute == attributes.firstObject || originMin < offsetMin) {
offsetMin = originMin;
}
// find the maximum end value of all the layout attributes
if (attribute == attributes.firstObject || endMax > offsetMax) {
offsetMax = endMax;
}
}


const CGFloat offsetMid = (offsetMin + offsetMax) / 2.0;
const CGFloat collectionViewWidth = collectionView.bounds.size.width;
const CGFloat collectionViewHeight = collectionView.bounds.size.height;
const UIEdgeInsets contentInset = collectionView.contentInset;
CGPoint contentOffset = collectionView.contentOffset;
switch (scrollDirection) {
case UICollectionViewScrollDirectionHorizontal:
contentOffset.x = offset - contentInset.left;
switch (scrollPosition) {
case UICollectionViewScrollPositionRight:
contentOffset.x = offsetMax - collectionViewWidth - contentInset.left;
break;
case UICollectionViewScrollPositionCenteredHorizontally:
contentOffset.x = offsetMid - collectionViewWidth / 2.0 - contentInset.left;
break;
default:
contentOffset.x = offsetMin - contentInset.left;
break;
}
break;
case UICollectionViewScrollDirectionVertical:
contentOffset.y = offset - contentInset.top;
switch (scrollPosition) {
case UICollectionViewScrollPositionBottom:
contentOffset.y = offsetMax - collectionViewHeight - contentInset.top;
break;
case UICollectionViewScrollPositionCenteredVertically:
contentOffset.y = offsetMid - collectionViewHeight / 2.0 - contentInset.top;
break;
default:
contentOffset.y = offsetMin - contentInset.top;
break;
}
break;
}

Expand Down
42 changes: 25 additions & 17 deletions Tests/IGListAdapterTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -532,14 +532,18 @@ - (void)test_whenScrollVerticallyToItem {
self.dataSource.objects = @[@1, @2, @3, @4, @5, @6];
[self.adapter reloadDataWithCompletion:nil];
XCTAssertEqual([self.collectionView numberOfSections], 6);
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
[self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 10);
[self.adapter scrollToObject:@3 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@3 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 30);
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 150);
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionCenteredVertically animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 105);
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionBottom animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 60);
}

- (void)test_whenScrollHorizontallyToItem {
Expand All @@ -548,13 +552,17 @@ - (void)test_whenScrollHorizontallyToItem {
self.layout.scrollDirection = UICollectionViewScrollDirectionHorizontal;
[self.adapter reloadDataWithCompletion:nil];
XCTAssertEqual([self.collectionView numberOfSections], 6);
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal animated:NO];
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
[self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal animated:NO];
[self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 100, 0);
[self.adapter scrollToObject:@3 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal animated:NO];
[self.adapter scrollToObject:@3 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 200, 0);
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal animated:NO];
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 500, 0);
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal atScrollPosition:UICollectionViewScrollPositionCenteredHorizontally animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 500, 0);
[self.adapter scrollToObject:@6 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionHorizontal atScrollPosition:UICollectionViewScrollPositionRight animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 500, 0);
self.layout.scrollDirection = UICollectionViewScrollDirectionVertical;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that these tests don't actually test the position since items are full width. We almost need a new test suite to actually cover these scenarios.

@jessesquires what do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep 😂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnystrom lol. yeah, lets:

  1. go ahead and merge this
  2. open a new issue to track this and follow-up there (and eventually submit new PR)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnystrom We need new sectionController and new datasource. I think it would be much easier if we can fix #183 first.

}
Expand All @@ -574,9 +582,9 @@ - (void)test_whenScrollToItem_thatSupplementarySourceSupportsSingleHeader {
[self.adapter performUpdatesAnimated:NO completion:nil];

XCTAssertNotNil([self.collectionView supplementaryViewForElementKind:UICollectionElementKindSectionHeader atIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]);
[self.adapter scrollToObject:@1 supplementaryKinds:@[UICollectionElementKindSectionHeader] scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@1 supplementaryKinds:@[UICollectionElementKindSectionHeader] scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
[self.adapter scrollToObject:@2 supplementaryKinds:@[UICollectionElementKindSectionHeader] scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@2 supplementaryKinds:@[UICollectionElementKindSectionHeader] scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 20);
}

Expand All @@ -596,17 +604,17 @@ - (void)test_whenScrollToItem_thatSupplementarySourceSupportsHeaderAndFooter {

XCTAssertNotNil([self.collectionView supplementaryViewForElementKind:UICollectionElementKindSectionHeader atIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]);
XCTAssertNotNil([self.collectionView supplementaryViewForElementKind:UICollectionElementKindSectionFooter atIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]]);
[self.adapter scrollToObject:@1 supplementaryKinds:@[UICollectionElementKindSectionHeader, UICollectionElementKindSectionFooter] scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@1 supplementaryKinds:@[UICollectionElementKindSectionHeader, UICollectionElementKindSectionFooter] scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
[self.adapter scrollToObject:@2 supplementaryKinds:@[UICollectionElementKindSectionHeader, UICollectionElementKindSectionFooter] scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@2 supplementaryKinds:@[UICollectionElementKindSectionHeader, UICollectionElementKindSectionFooter] scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 30);
}

- (void)test_whenScrollVerticallyToItem_thatFeedIsEmpty {
self.dataSource.objects = @[];
[self.adapter reloadDataWithCompletion:nil];
XCTAssertEqual([self.collectionView numberOfSections], 0);
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
}

Expand All @@ -615,13 +623,13 @@ - (void)test_whenScrollVerticallyToItem_thatItemNotInFeed {
self.dataSource.objects = @[@1, @2, @3, @4];
[self.adapter reloadDataWithCompletion:nil];
XCTAssertEqual([self.collectionView numberOfSections], 4);
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@1 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
[self.adapter scrollToObject:@5 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@5 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 0);
[self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@2 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 10);
[self.adapter scrollToObject:@5 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical animated:NO];
[self.adapter scrollToObject:@5 supplementaryKinds:nil scrollDirection:UICollectionViewScrollDirectionVertical atScrollPosition:UICollectionViewScrollPositionNone animated:NO];
IGAssertEqualPoint([self.collectionView contentOffset], 0, 10);
}

Expand Down