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

RxView scrollChangeEvents for api lower than M #315

Open
MickSparrow opened this issue Dec 5, 2016 · 6 comments
Open

RxView scrollChangeEvents for api lower than M #315

MickSparrow opened this issue Dec 5, 2016 · 6 comments

Comments

@MickSparrow
Copy link

For now scroll change events are supported only for @TargetApi(M).

 @TargetApi(M)
  @CheckResult @NonNull
  public static Observable<ViewScrollChangeEvent> scrollChangeEvents(@NonNull View view) {
    checkNotNull(view, "view == null");
    return Observable.create(new ViewScrollChangeEventOnSubscribe(view));
  }

I write simple solution for this:

  public static Observable<ViewScrollChangeEvent> scrollChangeEvents(View view) {
        checkNotNull(view, "view == null");
        return Observable.create(new CustomScrollChangeEventOnSubscribe(view));
    }
final class CustomScrollChangeEventOnSubscribe implements Observable.OnSubscribe<ViewScrollChangeEvent> {
    private final View view;
    private ViewTreeObserver.OnScrollChangedListener onScrollChangedListener;
    private ViewTreeObserver viewTreeObserver;

    CustomScrollChangeEventOnSubscribe(View view) {
        this.view = view;
    }

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

        ViewScrollChangeEventListener listener = new ViewScrollChangeEventListener() {
            @Override
            public void onViewScrollChangeEvent(ViewScrollChangeEvent viewScrollChangeEvent) {
                if (!subscriber.isUnsubscribed()) {
                    subscriber.onNext(viewScrollChangeEvent);
                }
            }
        };
        setScrollTouchListener(listener, view);

        subscriber.add(new MainThreadSubscription() {
            @Override
            protected void onUnsubscribe() {
                if (checkTreeObserver()) {
                    viewTreeObserver.removeOnScrollChangedListener(onScrollChangedListener);
                }
            }
        });
    }

    private void setScrollTouchListener(final ViewScrollChangeEventListener viewScrollChangeEventListener, final View viewToObserve) {
        onScrollChangedListener = new ViewTreeObserver.OnScrollChangedListener() {
            private int oldScrollX = 0;
            private int oldScrollY = 0;

            @Override
            public void onScrollChanged() {
                if (viewToObserve.getScrollX() != oldScrollX || viewToObserve.getScrollY() != oldScrollY) {
                    viewScrollChangeEventListener.onViewScrollChangeEvent(ViewScrollChangeEvent.create(viewToObserve, viewToObserve.getScrollX(), viewToObserve.getScrollY(), oldScrollX, oldScrollY));
                    oldScrollX = viewToObserve.getScrollX();
                    oldScrollY = viewToObserve.getScrollY();
                }
            }
        };
        viewTreeObserver = viewToObserve.getViewTreeObserver();
        if (checkTreeObserver()) {
            viewTreeObserver.addOnScrollChangedListener(onScrollChangedListener);
        }
    }

    private boolean checkTreeObserver() {
        return viewTreeObserver != null && viewTreeObserver.isAlive();
    }

    interface ViewScrollChangeEventListener {
        void onViewScrollChangeEvent(ViewScrollChangeEvent viewScrollChangeEvent);
    }

}

Maybe it can be added to library

@JakeWharton
Copy link
Owner

JakeWharton commented Dec 5, 2016 via email

@MickSparrow
Copy link
Author

OK, this solution is not passing test, but passes "eye test", so I must check it out, and when I'll send PR.

@MickSparrow
Copy link
Author

I tested my solution on some devices with api level from 16 to 23 and its working.
But this fails test because ViewTreeObserver.OnScrollChangedListener() is not calling in test enviroment.
So I don't know what to do because I have no time to investigate tests environment.

kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 20, 2018
@kujyp
Copy link

kujyp commented Feb 23, 2018

@MickSparrow
There are two problems here.

  1. View.getViewTreeObserver() returns mFloatingTreeObserver

In test case, view doesn't attach any contexts. So view.getViewTreeObserver() returns mFloatingTreeObserver. It doesn't invoke ViewTreeObserver.dispatchOnScrollChanged

RxViewText.java
private final View view = new View(context);
https://github.com/JakeWharton/RxBinding/blob/master/rxbinding/src/androidTest/java/com/jakewharton/rxbinding2/view/RxViewTest.java#L41

View.java

public ViewTreeObserver getViewTreeObserver() {
  if (mAttachInfo != null) {
     return mAttachInfo.mTreeObserver;
  }
  if (mFloatingTreeObserver == null) {
    mFloatingTreeObserver = new ViewTreeObserver(mContext);
  }
  return mFloatingTreeObserver;
}

https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/view/View.java#21287

So I added new test code RxViewScrollTest.java. It tests RxView.scrollChangeEvents with view attached a activity context.

RxViewScrollTestActivity.java

public final class RxViewScrollTestActivity extends Activity {
  View view;

  @Override protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    view = new View(this);

    setContentView(view);
  }
}

setContentView(view); solves this problem
kujyp@71a8629#diff-2fa09177d9d6ae1c0204b88a5b808cc7R18

  1. ViewTreeObserver.dispatchOnScrollChanged is called asynchronously.

ViewTreeObserver.dispatchOnScrollChanged called by Choreographer$FrameHandler
But it runs asynchronously. So it raises assertion before calling ViewTreeObserver.dispatchOnScrollChanged
So awaiting async ui update is needed here.

view.scrollTo(1, 1);
// Awaiting async ui update is needed here
ViewScrollChangeEvent event0 = o.takeNext();

kujyp@9a8b4aa#diff-7e13fb411759fc694fdb2e7a1cb0dbb8R43

kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 24, 2018
kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 24, 2018
kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 24, 2018
kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 25, 2018
kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 25, 2018
kujyp pushed a commit to kujyp/RxBinding that referenced this issue Feb 25, 2018
@kujyp
Copy link

kujyp commented Feb 25, 2018

@JakeWharton
5339796 passed the ci pipeline.

But I just made small changes, then 03685a0 failed.

I have no idea why it failed with the Classes that I didn't touch.

com.jakewharton.rxbinding2.view.RxViewSystemUiVisibilityTest > systemUiVisibilityChanges[test(AVD) - 4.3.1] FAILED
com.jakewharton.rxbinding2.view.RxViewSystemUiVisibilityTest > systemUiVisibilityChanges[test(AVD) - 4.3.1] FAILED
com.jakewharton.rxbinding2.widget.RxAutoCompleteTextViewTest > itemClickEvents[test(AVD) - 4.3.1] FAILED
com.jakewharton.rxbinding2.widget.RxRatingBarTest > ratingChangeEvents[test(AVD) - 4.3.1] FAILED
com.jakewharton.rxbinding2.widget.RxSeekBarTest > changeEvents[test(AVD) - 4.3.1] FAILED
...

https://travis-ci.org/JakeWharton/RxBinding/jobs/345882826#L1667

@kujyp
Copy link

kujyp commented Feb 28, 2018

It seems to be flaky tests, so I just leave it failed.
I'm sorry for pushing empty commits. I need some feedbacks that I didn't do wrong.

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

No branches or pull requests

3 participants