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

multi!: rewrite gesture handling #1733

Closed
wants to merge 113 commits into from
Closed

Conversation

josxha
Copy link
Contributor

@josxha josxha commented Nov 21, 2023

Main Issue:

Current state of development:

Move gestures

  • drag
  • flingAnimation
  • twoFingerMove (old: pinchMove)

Zoom gestures

  • twoFingerZoom (old: pinchZoom)
  • doubleTapZoomIn (old: doubleTapZoom)
  • doubleTapDragZoom
  • scrollWheelZoom
  • trackpadZoom

Rotate gestures

  • twoFingerRotate (old: rotate)
  • ctrlDragRotate (old: hidden gesture)

Gesture callbacks

  • onTap
  • onLongPress
  • onSecondaryTap
  • onSecondaryLongPress (new)
  • onTertiaryTap (new)
  • onTertiaryLongPress (new)
  • onPointerDown
  • onPointerUp
  • onPointerCancel
  • onPointerHover

Changes along the way:

  • rewrote FlutterMapInteractiveViewer as MapInteractiveViewer
  • created gestures as objects
  • remove PositionedTapDetector2
  • add new callbacks for secondaryLongPress, tertiaryTap, tertiaryLongPress gestures
  • fix [BUG] Double-tap-drag gesture doesn't cause new tiles to load after completion #1602
  • fix [FEATURE] Revert to using single onPanUpdate to detect pan events #1729
  • fix [FEATURE] Scroll/zoom should stop animation #1767
  • remove typdefs for PointerDownCallback, PointerUpCallback, PointerCancelCallback, PointerHoverCallback, MapEventCallback, IsKeyCursorRotationTrigger
  • turn MoveAndRotateResult into a class
  • Changed InteractionOptions.flags into InteractionOptions.enabledGestures that still supports flags
  • Remove debugMultiFingerGestureWinner, enableMultiFingerGestureRace, rotationWinGestures, pinchMoveWinGestures and pinchZoomWinGestures (use case unclear)
  • move directory /gestures to /map/gestures since it part of it
  • rename InterativeFlagsPage to more general GesturesPage
  • remove hidden gesture setNorthOnClick (press CTRL + click to rotate to 0°)
  • remove CursorRotationBehaviour
  • remove CursorKeyboardRotationOptions, merge gesture with EnabledGestures and keys option with InteractiveOptions
  • Use more descriptive names for InteractiveFlags: pinchMove -> twoFingerMove, doubleTapZoom -> doubleTapZoomIn, rotate -> twoFingerRotate
  • Remove MapEventSource.cursorKeyboardRotation in favor of ctrlDragRotateStart, ctrlDragRotateEnd and ctrlDragRotate

@josxha josxha self-assigned this Nov 21, 2023
@josxha josxha changed the title refactor: Gesture handling refactor: gesture handling Nov 21, 2023
@josxha josxha added the blocked This issue's resolution can't be worked on right now label Nov 26, 2023
@josxha josxha added this to the v7.0 milestone Dec 2, 2023
add polymorthism to gesture services
@josxha josxha marked this pull request as draft January 19, 2024 21:20
* add trackpad zoom gesture to EnabledGestures, as event and as service

* split both trackpad gestures into two services

* trackpad zoom center on cursor

* rename variable
@josxha
Copy link
Contributor Author

josxha commented Jan 20, 2024

Marking this pull request as ready to review again.

Some thoughts:

  • Touchpad (aka. Trackpad) rotate gesture to rotate the map hasn't been included because I'm not able to implement this without a Macbook. It could be that PointerPanZoomUpdateEvent.rotation provides the necessary rotation data but I can't tell.
  • The two finger scroll gesture hasn't been included (is not supported on master either). It seems to me that this event gets turned into a pan gesture with a ScaleUpdateDetails.pointerCount of 1.
  • Trackpad behaviour is extremely platform dependent. Web uses the legacy trackpad service, modern windows and macos devices use the new trackpad service.
  • The legacy trackpad service doesn't zoom on the cursor but on the center on the map.

@josxha josxha marked this pull request as ready for review January 20, 2024 21:35
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still untested, but here's review round 2 :D. Mostly just smaller stuff now, and there's less than it looks at first glance.
I would like for @mootw @TesteurManiak or @ibrierley to also review this PR, and any other community reviews also appricated! It's quite a big PR, so want to make sure I/we haven't missed anything.

lib/src/map/controller/map_controller.dart Outdated Show resolved Hide resolved
lib/src/map/controller/map_controller.dart Outdated Show resolved Hide resolved
lib/src/map/options/enabled_gestures.dart Outdated Show resolved Hide resolved
lib/src/map/options/map_options.dart Show resolved Hide resolved
lib/src/map/options/enabled_gestures.dart Outdated Show resolved Hide resolved
lib/src/map/options/enabled_gestures.dart Outdated Show resolved Hide resolved
lib/src/map/options/enabled_gestures.dart Outdated Show resolved Hide resolved
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • One bug with cursor keyboard rotation. Rotation is not tracking the cursor properly. I remember having a similar issue when implementing the algorithm initially. Maybe double checking that nothing in the implementaiton itself has changed, and every variable is being changed/reset at the correct points.

  • Cursor keyboard rotation also no longer sets rotation to the angle of cursor immediately when clicking.

2024-01-22.20-38-47.mp4

  • One bug with fling event end/finished event not being emmitted (when fling is cancelled with tap (as shown in video), or when finished naturally)
2024-01-22.20-47-41.mp4

  • Double tap zoom no longer emits correct event.
PR Master
!pr master

  • Can no longer fling if drag gesture is disabled. On master, enabling fling without drag would start a fling if the normal conditions were met, but wouldn't drag/move if not, and wouldn't move whilst normal dragging.

  • Double tap + drag gesture now loads tiles whilst zooming. master behaviour isn't correct either. The correct behaviour is that new tiles shouldn't load and old ones shouldn't be pruned whilst moving, and the refresh should occur when the gesture completes.

  • On tap events and callbacks are no longer triggered.

  • Can't seem to perform the new trackpad zoom gesture. Can you explain how to do it?

Still untested on any touch devices. Will perform that testing later.

@josxha
Copy link
Contributor Author

josxha commented Jan 23, 2024

Branch moved, see new pr here: #1809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
3 participants