Touch event is not emitted when handled func returns false in ViewTouchOnSubscribe

[phajduk]

Hi. Before creating a PR I would like to ask you guys if I'm right. ViewTouchOnSubscribe class which is internally used during for example RxView.touchesmethod contains:

@Override public void call(final Subscriber<? super MotionEvent> subscriber) {
    checkUiThread();

    View.OnTouchListener listener = new View.OnTouchListener() {
      @Override public boolean onTouch(View v, @NonNull MotionEvent event) {
        if (handled.call(event)) {
          if (!subscriber.isUnsubscribed()) {
            subscriber.onNext(event);
          }
          return true;
        }
        return false;
      }
    };
    view.setOnTouchListener(listener);

    subscriber.add(new MainThreadSubscription() {
      @Override protected void onUnsubscribe() {
        view.setOnTouchListener(null);
      }
    });
  }

Current implementation probably blocks possibility to handle emitted MotionEvent when handled function returns false for example:

RxView.touches(someEditText, motionEvent -> false)
        .subscribe(event -> {
    // do something here
});

In above case, no value will be emitted. When it's needed? When we want to detect touch on EditText widget and don't "consume" the MotionEvent. We can't consume it if we just want to make some extra action but keep EditText to get focus (that's why handled have to return false).

My proposed solution:

View.OnTouchListener listener = new View.OnTouchListener() {
    @Override public boolean onTouch(View v, @NonNull MotionEvent event) {
        if (!subscriber.isUnsubscribed()) {
            subscriber.onNext(event);
        }

        return handled.call(event);
    }
};

Am I correct or I did something wrong?

[dlew]

That seems reasonable to me, but it does mean there will be a behavior change. I think that's more correct though, since sometimes you do just want to listen (but not fully handle) touch events.

[phajduk]

According to this commit I believe it applies to more classes: ViewLongClickEventOnSubscribe, ViewLongClickOnSubscribe, ViewDragOnSubscribe, ViewDragEventOnSubscribe, ViewTouchEventOnSubscribe, ViewTouchOnSubscribe, AdapterViewItemLongClickEventOnSubscribe, AdapterViewItemLongClickOnSubscribe, TextViewEditorActionEventOnSubscribe, TextViewEditorActionOnSubscribe which means that's serious change 😢

[JakeWharton]

I would not be happy with that change as proposed. There's no longer a way for the downstream code to know if the event was handled or not when the function is truly conditional (e.g., point inside a circle). I understand the need to observe without consuming, but I think we need to come up with a way to support the filtering case as well, without having to run the conditional function twice.

[phajduk]

@JakeWharton you're right that there's no longer a way for downstream code to know if the event was handled. However in current implementation value won't be propagated to the downstream code right?
I totally agree with statement that we should avoid to run conditional function twice but we also don't have guarantee that it's always the same predicate (in handled and filter functions).

[DrewCarlson]

Any progress on an approach to take here?

I have a case with nested ViewPagers where the inner ViewPager needs to prevent the parent from changing pages if the inner's current item is the first or last item. Following this, the inner ViewPager automatically changes it's current item after a period of time, but this needs to be cancelled on ACTION_MOVE.

[ar-g]

RatingBar won't work if subscribe to touches().

[Den-Rimus]

I have a view, I want to listen to MotionEvent.ACTION_DOWN touch events and I need to pass event further.

Using stock android approach I do

mapTouchView.setOnTouchListener((v, event) -> {
                if (event.getAction() == MotionEvent.ACTION_DOWN) {
                    hideInfoIfShown();
                }
                return false;
            });

and it works.

Switching to RxBinding approach my code is

RxView.touches(mapTouchView, motionEvent -> false) // I want touchListener to return false to pass
                                                   // motion event further to underlaying whatever-there-is
                    .filter(motionEvent -> motionEvent.getAction() == MotionEvent.ACTION_DOWN)
                    .compose(RxLifecycle.bindView(this))
                    .subscribe(motionEvent -> hideInfoIfShown());

and it won't work since passed handled Func1 returned value of false used not as returning value for View.OnTouchListener but as a condition to check whether to call onNext for subscriber. How is that usable at all?

Shouldn't this:

be this:

So far I'll have to stick with android-way, because Rx-way in just not usable for my case.

[phajduk]

@Den-Rimus Does your solution differ somehow from mine posted in very first comment?

[Den-Rimus]

Actually, it does not, sorry. Now that I've read thread more carefully I see that the problem was fully discussed, but can't be fixed due to breaking existing API.

Note that documentation on second param says exactly what we would expect, and not how it works now.

[govindr91]

Any update on this? I'm having a scenario where longClickEvent is being interrupted by this event.

[ZakTaccardi]

I understand the need to observe without consuming

Why not an overloaded method specifically for this use case? (non-breaking change)

public static Observable<MotionEvent> touches(@NonNull View view, boolean consume)

//calling it
RxView.touches(view, false)

[JakeWharton]

That overload already exists: RxView.touches(view, e -> false)

[ZakTaccardi]

That overload already exists: RxView.touches(view, e -> false)

If the e -> false predicate returns false, .onNext() doesn't get called.

My use case is that I want to observe touch events without consuming them.

    @Override public boolean onTouch(View v, MotionEvent event) {
      if (!isDisposed()) {
        try {
          if (handled.test(event)) {
            observer.onNext(event); //no on next if predicate is false
            return true;
          }
        } catch (Exception e) {
          observer.onError(e);
          dispose();
        }
      }

      return false;

From ViewTouchObservable

[b22n]

I expected it would work like this, too

try {
  observer.onNext(event);
  return handled.test(event);
} catch(Exception e) {
  observer.onError(e);
  dispose();
}
return false;

[Troy1010]

This is my work-around for the moment:

fun EditText.editorActionEvents2(handled: (TextViewEditorActionEvent) -> Boolean = { true }): Observable<TextViewEditorActionEvent> =
    Observable.create { downstream ->
        editorActionEvents { downstream.onNext(it); handled(it) }
            .subscribe({}, { downstream.onError(it) }) { downstream.onComplete() }
            .also { downstream.setDisposable(it) }
    }