IGListAdapter support for scrollViewDidEndDecelerating #899
Conversation
|
@plarson updated the pull request - view changes |
|
@plarson updated the pull request - view changes |
There was a problem hiding this comment.
Thanks @plarson ! 💯
Overall looks great. 👍 If possible, it'd be great to add a unit test. I think there are tests for the existing scroll methods, so you can follow that pattern.
One issue is that this is a breaking change for 3.0 clients. We have 2 options:
- defer this to 4.0 when breaking changes are acceptable to make
- make this additional method
@optionaland create a task to make it non-optional for the 4.0 release
We've already done (2) (I think) for another would-be-breaking API change in 3.1.
I'm fine with doing (2). cc @rnystrom for final decision here (due to how this will affect IG)
Source/IGListScrollDelegate.h
Outdated
| @param listAdapter The list adapter whose collection view ended decelerating. | ||
| @param sectionController The visible section controller that ended decelerating. | ||
| */ | ||
| - (void)listAdapter:(IGListAdapter *)listAdapter didEndDeceleratingSectionController:(IGListSectionController *)sectionController; |
There was a problem hiding this comment.
breaking change here.
…roller:] optional
|
@plarson updated the pull request - view changes |
|
Ok @jessesquires, I made that method optional and added respondsToSelector checks |
|
Yup lets make this optional with a note that it will be required in 4.0 |
add `@optional` explanation
|
@plarson updated the pull request - view changes |
There was a problem hiding this comment.
@plarson would you mind adding some tests for both the adapter and stacked SC? We have some examples that should make this easy:
Once those are in we can merge this!
|
I'm gonna bump this to 3.2 since its a new feature anyways. @plarson lmk if you're still able to work on this, if not we can take over the PR. Thanks! |
…ingSectionController:]
|
@plarson updated the pull request - view changes |
|
Ok, I added a test. I also adedd a NS_SWIFT_NAME for the selector because it wasn't translating automatically to match the others. |
|
@plarson awesome! We just need another test to cover IGListAdapter forwarding to other SCs (see example I posted above) and we're set! |
|
My bad, added |
|
@plarson updated the pull request - view changes |
scroll delegate in local var
|
@plarson updated the pull request - view changes |
stack SC local scroll delegate var
|
@plarson updated the pull request - view changes |
|
@rnystrom has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
References #842
We use -[UIScrollViewDelegate scrollViewDidEndDecelerating:] delegate callback in the app we are building to hide/show elements when motion has ended.