Add ViewObservables.listViewScroll(AbsListView). #57
Conversation
|
|
||
| import android.widget.AbsListView; | ||
|
|
||
| public class OnListViewScrollEvent { |
There was a problem hiding this comment.
This whole class can be replaced with auto/value. The best code is the code you neither write nor have to maintain.
There was a problem hiding this comment.
Neat! Didn't know about that. From what I can tell auto is not a dependency in RxAndroid yet and that seems like a wider discussion of tooling, style, and personal preferences.
There was a problem hiding this comment.
There is no runtime component. It's entirely an implementation detail.
There was a problem hiding this comment.
(i.e., it's a build-time only dependency)
There was a problem hiding this comment.
@JakeWharton is it cool with you if we do that in a separate step? We can log an issue to add auto support to remove boiler plate like this and then port things over.
hamidp
force-pushed
the
list-scroll-observable
branch
from
4a4ccbd to
dd9f301
Compare
hamidp
force-pushed
the
list-scroll-observable
branch
from
dd9f301 to
bc22439
Compare
|
All of the comments should be addressed now. I've also squashed an rebased. |
| }; | ||
|
|
||
| composite.addOnScrollListener(listener); | ||
| observer.add(AndroidSubscriptions.unsubscribeInUiThread(new Action0() { |
There was a problem hiding this comment.
I always thought these double dispatches are weird. We're already testing as a precondition that we're on the main thread, so unless we're rescheduling the observable purposefully to deliver notifications on a background thread after this operator is applied, we will also unsubscribe on the main thread.
I've seen this being done a lot in the last few PRs and always wondered if we're just being defensive or if this will not simply cause an extra cycle through the message loop in the normal case, thus unnecessarily delaying unsubscription?
|
👍 |
|
I don't have write access so can someone with write merge when it all looks good? |
Add ViewObservables.listViewScroll(AbsListView).
No description provided.