Fixed: Event listener cancellation and hover handling on web. (#623)

Bug fixes:
- Resolved issue where _map.off() wasn't properly removing event
listeners because it wasn't passing the JavaScript function reference
- Fixed issue where hovered event handling wasn't working propery with
different layers on the map

Added:
- Parse layer ID from feature json object on web.

Author: TarekTolba1

maplibre_gl/CHANGELOG.md

@@ -7,6 +7,7 @@
 * Added `onFeatureHover` to the controller for listening to hover interactions.
 * Fixed: annotationConsumeTapEvents previously had no effect. This has been fixed, and it now properly controls whether tap events on annotations are consumed.
 * Fixed: Avoided calling notifyListeners() on a disposed controller.
+* Fixed: Internal event listeners wasn't properly removed/off in web.
 
 ## [0.22.0](https://github.com/maplibre/flutter-maplibre-gl/compare/v0.21.0...v0.22.0)
 

maplibre_gl/lib/src/controller.dart

@@ -8,7 +8,7 @@ typedef OnMapClickCallback = void Function(
     Point<double> point, LatLng coordinates);
 
 typedef OnFeatureInteractionCallback = void Function(Point<double> point,
-    LatLng coordinates, Annotation annotation, String? layerId);
+    LatLng coordinates, Annotation annotation, String layerId);
 
 typedef OnFeatureDragCallback = void Function(
   Point<double> point,

maplibre_gl_web/lib/src/geo/geojson.dart

@@ -36,18 +36,22 @@ class Feature extends JsObjectWrapper<FeatureJsImpl> {
 
   String get source => jsObject.source;
 
+  String get layerId => jsObject.layer.id;
+
   factory Feature({
     dynamic id,
     required Geometry geometry,
     Map<String, dynamic>? properties,
     String? source,
+    String? layerId,
   }) =>
       Feature.fromJsObject(FeatureJsImpl(
         type: 'Feature',
         id: id,
         geometry: geometry.jsObject,
         properties: properties == null ? jsify({}) : jsify(properties),
         source: source,
+        layer: layerId != null ? FeatureLayerJsImpl(id: layerId) : null,
       ));
 
   Feature copyWith({

maplibre_gl_web/lib/src/interop/geo/geojson_interop.dart

@@ -23,12 +23,14 @@ class FeatureJsImpl {
   external GeometryJsImpl get geometry;
   external dynamic get properties;
   external String get source;
+  external FeatureLayerJsImpl get layer;
   external factory FeatureJsImpl({
     dynamic id,
     String? type,
     GeometryJsImpl geometry,
     dynamic properties,
     String? source,
+    FeatureLayerJsImpl? layer,
   });
 }
 
@@ -42,3 +44,12 @@ class GeometryJsImpl {
     dynamic coordinates,
   });
 }
+
+@JS()
+@anonymous
+class FeatureLayerJsImpl {
+  external String get id;
+  external factory FeatureLayerJsImpl({
+    String id,
+  });
+}

maplibre_gl_web/lib/src/maplibre_web_gl_platform.dart

@@ -7,11 +7,11 @@ class MapLibreMapController extends MapLibrePlatform
   late Map<String, dynamic> _creationParams;
   late MapLibreMap _map;
   dynamic _draggedFeatureId;
-  List<dynamic> _hoveredFeatureIds = [];
   LatLng? _dragOrigin;
   LatLng? _dragPrevious;
   bool _dragEnabled = true;
   final _addedFeaturesByLayer = <String, FeatureCollection>{};
+  final _hoveredFeatureIdsByLayer = <String, List<dynamic>>{};
 
   final _interactiveFeatureLayerIds = <String>{};
 
@@ -109,7 +109,7 @@ class MapLibreMapController extends MapLibrePlatform
     await addImage(imagePath, bytes.buffer.asUint8List());
   }
 
-  _onMouseDown(Event e) {
+  void _onMouseDown(Event e, String? layerId) {
     final isDraggable = e.features[0].properties['draggable'];
     if (isDraggable != null && isDraggable) {
       // Prevent the default map drag behavior.
@@ -528,6 +528,7 @@ class MapLibreMapController extends MapLibrePlatform
       'point': Point<double>(e.point.x.toDouble(), e.point.y.toDouble()),
       'latLng': LatLng(e.lngLat.lat.toDouble(), e.lngLat.lng.toDouble()),
       if (features.isNotEmpty) "id": features.first.id,
+      if (features.isNotEmpty) "layerId": features.first.layerId,
     };
     if (features.isNotEmpty) {
       onFeatureTappedPlatform(payload);
@@ -782,9 +783,9 @@ class MapLibreMapController extends MapLibrePlatform
   void setStyle(dynamic styleObject) {
     //remove old mouseenter callbacks to avoid multicalling
     for (final layerId in _interactiveFeatureLayerIds) {
-      _map.off('mouseenter', layerId, _onMouseEnterFeature);
-      _map.off('mousemove', layerId, _onMouseMoveInFeature);
-      _map.off('mouseleave', layerId, _onMouseLeaveFeature);
+      _map.off('mouseenter', layerId, _handleLayerMouseMove);
+      _map.off('mousemove', layerId, _handleLayerMouseMove);
+      _map.off('mouseleave', layerId, _handleLayerMouseMove);
       if (_dragEnabled) _map.off('mousedown', layerId, _onMouseDown);
     }
     _interactiveFeatureLayerIds.clear();
@@ -1070,34 +1071,37 @@ class MapLibreMapController extends MapLibrePlatform
 
     if (enableInteraction) {
       _interactiveFeatureLayerIds.add(layerId);
-      _map.on('mouseenter', layerId, _onMouseEnterFeature);
-      _map.on('mousemove', layerId, _onMouseMoveInFeature);
-      _map.on('mouseleave', layerId, _onMouseLeaveFeature);
+      _map.on('mouseenter', layerId, _handleLayerMouseMove);
+      _map.on('mousemove', layerId, _handleLayerMouseMove);
+      _map.on('mouseleave', layerId, _handleLayerMouseMove);
       if (_dragEnabled) _map.on('mousedown', layerId, _onMouseDown);
     }
   }
 
-  void _onMouseEnterFeature(Event e) {
-    if (_draggedFeatureId == null) {
-      _map.getCanvas().style.cursor = 'pointer';
-    }
-    _onFeatureHover(e);
-  }
+  void _handleLayerMouseMove(Event e, String layerId) {
+    final currentHoveredFeatures = e.features.map((f) => f.id).toList();
+    final lastHoveredFeatures = _hoveredFeatureIdsByLayer[layerId] ?? [];
+    final features = <String>{
+      ...currentHoveredFeatures,
+      ...lastHoveredFeatures
+    };
+    _hoveredFeatureIdsByLayer[layerId] = currentHoveredFeatures;
 
-  void _onFeatureHover(Event e) {
-    final currentFeatureIds = e.features.map((f) => f.id).toList();
-    final allFeatureIds = <String>{...currentFeatureIds, ..._hoveredFeatureIds};
-    for (final feature in allFeatureIds) {
-      final isCurrentlyHovered = currentFeatureIds.contains(feature);
-      final isPreviouslyHovered = _hoveredFeatureIds.contains(feature);
+    for (final feature in features) {
+      final isCurrentlyHovered = currentHoveredFeatures.contains(feature);
+      final isPreviouslyHovered = lastHoveredFeatures.contains(feature);
       late final String eventType;
       if (isCurrentlyHovered && isPreviouslyHovered) {
         eventType = 'move';
       } else if (isCurrentlyHovered && !isPreviouslyHovered) {
         eventType = 'enter';
+        if (_draggedFeatureId == null) {
+          _map.getCanvas().style.cursor = 'pointer';
+        }
       } else if (!isCurrentlyHovered && isPreviouslyHovered) {
         eventType = 'leave';
       }
+
       onFeatureHoverPlatform({
         'id': feature,
         'point': Point<double>(e.point.x.toDouble(), e.point.y.toDouble()),
@@ -1106,16 +1110,14 @@ class MapLibreMapController extends MapLibrePlatform
       });
     }
 
-    _hoveredFeatureIds = currentFeatureIds;
-  }
-
-  void _onMouseMoveInFeature(Event e) {
-    _onFeatureHover(e);
-  }
-
-  void _onMouseLeaveFeature(Event e) {
-    _map.getCanvas().style.cursor = '';
-    _onFeatureHover(e);
+    final isAnyFeatureHovered = _hoveredFeatureIdsByLayer.values
+        .any((hoveredFeatures) => hoveredFeatures.isNotEmpty);
+    if (isAnyFeatureHovered && _draggedFeatureId == null) {
+      _map.getCanvas().style.cursor = 'pointer';
+    }
+    if (!isAnyFeatureHovered) {
+      _map.getCanvas().style.cursor = '';
+    }
   }
 
   @override

maplibre_gl_web/lib/src/util/evented.dart

@@ -2,13 +2,14 @@ import 'dart:js';
 
 import 'package:maplibre_gl_web/src/geo/geojson.dart';
 import 'package:maplibre_gl_web/src/geo/lng_lat.dart';
+import 'package:maplibre_gl_web/src/geo/point.dart';
 import 'package:maplibre_gl_web/src/interop/interop.dart';
 import 'package:maplibre_gl_web/src/ui/control/geolocate_control.dart';
 import 'package:maplibre_gl_web/src/ui/map.dart';
-import 'package:maplibre_gl_web/src/geo/point.dart';
 
 typedef Listener = dynamic Function(Event object);
 typedef GeoListener = dynamic Function(dynamic object);
+typedef LayerEventListener = dynamic Function(Event object, String layerId);
 
 class Event extends JsObjectWrapper<EventJsImpl> {
   String get id => jsObject.id;
@@ -46,38 +47,51 @@ class Event extends JsObjectWrapper<EventJsImpl> {
 }
 
 class Evented extends JsObjectWrapper<EventedJsImpl> {
+  /// Store listener references so `off` can use the same one.
+  /// Key is a composite of (eventType, layerIdOrListener.hashCode?, listener.hashCode)
+  final _listeners = <String, dynamic>{};
+
+  /// Build a composite key (eventType::layerId::listenerHashCode).
+  String _listenerKey(
+      String type, dynamic layerIdOrListener, LayerEventListener? listener) {
+    return '$type::${layerIdOrListener?.hashCode}::${listener?.hashCode}';
+  }
+
   ///  Adds a listener to a specified event type.
   ///
   ///  @param {string} type The event type to add a listen for.
   ///  @param {Function} listener The function to be called when the event is fired.
   ///    The listener function is called with the data object passed to `fire`,
   ///    extended with `target` and `type` properties.
   ///  @returns {Object} `this`
-  MapLibreMap on(String type, [dynamic layerIdOrListener, Listener? listener]) {
+  MapLibreMap on(String type,
+      [dynamic layerIdOrListener, LayerEventListener? listener]) {
+    final ListenerJsImpl jsFn;
+    final MapLibreMapJsImpl mapJsImpl;
     if (this is GeolocateControl && layerIdOrListener is GeoListener) {
-      return MapLibreMap.fromJsObject(
-        jsObject.on(type, allowInterop(
-          (dynamic position) {
-            layerIdOrListener(position);
-          },
-        )),
+      jsFn = allowInterop(
+        (dynamic position) {
+          layerIdOrListener(position);
+        },
       );
-    }
-    if (layerIdOrListener is Listener) {
-      return MapLibreMap.fromJsObject(
-        jsObject.on(type, allowInterop(
-          (EventJsImpl object) {
-            layerIdOrListener(Event.fromJsObject(object));
-          },
-        )),
+      mapJsImpl = jsObject.on(type, jsFn);
+    } else if (layerIdOrListener is Listener) {
+      jsFn = allowInterop(
+        (EventJsImpl object) {
+          layerIdOrListener(Event.fromJsObject(object));
+        },
       );
+      mapJsImpl = jsObject.on(type, jsFn);
+    } else {
+      jsFn = allowInterop((EventJsImpl object) {
+        listener!(Event.fromJsObject(object), layerIdOrListener);
+      });
+      mapJsImpl = jsObject.on(type, layerIdOrListener, jsFn);
     }
-    return MapLibreMap.fromJsObject(
-        jsObject.on(type, layerIdOrListener, allowInterop(
-      (EventJsImpl object) {
-        listener!(Event.fromJsObject(object));
-      },
-    )));
+
+    _listeners[_listenerKey(type, layerIdOrListener, listener)] = jsFn;
+
+    return MapLibreMap.fromJsObject(mapJsImpl);
   }
 
   ///  Removes a previously registered event listener.
@@ -86,22 +100,17 @@ class Evented extends JsObjectWrapper<EventedJsImpl> {
   ///  @param {Function} listener The listener function to remove.
   ///  @returns {Object} `this`
   MapLibreMap off(String type,
-      [dynamic layerIdOrListener, Listener? listener]) {
-    if (layerIdOrListener is Listener) {
-      return MapLibreMap.fromJsObject(
-        jsObject.off(type, allowInterop(
-          (EventJsImpl object) {
-            layerIdOrListener(Event.fromJsObject(object));
-          },
-        )),
-      );
+      [dynamic layerIdOrListener, LayerEventListener? listener]) {
+    final key = _listenerKey(type, layerIdOrListener, listener);
+    final jsFn = _listeners.remove(key);
+    final MapLibreMapJsImpl mapJsImpl;
+
+    if (layerIdOrListener is Listener || layerIdOrListener is GeoListener) {
+      mapJsImpl = jsObject.off(type, jsFn);
+    } else {
+      mapJsImpl = jsObject.off(type, layerIdOrListener, jsFn);
     }
-    return MapLibreMap.fromJsObject(
-        jsObject.off(type, layerIdOrListener, allowInterop(
-      (EventJsImpl object) {
-        listener!(Event.fromJsObject(object));
-      },
-    )));
+    return MapLibreMap.fromJsObject(mapJsImpl);
   }
 
   ///  Adds a listener that will be called only once to a specified event type.