-
Notifications
You must be signed in to change notification settings - Fork 970
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
Adds support for gesture observables. #128
base: master
Are you sure you want to change the base?
Conversation
Will look soon. Catching up from 4-day vacation / weekend. |
private PublishSubject<ViewGestureEvent> singleTapUpGestureObservable; | ||
private PublishSubject<ViewGestureScrollEvent> scrollGestureObservable; | ||
private PublishSubject<ViewGestureEvent> longPressGestureObservable; | ||
private PublishSubject<ViewGestureFlingEvent> flingGestureObservable; |
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.
All these makes me super nervous. I'd rather use a single PublishSubject
and then just use ofType
to filter for the convenience methods.
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.
Actually we'd probably create an OnSubscribe
class like the other listener bindings and just emit ViewGestureEvent
. Then callers can filter or we can provide filters.
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.
Not at the computer, but IIRC the issue with the custom OnSubscribe is
setting the OnTouchListener. Since we can only have one at a time, we
needed to be able to add these without requiring that this be the only
listener, so I chose this pattern so that you could get the onTouches
subscription and simply pass it to this, in addition to doing whatever
other things you want with it. Let me know if I'm wrong there, haven't
looked in a while.
Also, I like the single PublishSubject with ofType, I didn't know about
that.
On Sat, Sep 26, 2015, 11:05 PM Jake Wharton [email protected]
wrote:
In
rxbinding/src/main/java/com/jakewharton/rxbinding/view/ObservableGestureListener.java
#128 (comment):+import android.view.MotionEvent;
+import android.view.View;
+
+import rx.Observable;
+import rx.subjects.PublishSubject;
+
+final class ObservableGestureListener implements GestureDetector.OnGestureListener {
+
- final View view;
- private PublishSubject downGestureObservable;
- private PublishSubject showPressGestureObservable;
- private PublishSubject singleTapUpGestureObservable;
- private PublishSubject scrollGestureObservable;
- private PublishSubject longPressGestureObservable;
- private PublishSubject flingGestureObservable;
Actually we'd probably create an OnSubscribe class like the other
listener bindings and just emit ViewGestureEvent. Then callers can filter
or we can provide filters.—
Reply to this email directly or view it on GitHub
https://github.com/JakeWharton/RxBinding/pull/128/files#r40501187.
I'll be doing some investigation to verify, but on first pass it seems that the performance of this isn't optimal. Unlike things like clicks and text entry, gestures are fast operations that update continuously and this adds several layers between the gesture callback and the actual resulting code execution, so this might cause backpressure quite easily in this case. As I said, I'm doing some investigation, but at this point I've reverted my code to using standard listener callbacks here. |
Also... the gesture detector spies on MotionEvents and delegates decisions to a gesture listener. For performance reasons, maybee it would be best to be as close to the metal as possible instead of doing the gesture detection down a Rx pipeline. At the moment, I'm using a simple gesture listener that filters swipes and delegates to a swipeListener that I added to the TextSwitcher class and I'm using the regular onSubscribe RxBinding Classes on that. I was wondering if it was possible to add gesture detection without relying on inheritance and an override of onTouchEvent(), through a call to setOnTouchListener that add a listener that would highjack custom events when needed (say when swiping) and delegates events to the class in the other cases In which case, we could write helper methods to build a gestureListener and then use the onSubscribe RxBinding classes to set/unset subscribe/unsubscribe to that... The Kotlin code I'm using at the moment (inspired from stuff read on stackoverflow) : {code}
} |
This is a first pass, so notes are welcome if you have any.