mapbox-gl-js icon indicating copy to clipboard operation
mapbox-gl-js copied to clipboard

Marker needs more events such as "click"

Open cs09g opened this issue 6 years ago • 25 comments

Motivation

Marker has only three events "dragstart", "drag", "dragend". What if you want to get something from marker when it's cliked? You can use addEventListener on its element from getElement but the data from DOM Listener doesn't include what Marker object has. e.g. position of Marker, its draggability, etc.

So, it needs more event types including "click", "dblclick", "mouseenter", etc. Then we can get what we want when we need.

Design Alternatives

I'm not so sure about inner-logic how it implements for now. But I can guess, the whatever events are added into map's container. image

I tried to use preventDefault and stopPropagation but it does not work because it actually does not have a event on it.

  • Use case for needs of prevendDefault, stopPropagation thing
    1. There are two markers on map.
    2. Add event on each marker but second marker should not trigger map's event.

Mock-Up

The look should be like: marker.on(type, listener, stopPropagation); stopPropagation for prevent map's event.

Implementation

Following is the workaround:

// for map
map.on("click", function(e) {
  if (e.originalEvent.target !== marker.getElement()) {
    // it's map!
  }
});

// for marker
map.on("click", function(e) {
  if (e.originalEvent.target === marker.getElement()) {
    // it's marker!
  }
});

It seems good when we have only one marker, but if we have lots of markers, we can not use condition like:

if (e.originalEvent.target !== marker.getElement() &&
   e.originalEvent.target !== marker2.getElement() &&
  ... ) {

Sorry about mixing up with adding click event and needs of stopPropagation thing, I think it should be considered when the event things are implemented.

here is fiddle: https://jsfiddle.net/roqh9gjw/4/

cs09g avatar Jan 21 '19 09:01 cs09g

A non ideal way to do this is to add the event listener to the marker element - I've added that on to your fiddle over at https://jsfiddle.net/qnr246xd/ .

I think it definitely makes sense to have marker based events - eg if clicking a marker on the map needs to trigger a scroll in a list in html instead of showing a popup.

martsie avatar Mar 05 '19 04:03 martsie

@martsie Yes, absolutely the implementation as workround is not ideal. The events should be added on the Marker object itself but element. What I meant was the same as you wrote on your fiddle. Maybe my writing on the section(Implementation) made you confused.

cs09g avatar Mar 05 '19 04:03 cs09g

I accidentally left some code in at the bottom of the fiddle that was a distraction. https://jsfiddle.net/a26pxgr5/ is the latest. The difference is that in your implementation you are adding the event listener to the map - whereas in the jsfiddle I posted the click event listener is added to the marker element itself. I think from a developer-experience perspective it makes better sense to be assigning a click event to a marker element than the map.

martsie avatar Mar 05 '19 06:03 martsie

Umm, that's why my code is workaround. It should be used as you code to get position of marker:

el.addEventListener('click', function(){
  console.log(marker1.getLngLat());
})

It's not part of API, and we should add addEventListener explicitly which is just Web API. It's suggestion for making the functionality as a part of mapbox API no matter the inner logic is the same as you suggested or other ways.

e.g.) OpenLayers' on for Overlay

cs09g avatar Mar 05 '19 06:03 cs09g

Any update?

secretgspot avatar Sep 19 '19 00:09 secretgspot

