alexvasilkov / GestureViews

ImageView and FrameLayout with gestures control and position animation
Apache License 2.0
2.38k stars 384 forks source link

Blindly passing touch events to ViewPager is wrong #79

Closed saket closed 7 years ago

saket commented 7 years ago

I found an issue while trying to block my ViewPager from scrolling. GestureControllerForPager#passEventToViewPager() always passes touch events to the ViewPager, even when it's possible that requestDisallowInterceptTouchEvents() was called on it to block scrolling. Unfortunately, I cannot think of any solution other than subclassing ViewPager because there's no getter to check if intercepting is disabled.

XJIOP commented 7 years ago

My solution

Create custom ViewPager

import android.content.Context;
import android.support.v4.view.ViewPager;
import android.util.AttributeSet;
import android.view.MotionEvent;

import static ViewImageActivity.swapEnabled;

public class CustomViewPager extends ViewPager {

    public CustomViewPager(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    @Override
    public boolean onTouchEvent(MotionEvent event) {
        if (swapEnabled) {
            return super.onTouchEvent(event);
        }
        return false;
    }

    @Override
    public boolean onInterceptTouchEvent(MotionEvent event) {
        if (swapEnabled) {
            return super.onInterceptTouchEvent(event);
        }
        return false;
    }
}

and listen state changes

CustomViewPager mViewPager = (CustomViewPager) findViewById(R.id.viewimage_container);

...

public static boolean swapEnabled = true;
final float[] mZoom = new float[photo.size()];
final GestureImageView gestureImageView = rootView.findViewById(R.id.view_image);

gestureImageView.getController().addOnStateChangeListener(new GestureController.OnStateChangeListener() {

    @Override
    public void onStateChanged(State state) {
        swapEnabled = state.getZoom() <= mZoom[position];
    }

    @Override
    public void onStateReset(State oldState, State newState) {
        mZoom[position] = newState.getZoom();
    }
});
saket commented 7 years ago

@XJIOP Overriding ViewPager#canScroll() might a better idea. Here's my solution: https://gist.github.com/554a2356fc744f66f73360e44ea0be87

alexvasilkov commented 7 years ago

Hi guys, sorry for late response. Internally GestureControllerForPager is already calling requestDisallowInterceptTouchEvent(true) on ViewPager and then manually passes all scroll events to it. To prevent ViewPager from scrolling you can use disableViewPager(...) method: image.getController().disableViewPager(true)

saket commented 7 years ago

To prevent ViewPager from scrolling you can use disableViewPager(...)

But by doing so you also lose the ability to block ViewPager from scrolling when the image can be panned.

saket commented 7 years ago

Also, a curious question: overriding ViewPager#canScroll() is a far easier solution than GestureControllerForPager. What is the reason behind it's complexity?

alexvasilkov commented 7 years ago

But by doing so you also lose the ability to block ViewPager from scrolling when the image can be panned.

Hm, you can still pan the image if ViewPager is disabled. Not sure I understand the problem. Do you want to prevent ViewPager from scrolling only in some specific cases, or what?

Also, a curious question: overriding ViewPager#canScroll() is a far easier solution than GestureControllerForPager. What is the reason behind it's complexity?

First of all I didn't noticed it before :) But anyway, I doubt that logic would be very simple, and it will also force everyone to use custom ViewPager implementation which I tried to avoid.

alexvasilkov commented 7 years ago

I'll close this issue since I see no ways how I can help you beyond suggesting to use disableViewPager(). Let me know if you have any better ideas.