Fix two-finger pinch-to-zoom in reader modes (#256)#378
Open
aaronbamblett wants to merge 1 commit into
Open
Conversation
Two-finger pinch was almost unusable in both reader modes: the horizontal/vertical drag recognizers in DirectionalSwipeGestureHandler accept the gesture as soon as either finger drifts past drag slop, beating InteractiveViewer's scale recognizer to the arena. Fast pinches occasionally race through; normal pinches don't. Replace InteractiveViewer with zoom_view, which coordinates scroll position with zoom via a shared ScrollController and exposes a forceHoldOnPointerDown flag specifically for the ScrollablePositionedList case (recommended by the package author in the linked Flutter framework issue). ScrollablePositionedList doesn't expose a standard ScrollController, so pin scrollable_positioned_list to yakagami's fork via git URL. The fork adds a 2-line ScrollPosition getter on ScrollOffsetController; the same change is sitting in google/flutter.widgets#535 awaiting upstream merge. Replace DirectionalSwipeGestureHandler's plain HorizontalDragGestureRecognizer / VerticalDragGestureRecognizer with subclasses that track the pointers they personally see and reject their in-flight gesture (via resolve(GestureDisposition.rejected)) when a second pointer arrives. Single-finger swipes still drive the existing swipe-at-chapter-boundary navigation; two-finger gestures fall through to ZoomView. Tap and long-press handlers stay on a nested GestureDetector since they don't compete with multi-touch. Self-tracked pointer state rather than a Listener-maintained counter — pointer-down events reach the gesture recognizer BEFORE they reach outer Listeners in Flutter's dispatch order, so a Listener-maintained count is stale by one event. Tests under test/src/widgets/zoom/. The fixture deliberately includes a no-op ScaleGestureRecognizer so the gesture arena has competition; without it the lone drag recognizer eager-accepts and masks the bug.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #256.
The bug
Reported in #256: two-finger pinch is "almost unusable" on Android — you have to "find the right angle" before it triggers. In practice it works once in ten attempts, never reliably.
Root cause
Gesture-arena race. The reader wraps content in
DirectionalSwipeGestureHandler, whoseHorizontalDragGestureRecognizer/VerticalDragGestureRecognizeraccept the gesture as soon as either finger drifts past drag-slop (~18px). MeanwhileInteractiveViewer's scale recognizer is waiting for an actual scale delta to claim. The drag recognizer almost always wins, eating the pinch. Fast pinches occasionally race through because the scale delta accumulates faster.Verified on a Pixel 9 Pro with an in-app diagnostic counter overlay (since stripped): scale callbacks fired zero times during slow pinches, while the outer drag recognizer's end fired every time.
The fix
Two pieces:
Replace
InteractiveViewerwithzoom_view. Suggested by @DattatreyaReddy in the issue thread itself. It's a Flutter package designed for the "zoom inside a scrollable" case — handles gesture-arena coordination between scale and scroll via a shared controller.Small
HorizontalDragGestureRecognizer/VerticalDragGestureRecognizersubclasses inDirectionalSwipeGestureHandlerthat reject multi-touch. When a second finger lands, the recognizer rejects its in-flight gesture viaresolve(GestureDisposition.rejected)so the scale recognizer can claim. Single-finger swipes (the swipe-at-chapter-boundary feature) are unchanged. Tap and long-press handlers stay on a nested standardGestureDetectorsince they don't compete with multi-touch.Configured
zoom_view'sforceHoldOnPointerDown: trueflag (recommended by the package for use insideScrollablePositionedList) anddoubleTapDrag: truefor the standard double-tap-to-zoom affordance.Dependency situation (please read)
ScrollablePositionedListdoesn't expose a standardScrollController—zoom_viewneeds one to coordinate scroll position with zoom. There's an open upstream PR at google/flutter.widgets#535 (byyakagami, the same author aszoom_view) that adds the neededScrollPositiongetter onScrollOffsetController— 2-line change. It's been open since June 2024.Pending that merge, this PR pins
scrollable_positioned_listto yakagami's fork via git URL inpubspec.yaml. There's a comment in pubspec linking to PR #535 noting the situation; once Google's PR lands, the line can be reverted to apub.devversion.If you'd rather not introduce a git dep, the alternative is to vendor the 2-line change into a local copy of the package. Happy to take that path instead — just say the word.
Tests
Three widget tests under
test/src/widgets/zoom/cover the recognizer behavior:One important note: the widget-test gesture arena is more permissive than the real-device arena. The test fixture deliberately includes a no-op
ScaleGestureRecognizerso the arena has competition — without it, the lone drag recognizer eager-accepts and masks the bug. There's a documentation-only test calling this limitation out explicitly.