Attach marker click listener to map to avoid setTimeout

mapbox · Sep 1, 2016 · c8d07f9 · c8d07f9
1 parent 3d51e06
commit c8d07f9
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 13 deletions.
diff --git a/js/ui/marker.js b/js/ui/marker.js
@@ -24,7 +24,7 @@ function Marker(element, options) {
     this._offset = Point.convert(options && options.offset || [0, 0]);
 
     this._update = this._update.bind(this);
-    this._onClick = this._onClick.bind(this);
+    this._onMapClick = this._onMapClick.bind(this);
 
     if (!element) element = DOM.create('div');
     element.classList.add('mapboxgl-marker');
@@ -43,9 +43,14 @@ Marker.prototype = {
         this.remove();
         this._map = map;
         map.getCanvasContainer().appendChild(this._element);
-        this._element.addEventListener('click', this._onClick);
         map.on('move', this._update);
         this._update();
+
+        // If we attached the `click` listener to the marker element, the popup
+        // would close once the event propogated to `map` due to the
+        // `Popup#_onClickClose` listener.
+        this._map.on('click', this._onMapClick);
+
         return this;
     },
 
@@ -58,11 +63,11 @@ Marker.prototype = {
      */
     remove: function () {
         if (this._map) {
+            this._map.off('click', this._onMapClick);
             this._map.off('move', this._update);
             this._map = null;
         }
         DOM.remove(this._element);
-        this._element.removeEventListener('click', this._onClick);
         if (this._popup) this._popup.remove();
         return this;
     },
@@ -114,8 +119,13 @@ Marker.prototype = {
         return this;
     },
 
-    _onClick: function(event) {
-        if (this._popup) this.togglePopup();
+    _onMapClick: function(event) {
+        var targetElement = event.originalEvent.target;
+        var element = this._element;
+
+        if (this._popup && (targetElement === element || element.contains(targetElement))) {
+            this.togglePopup();
+        }
     },
 
     /**

diff --git a/js/ui/popup.js b/js/ui/popup.js
@@ -60,14 +60,7 @@ Popup.prototype = util.inherit(Evented, /** @lends Popup.prototype */{
         this._map = map;
         this._map.on('move', this._update);
         if (this.options.closeOnClick) {
-            // We attach the `Map#click` listener asynchronously so that it is
-            // possible to open a popup within a `click` event on a child of
-            // the map's container. If we attached the listener syncronously
-            // the popup would close when the click event propogated to `map`.
-            var that = this;
-            setTimeout(function() {
-                that._map.on('click', that._onClickClose);
-            }, 0);
+            this._map.on('click', this._onClickClose);
         }
         this._update();
         return this;