flutter_map icon indicating copy to clipboard operation
flutter_map copied to clipboard

[SPEC] Overhaul gesture system

Open josxha opened this issue 2 years ago • 7 comments

Request for comments

Motivation

As mentioned by @JaffaKetchup in https://github.com/fleaflet/flutter_map/issues/1529#issue-1718239936, "much of the [gesture handling] code that does this was written in the first few releases, and has not been significantly changed since. [...] Gesture handling at this low-level often differs slightly between platforms, which means that manual testing succeeds on one platform, but a bug occurs on another."

Rewrite Proposals

  • Break down and structure the FlutterMapInteractiveViewer class to improve readablity and reduce its complexity.
  • Remove the usage of PositionedTapDetector2 and merge its GestureDetector with the one in FlutterMapInteractiveViewer.
  • Use the high level GestureDetector as much as possible. This ensures the use of flutters GestureArena.
  • Get rid of RawGestureDetector to avoid platform specific bugs (see issue tracker here https://github.com/fleaflet/flutter_map/issues/1529).
  • ~~Use boolean fields in InteractiveFlag and MultiFingerGesture. This simplifies internal usage, is more beginner friendly in our public API and has no performance penalty. I'll write some code examples in the next comment.~~
  • ~~https://github.com/fleaflet/flutter_map/issues/1732~~
  • open for extendibility like https://github.com/fleaflet/flutter_map/issues/1432

If feasible do the following changes:

  • https://github.com/fleaflet/flutter_map/issues/1729

More resources

josxha avatar Nov 03 '23 07:11 josxha

Example implementation of the new `InteractiveFlags` class
@immutable
class InteractiveFlags {
  const InteractiveFlags._({
    required this.drag,
    required this.flingAnimation,
    required this.pinchMove,
    required this.pinchZoom,
    required this.doubleTapZoom,
    required this.doubleTapDragZoom,
    required this.scrollWheelZoom,
    required this.rotate,
  });

  const InteractiveFlags.all({
    this.drag = true,
    this.flingAnimation = true,
    this.pinchMove = true,
    this.pinchZoom = true,
    this.doubleTapZoom = true,
    this.doubleTapDragZoom = true,
    this.scrollWheelZoom = true,
    this.rotate = true,
  });

  const InteractiveFlags.none({
    this.drag = false,
    this.flingAnimation = false,
    this.pinchMove = false,
    this.pinchZoom = false,
    this.doubleTapZoom = false,
    this.doubleTapDragZoom = false,
    this.scrollWheelZoom = false,
    this.rotate = false,
  });

  /// Enable panning with a single finger or cursor
  final bool drag;

  /// Enable fling animation after panning if velocity is great enough.
  final bool flingAnimation;

  /// Enable panning with multiple fingers
  final bool pinchMove;

  /// Enable zooming with a multi-finger pinch gesture
  final bool pinchZoom;

  /// Enable zooming with a single-finger double tap gesture
  final bool doubleTapZoom;

  /// Enable zooming with a single-finger double-tap-drag gesture
  ///
  /// The associated [MapEventSource] is [MapEventSource.doubleTapHold].
  final bool doubleTapDragZoom;

  /// Enable zooming with a mouse scroll wheel
  final bool scrollWheelZoom;

  /// Enable rotation with two-finger twist gesture
  ///
  /// For controlling cursor/keyboard rotation, see
  /// [InteractionOptions.cursorKeyboardRotationOptions].
  final bool rotate;

  bool hasMultiFinger() => pinchMove || pinchZoom || rotate;

  /// This constructor gives wither functionality to the model
  InteractiveFlags withFlag({
    bool? pinchZoom,
    bool? drag,
    bool? flingAnimation,
    bool? pinchMove,
    bool? doubleTapZoom,
    bool? doubleTapDragZoom,
    bool? scrollWheelZoom,
    bool? rotate,
  }) =>
      InteractiveFlags._(
        pinchZoom: pinchZoom ?? this.pinchZoom,
        drag: drag ?? this.drag,
        flingAnimation: flingAnimation ?? this.flingAnimation,
        pinchMove: pinchMove ?? this.pinchMove,
        doubleTapZoom: doubleTapZoom ?? this.doubleTapZoom,
        doubleTapDragZoom: doubleTapDragZoom ?? this.doubleTapDragZoom,
        scrollWheelZoom: scrollWheelZoom ?? this.scrollWheelZoom,
        rotate: rotate ?? this.rotate,
      );
}

This would make the following changes:

 final int flags;
// turns into
final InteractiveFlags flags;

flags: InteractiveFlags.all - InteractiveFlags.rotate,
// turns into
flags: InteractiveFlags.all(rotate: false), 

if (InteractiveFlag.hasDoubleTapZoom(newOptions.flags)) {
// turns into
if (newOptions.flags.doubleTapZoom) {

josxha avatar Nov 03 '23 07:11 josxha

If we are looking at this, could it be possible to add a feature request to be able to get a LatLng from a click on the map please?

hobleyd avatar Nov 03 '23 20:11 hobleyd

Hi @hobleyd, This is already possible. Use onTap inside MapOptions. Unless I'm misunderstanding something?

JaffaKetchup avatar Nov 03 '23 20:11 JaffaKetchup

Thanks. I missed that. Humblest apologies.

hobleyd avatar Nov 03 '23 20:11 hobleyd

No worries!

JaffaKetchup avatar Nov 03 '23 20:11 JaffaKetchup

Currently blocked by:

  • [x] specify the scope of animations in flutter_map
  • [x] implement the specified animation logic to the map controller.

josxha avatar Nov 06 '23 12:11 josxha

I think the rough rule on scope of animations is I think the rough rule is we provide basic animations in response to gestures only. It's for plugins to provide animations for programmatic controls.

JaffaKetchup avatar Nov 06 '23 17:11 JaffaKetchup