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 #1809

Draft
wants to merge 124 commits into
base: master
Choose a base branch
from
Draft

multi!: rewrite gesture handling #1809

wants to merge 124 commits into from

Conversation

josxha
Copy link
Contributor

@josxha josxha commented Jan 23, 2024

Moved the gesture-rewrite branch to the flutter_map repo to have pull requests for this branch on here too.
For previous discussions see #1733.

Changes in addition to the ones listed in #1733:

  • tiles load continuously for doubleTapDragZoom gesture
  • fling gesture has been turned into a gesture option for the drag gesture
  • now all tap gesture only emit events when they have a registered callback (before double tap gestures haven't while regular taps have emitted an event).
  • all gesture animation gets cancelled on a pointer down event.
  • changes to the trackpad (aka. touchpad) gestures

@josxha josxha added this to the v7.0 milestone Jan 23, 2024
@josxha josxha mentioned this pull request Jan 23, 2024
38 tasks
@josxha josxha changed the title multi!: rewrite gesture handling (new) multi!: rewrite gesture handling Jan 23, 2024
@josxha josxha modified the milestones: v7.0, Gesture Rewrite Jan 23, 2024
@JaffaKetchup JaffaKetchup self-requested a review February 10, 2024 14:34
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.

  1. Double tap + drag zoom gesture emits MapEventMove rather than a zoom gesture
  2. Double tap + drag zoom gesture emits 'Unknown' event on completion - need to update example app
2024-02-10.14-34-03.mp4

  1. Fling gesture can no longer be toggled in gesture app

  1. Two-finger drag checkbox doesn't appear to make any difference on Windows 11 trackpad (Lenovo). Is two-finger drag intended for mobile?

  2. Trackpad zoom gesture toggle has no effect, and cannot zoom with trackpad. Instead, scroll gesture controls the trackpad zoom.

image

2024-02-10.14-47-37.mp4

  1. The new system where events aren't emitted unless a callback is directly assigned to listen to that event kinda breaks the whole point of the onMapEvent callback.

  2. Defining/undefining a specific callback does not change whether the event was emitted from the original state (state is not changed properly).


  1. Double tap zoom gesture has no delay when performed on mouse (which is good), but there is delay when gesture performed on trackpad.

I'm a little concerned that we're splitting the mouse, touchpad, and mobile/touchscreen gestures apart. As far as I'm concerned the touchpad gestures should be the same as the mobile/touchscreen gestures. Is that what you're aiming for as well? For example, the pinch zoom gesture is the same pinch that's done on the trackpad.

@josxha
Copy link
Contributor Author

josxha commented Feb 14, 2024

  1. Trackpad zoom gesture toggle has no effect, and cannot zoom with trackpad. Instead, scroll gesture controls the trackpad zoom.

This is expected behaviour. Since the driver release date is 2006 I assume that your touchpad doesn't support the new gestures of flutter.
Depending on the platform and specific trackpad model, the new system might not be used, if not enough data is provided to the Flutter engine by platform APIs. This includes on Windows, where trackpad gesture support is dependent on the trackpad’s driver, and the Web platform, where not enough data is provided by browser APIs, and trackpad scrolling must still use the old PointerScrollSignal system. https://docs.flutter.dev/release/breaking-changes/trackpad-gestures#description-of-change

  1. Two-finger drag checkbox doesn't appear to make any difference on Windows 11 trackpad (Lenovo). Is two-finger drag intended for mobile?

There was up to this point no intention for a two-finger drag on trackpads and I'm not able to perform this gesture on other map libraries. Is this another flutter_map specific gesture?

  1. The new system where events aren't emitted unless a callback is directly assigned to listen to that event kinda breaks the whole point of the onMapEvent callback.

Please note that events haven't been emitted concistently. normal tap gestures have while the double tap hasn't. The implementation in this pr only registers callbacks if they are needed and therefore only emits an event if the user listens to it. Maybe I understood the onMapEvent callback wrong. For my understanding this callback provides all events that happen to the user, not every event that would be possible. If all callbacks are provided to the event system anyways what is the point of the gesture callbacks in the first place?

  1. Double tap zoom gesture has no delay when performed on mouse (which is good), but there is delay when gesture performed on trackpad.

I experience the same behaviour for master. Have you really experienced different behaviours between master and this pr?


I'm a little concerned that we're splitting the mouse, touchpad, and mobile/touchscreen gestures apart. As far as I'm concerned the touchpad gestures should be the same as the mobile/touchscreen gestures.

Touchpads and touchscreens have different gestures. For example: Run the app from the master branch and do a doubleClickDragZoom gesture. On touchpad this zooms the map in and out to allow zooming with one finger. On touchpad however this gesture moves viewport which is the expected touchpad behaviour. If your goal is it to provide the same gestures on trackpad, touchpad and mouse, a high level implementation is not sufficient and we need to use a RawGestureDetector.

For example, the pinch zoom gesture is the same pinch that's done on the trackpad.

The classic windows zoom gesture is to move two fingers vertically on the touchpad. You can use this gesture on the web live demo: https://demo.fleaflet.dev/ More advanced touchpads and mac touchpads support the pinch zoom gesture as well. But we can't assume that this gesture is supported by any touchpad.

@josxha
Copy link
Contributor Author

josxha commented Mar 4, 2024

Just a quick update to this pull request for everyone who's following along. It was decided internally to collect all gestures with their supported platforms and input devices to get a better overview. I created a spreadsheet for this:
https://docs.google.com/spreadsheets/d/10N41ueX9Pr0H3gc89hFQLbRwcqKpeOMXNVrYR-FR0g4/edit?usp=sharing
(note that there are multiple sites)

Looking forward for some feedback or comments to be able to continue.

@josxha josxha added the blocked This issue's resolution can't be worked on right now label Mar 5, 2024
@JaffaKetchup JaffaKetchup marked this pull request as draft March 13, 2024 21:37
@josxha josxha mentioned this pull request May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue's resolution can't be worked on right now
Projects
Status: Review in progress
2 participants