I use marker.on('dragend', function(e) ... and there is no way to do e.stopPropagation(), therefore I must (a) detect distance dragged in 'dragend' event to detect a "click", and (b) somehow set a global variable with a timeout that a marker was clicked, so in my map.on('click' function() ... I can ignore if that marker-clicked-global-variable is set...

ghost avatar Oct 25 '19 23:10 ghost

Like so:

		marker.on('dragend', function(e) {
			var that = this;
			var marker = e.target;
			var new_lnglat = marker.getLngLat();
			var new_pos = marker._pos;
			var old_pos = map.project([marker.lon, marker.lat]);
			var dist = Math.sqrt((new_pos.x - old_pos.x)**2 + (new_pos.y - old_pos.y)**2);
			
			if (dist < 16) {
				// "clicked"
				setTimeout(function() {
					if (!that.getPopup().isOpen()) that.togglePopup();
				}, 20);
			} else {
				// "dragged"
				blink_location(marker.id);
				var row = $("tr[data-location_id='" + marker.id + "']");
				toggle_location_edit(row, true);
				row.find('.location-edit-lat').val(new_lnglat.lat.toFixed(3))
				row.find('.location-edit-long').val(new_lnglat.lng.toFixed(3))
			}
			
			marker.lat = new_lnglat.lat;
			marker.lon = new_lnglat.lng;
		});

ghost avatar Oct 25 '19 23:10 ghost

Catching click events on marker would make life much easier. Right now it's hard.

nijynot avatar Dec 18 '19 10:12 nijynot

Any update?...

Juarrow avatar Feb 12 '20 09:02 Juarrow

+1 on this, being able to handle marker clicks with custom events is an important part of creating a rich UI

atyshka avatar May 22 '20 20:05 atyshka

The documentation shows how to add a marker in the first doc.

It then proceeds to explain all other things through the use the more complex layers.

Please add marker.on() event handlers to make markers more useful for basic UI.

PascalPixel avatar Jun 09 '20 13:06 PascalPixel

+1 on this.

I want to intercept click events on HTML map markers while being able to access the markers location.

I've worked around this using a DOM event listener in combination with a closure as follows:

    // the Mapbox GL JS API does not provide a mechanism for getting click events
    // on HTML markers. We can hook the DOM click event on the marker element but
    // that then does not give us the location or the marker the was clicked.
    //
    // By using a closure we can associate the click event with it's corresponding
    // marker. 

    let markerElement = <some div element>
    let marker = <marker Instance>
    let popup = <popup Instance>
    let map = <mapbox instance>

    function createClickListener() {
      return function() {

        // marker location is available as marker.getLngLat()

        popup.setLngLat( marker.getLngLat() ).addTo( map );
      }
    }

    this.onClickListener = createClickListener();
    markerElement.addEventListener( 'click', this.onClickListener ); 

Yermo avatar Jul 09 '20 16:07 Yermo

Please add this, at least "click" event but more would be preferable. Also an easy way to detach handlers or a way to pass option to marker#remove method that could also detach handlers.

comatory avatar Sep 14 '20 14:09 comatory

Would be great to have this feature from library. In the meantime you could use this workaround and build your own onClick method by extending the class: https://bl.ocks.org/chriswhong/8977c0d4e869e9eaf06b4e9fda80f3ab

bart avatar Oct 10 '20 10:10 bart

+1 on this.

christopherritter avatar Apr 04 '21 14:04 christopherritter

+1

julogav avatar Apr 28 '21 15:04 julogav

Sort of solution for the marker;

I had to hijack by creating my own onClick event which works and gives me what I need for over a year now.

	// Override internal functionality
	mapbox.Marker.prototype.onClick = function(handleClick) {
		this._handleClick = handleClick;
		return this;
	};
	mapbox.Marker.prototype._onMapClick = function(t) {
		const targetElement = t.originalEvent.target;
		const element = this._element;
		if (this._handleClick && (targetElement === element || element.contains((targetElement)))) {
			this.togglePopup();
			this._handleClick();
		}
	};
	const marker = new mapbox.Marker({ color: createColor(property) })
		.setLngLat([lng || lon, lat])
		.setPopup(popup)
		.onClick(() => {
				dispatch('showpreview', property.id);
				//map.setCenter([lon, lat]);
			})
		.addTo(map);

hopefully that helps. But it would be great if we had Marker.on('click', => {})

you can see full code here: https://github.com/secretgspot/cariari-realty/blob/master/src/components/Map/Marker.svelte

secretgspot avatar Apr 29 '21 20:04 secretgspot

this does help! thanks. but still, why such an easy obvious call doesnt exsit to begin with? leaflet, coooome on!!!

I had to hijack by creating my own onClick event which works and gives me what I need for over a year now.

	// Override internal functionality
	mapbox.Marker.prototype.onClick = function(handleClick) {
		this._handleClick = handleClick;
		return this;
	};
	mapbox.Marker.prototype._onMapClick = function(t) {
		const targetElement = t.originalEvent.target;
		const element = this._element;
		if (this._handleClick && (targetElement === element || element.contains((targetElement)))) {
			this.togglePopup();
			this._handleClick();
		}
	};
	const marker = new mapbox.Marker({ color: createColor(property) })
		.setLngLat([lng || lon, lat])
		.setPopup(popup)
		.onClick(() => {
				dispatch('showpreview', property.id);
				//map.setCenter([lon, lat]);
			})
		.addTo(map);

hopefully that helps. But it would be great if we had Marker.on('click', => {})

you can see full code here: https://github.com/secretgspot/cariari-realty/blob/master/src/components/Map/Marker.svelte

julogav avatar May 07 '21 19:05 julogav

this does help! thanks. but still, why such an easy obvious call doesn't exsit to begin with? leaflet, coooome on!!!

it only doesn't exist for individual marker (dunno why), but they do have built-in onClick for marker when you provide it as large dataset of markers. I too wish they would make it for individual marker so I wouldn't have to hijack actions and possibly deal with bugs in the future

secretgspot avatar May 19 '21 01:05 secretgspot

cool, good to know!

thank you very much for your answer

Juliet Gavison 0526737323

On Wed, May 19, 2021 at 4:15 AM secretgspot @.***> wrote:

this does help! thanks. but still, why such an easy obvious call doesn't exsit to begin with? leaflet, coooome on!!!

it only doesn't exist for individual marker (dunno why), but they do have built-in onClick for marker when you provide it as large dataset of markers. I too wish they would make it for individual marker so I wouldn't have to hijack actions and possibly deal with bugs in the future

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mapbox/mapbox-gl-js/issues/7793#issuecomment-843672296, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIAD3VRS3O62CDWEBISLBDLTOMGMRANCNFSM4GRJTVEQ .

julogav avatar May 19 '21 07:05 julogav

3 years later and we still waiting for a click event?

rdzar avatar Mar 12 '22 14:03 rdzar

Yes, indeed we do

3 years later and we still waiting for a click event?

SpiridonRoxana avatar Jun 02 '22 20:06 SpiridonRoxana

Still waiting 😴

LuisJoseSanchez avatar Sep 03 '22 12:09 LuisJoseSanchez

Oh, come on! :(

alexblaster85 avatar Apr 25 '23 22:04 alexblaster85

still waiting :/

vindicoMeg avatar Apr 12 '24 10:04 vindicoMeg