-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Conversation
@zhubofei updated the pull request - view changes |
@zhubofei updated the pull request - view changes |
@zhubofei updated the pull request - view changes |
1 similar comment
This is awesome. Since its a useful and breaking change, let's add this to 2.0. @zhubofei can you update |
@@ -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) |
There was a problem hiding this comment.
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"
@zhubofei updated the pull request - view changes |
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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rnystrom lol. yeah, lets:
- go ahead and merge this
- open a new issue to track this and follow-up there (and eventually submit new PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhubofei -- Looks like we need to resolve conflicts in After that, I think we're good here 👍 |
@zhubofei updated the pull request - view changes |
@jessesquires g2g 💪💪 |
@zhubofei updated the pull request - view changes |
@jessesquires has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Thanks @zhubofei ! 💯 |
#139