-
-
Notifications
You must be signed in to change notification settings - Fork 123
Allow jumping to previous/next page with horizontal swipe gesture #603
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,9 @@ | |
| import android.view.Menu; | ||
| import android.view.MenuInflater; | ||
| import android.view.MenuItem; | ||
| import android.view.MotionEvent; | ||
| import android.view.View; | ||
| import android.view.ViewConfiguration; | ||
| import android.webkit.CookieManager; | ||
| import android.webkit.JavascriptInterface; | ||
| import android.webkit.RenderProcessGoneDetail; | ||
|
|
@@ -28,6 +30,7 @@ | |
| import androidx.activity.result.ActivityResultLauncher; | ||
| import androidx.activity.result.contract.ActivityResultContracts; | ||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
| import androidx.appcompat.app.AppCompatActivity; | ||
| import androidx.core.view.WindowCompat; | ||
| import androidx.fragment.app.Fragment; | ||
|
|
@@ -125,6 +128,8 @@ public class PdfViewer extends AppCompatActivity implements LoaderManager.Loader | |
| private float mZoomRatio = 1f; | ||
| private float mZoomFocusX = 0f; | ||
| private float mZoomFocusY = 0f; | ||
| private int swipeThreshold; | ||
| private int swipeVelocityThreshold; | ||
|
Comment on lines
+131
to
+132
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: these should be prefixed with m
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I saw the other PR, this is probably okay
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this is based on the assumption that it will be on top of #616. |
||
| private int mDocumentOrientationDegrees; | ||
| private int mDocumentState; | ||
| private String mEncryptedDocumentPassword; | ||
|
|
@@ -431,6 +436,8 @@ public boolean onRenderProcessGone(WebView view, RenderProcessGoneDetail detail) | |
| } | ||
| }); | ||
|
|
||
| initializeGestures(); | ||
|
|
||
| GestureHelper.attach(PdfViewer.this, binding.webview, | ||
| new GestureHelper.GestureListener() { | ||
| @Override | ||
|
|
@@ -450,6 +457,39 @@ public boolean onTapUp() { | |
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean onFling(@Nullable MotionEvent e1, @NonNull MotionEvent e2, float velocityX, float velocityY) { | ||
| if (e1 == null) return false; | ||
|
|
||
| float deltaX = e2.getX() - e1.getX(); | ||
| float deltaY = e2.getY() - e1.getY(); | ||
| float absDeltaX = Math.abs(deltaX); | ||
| float absDeltaY = Math.abs(deltaY); | ||
|
|
||
| // Check primarily horizontal | ||
| if (absDeltaX > absDeltaY && | ||
| absDeltaX > swipeThreshold && | ||
| Math.abs(velocityX) > swipeVelocityThreshold) { | ||
|
|
||
| boolean swipeLeft = deltaX < 0; | ||
| boolean swipeRight = deltaX > 0; | ||
|
|
||
| // Edge detection | ||
| boolean atLeftEdge = !binding.webview.canScrollHorizontally(-1); | ||
| boolean atRightEdge = !binding.webview.canScrollHorizontally(1); | ||
|
|
||
| if (swipeLeft && atRightEdge) { | ||
| onJumpToPageInDocument(mPage + 1); | ||
| return true; | ||
| } else if (swipeRight && atLeftEdge) { | ||
| onJumpToPageInDocument(mPage - 1); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public void onZoom(float scaleFactor, float focusX, float focusY) { | ||
| zoom(scaleFactor, focusX, focusY, false); | ||
|
|
@@ -520,6 +560,12 @@ public void onZoomEnd() { | |
| } | ||
| } | ||
|
|
||
| private void initializeGestures() { | ||
| ViewConfiguration vc = ViewConfiguration.get(this); | ||
| swipeThreshold = vc.getScaledTouchSlop() * 4; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's a bit too sensitive, maybe 6 would work better |
||
| swipeVelocityThreshold = vc.getScaledMinimumFlingVelocity(); | ||
| } | ||
|
|
||
| private void purgeWebView() { | ||
| binding.webview.removeJavascriptInterface("channel"); | ||
| binding.getRoot().removeView(binding.webview); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Android delivers touch events one at a time. When the user lifts their fingers from a pinch, this always happens as two separate events, one per finger (physically impossible to lift both fingers in the exact same instant)
The first finger to leave the screen produces a ACTION_POINTER_UP event. At this point, the
ScaleGestureDetectorsees that only one finger remains, so it ends the scale gesture and setsisInProgress()to false. however,GestureDetectordoes nothing special on ACTION_POINTER_UP; it never checks for flings on this event type. It only checks for flings when the last finger leaves the screen.The second (last) finger to leave produces a different ACTION_UP event. Now
GestureDetectorwakes up: it checks the remaining finger's velocity, and if it's fast enough, firesonFling. But the scale already ended on the previous event. By the timeonFlingruns and checksisInProgress(), the answer is always false, as the scale ended one event ago.This means the
isInProgress()guard only works if both fingers are still touching the screen whenonFlingfires. But that never happens becauseonFlingrequires the last finger to lift (ACTION_UP), and the scale requires at least two fingers to stay active. The scale always ends first.The same two-event split explains why the pointer count check also doesn't work as intended. The
e1event (the original ACTION_DOWN) was recorded when the first finger touched so that's one pointer. Thee2event (the ACTION_UP) is recorded when the last finger lifts so that's also one pointer, because the other finger already left on the earlier ACTION_POINTER_UP.GestureDetectornever updatese1when a second finger arrives, ande2only ever sees the one finger that remains. So bothe1.getPointerCount()ande2.getPointerCount()are1, and the multi-touch guard sees a gesture that looks entirely single-finger.Can add these logs to GestureHelper to confirm this:
10:51:17 is when I put both fingers down, 10:51:20 is when I released both fingers during a zoom gesture at the side. In terms of
onFling,e1happens at 10:51:17.887 (only 1 pointer), ande2happens at 10:51:20.992.We could add a
mWasScalingtoGestureHelperinside ofgestureView.setOnTouchListenerand set that to true whenscaleDetector.isInProgress(). ThismWasScalingcan replace both the scaleDetector and pointer count checks. Then only resetmWasScalingto false when user taps again, i.e., ACTION_DOWN is received