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

Adds support for gesture observables. #128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rharter
Copy link

@rharter rharter commented Sep 7, 2015

This is a first pass, so notes are welcome if you have any.

@JakeWharton
Copy link
Owner

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;
Copy link
Owner

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.

Copy link
Owner

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.

Copy link
Author

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.

@rharter
Copy link
Author

rharter commented Oct 2, 2015

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.

@Lakedaemon
Copy link

Also... the gesture detector spies on MotionEvents and delegates decisions to a gesture listener.
Depending on the implemented interface and of the feeback of the gesture listener, it emits higher level events (confirmedClick, doubleTap...)

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}
class SwipingTextSwitcher(context: Context, attrs: AttributeSet) : TextSwitcher(context, attrs) {
private val simpleSwipeListener = object : GestureDetector.SimpleOnGestureListener() {
override fun onDown(e1: MotionEvent): Boolean = true
override fun onFling(e1: MotionEvent, e2: MotionEvent, velocityX: Float, velocityY: Float): Boolean {
if (Math.abs(e1.y - e2.y) > SWIPE_MAX_OFF_PATH) return false

            if (e1.x - e2.x > SWIPE_MIN_DISTANCE && Math.abs(velocityX) > SWIPE_THRESHOLD_VELOCITY) {
                //do what you want on left swipe
                if (onSwipeListener != null) return onSwipeListener!!.onSwipe(this@SwipingTextSwitcher, OnSwipeListener.LEFT)
            } else if (e2.x - e1.x > SWIPE_MIN_DISTANCE && Math.abs(velocityX) > SWIPE_THRESHOLD_VELOCITY) {
                //do what you want on right swipe
                if (onSwipeListener != null) return onSwipeListener!!.onSwipe(this@SwipingTextSwitcher, OnSwipeListener.RIGHT)
            }

        return false
    }

    private val SWIPE_MIN_DISTANCE = 120
    private val SWIPE_MAX_OFF_PATH = 250
    private val SWIPE_THRESHOLD_VELOCITY = 200
}

public var onSwipeListener: OnSwipeListener? = null

private val gestureDetector = GestureDetector(context, simpleSwipeListener)

}
{code}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants