Leaflet / Leaflet

🍃 JavaScript library for mobile-friendly interactive maps 🇺🇦
https://leafletjs.com
BSD 2-Clause "Simplified" License
40.28k stars 5.76k forks source link

Click on popup text, event not catchable #765

Closed Radamanf closed 11 years ago

Radamanf commented 11 years ago

I can do something wrong but. I've tried to create a click event listener on text (span tag) placed in PopUp and unable to catch click event in any browser, sometimes it does works, but most of the time it's not. If I change span to a tag it can start to work but for one popup first in list, and doesn't work for the rest. It's only me having this issue?

It's easy to open popup from my list of popup, but to open this list of popups from map (popup) is not possible currently.

Here is my code to open popup it works:

    $('.pointpopup').click(function(){
        var pointname = this.id;
        map.setView(point[pointname].getLatLng(),15);
        point[pointname].openPopup();
    });

Here is code to open list from popup, does not work, (test code to fire event):

    $('.map_popup').click(function(){
        //alert('Try to open Accordion ')
        console.log('Try to open Accordion' + $(this).attr('href'));
    })
mourner commented 11 years ago

Please provide a http://jsfiddle.net/ test case so I could take a look at the issue.

Radamanf commented 11 years ago

Here is my link for JS Fiddle, sorry this take some time to insert all in there http://jsfiddle.net/M5Ntr/4/

I assume this problem can be cause by stopPropagation, disableClickPropagation or preventDefault that are in Leaflet.js in several places, here is some basic description for this problem http://www.bennadel.com/blog/1751-jQuery-Live-Method-And-Event-Bubbling.htm

mourner commented 11 years ago

Yep, Leaflet stops click propagation on popup outmost DOM element, but you can put the click on one of the inner elements, e.g. .leaflet-popup-content-wrapper (inspect it in the dev tools to see).

Radamanf commented 11 years ago

Can you please change JS Fiddler accordingly and post link to changes here?

mourner commented 11 years ago

http://jsfiddle.net/M5Ntr/5/

Radamanf commented 11 years ago

OK, I can see that it does make something. Thank you for this, I thought you just ignoring this. But you can see that this workaround doesn't work in both directions, so you initially should click on list and then clicking on popup map starting to work. Do you still think that this is not an issue?

http://jsfiddle.net/M5Ntr/10/

Plus every point should be like "initiallized" before they start to work ...

mourner commented 11 years ago

@Radamanf it's your code's issue, not Leaflet's. You need to rewrite your code to pass DOM elements instead of HTML strings to bindPopup method, adding events when creating the DOM elements for popup contents.

Radamanf commented 11 years ago

Thank you for your explanation. Sorry for being silly.

mourner commented 11 years ago

You're welcome.

wiedikerli commented 11 years ago

is there any final example? (the jsfiddle not work)

seasoup commented 11 years ago

"Yep, Leaflet stops click propagation on popup outmost DOM element, but you can put the click on one of the inner elements, e.g. .leaflet-popup-content-wrapper (inspect it in the dev tools to see).

Why does Leaflet stop the click propagation? It prevents using event delegation which is a very handy technique.

danzel commented 11 years ago

If leaflet didn't stop the click event falling out of the popup, it would bubble up to the map, which would close the popup.

seasoup commented 11 years ago

Here's a quick fiddle showing how to avoid that by checking in the map click handler to make sure the click did not originate inside of the popup div. In the example, I just have three nested divs and make sure that only clocking on the top div works. You could add other divs adjacent to the middle tag and they would still propagate upwards.

http://jsfiddle.net/kEdxr/1/

On Thu, Mar 14, 2013 at 3:39 PM, Dave Leaver notifications@github.comwrote:

If leaflet didn't stop the click event falling out of the popup, it would bubble up to the map, which would close the popup.

— Reply to this email directly or view it on GitHubhttps://github.com/Leaflet/Leaflet/issues/765#issuecomment-14933820 .

seasoup commented 11 years ago

When trying to pas a DOM node to bindPopup() instead of a text string I get this error:

Uncaught Error: NotFoundError: DOM Exception 8

Essentially, this works:

this.rectangle().bindPopup( this.popup_text(), this.popup_options );

but this doesn't:

this.rectangle().bindPopup( $(this.popup_text()), this.popup_options );

The docs say that a string is required, but in the comments above someone said that a DOM element could be passed in, am I just doing it wrong?

On Thu, Mar 14, 2013 at 4:04 PM, Josh Powell seasoup@gmail.com wrote:

Here's a quick fiddle showing how to avoid that by checking in the map click handler to make sure the click did not originate inside of the popup div. In the example, I just have three nested divs and make sure that only clocking on the top div works. You could add other divs adjacent to the middle tag and they would still propagate upwards.

http://jsfiddle.net/kEdxr/1/

On Thu, Mar 14, 2013 at 3:39 PM, Dave Leaver notifications@github.comwrote:

If leaflet didn't stop the click event falling out of the popup, it would bubble up to the map, which would close the popup.

— Reply to this email directly or view it on GitHubhttps://github.com/Leaflet/Leaflet/issues/765#issuecomment-14933820 .

danzel commented 11 years ago

$(this.popup_text()) would be a jquery object wouldn't it? Maybe try $(this.popup_text())[0] instead?

Prinzhorn commented 11 years ago

There are cases were the workaround just doesn't nail it. I am using Leaflet inside a PhoneGap app. I don't use regular links, but instead every element can have a data-url attribute. I'm listening for clicks on [data-url] elements on document level. I do this for three reasons:

  1. I overcome the 300ms click delay by listening for touch events (I'm not listening for the click event actually)
  2. To handle my routing logic centrally (Backbone app)
  3. To handle internal/external links differently (using navigator.app.loadUrl for external links)

Are there any plans to fix this?

If leaflet didn't stop the click event falling out of the popup, it would bubble up to the map, which would close the popup.

Isn't this a bug inside the map code then?

Thanks!

tjwebb commented 11 years ago

Can we solve this?

seasoup commented 11 years ago

I'm using the jquery UI autocomplete instead. Works like a charm

Josh Powell

On Apr 23, 2013, at 5:06 PM, Travis Webb notifications@github.com wrote:

Can we solve this?

— Reply to this email directly or view it on GitHub.

tjwebb commented 11 years ago

But that is not a general solution to the Popup event propagation issue. On Apr 23, 2013 8:12 PM, "seasoup" notifications@github.com wrote:

I'm using the jquery UI autocomplete instead. Works like a charm

Josh Powell

On Apr 23, 2013, at 5:06 PM, Travis Webb notifications@github.com wrote:

Can we solve this?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/Leaflet/Leaflet/issues/765#issuecomment-16897812 .

seasoup commented 11 years ago

Oops sorry, wrong thread.

Josh Powell

On Apr 23, 2013, at 5:20 PM, Travis Webb notifications@github.com wrote:

But that is not a general solution to the Popup event propagation issue. On Apr 23, 2013 8:12 PM, "seasoup" notifications@github.com wrote:

I'm using the jquery UI autocomplete instead. Works like a charm

Josh Powell

On Apr 23, 2013, at 5:06 PM, Travis Webb notifications@github.com wrote:

Can we solve this?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/Leaflet/Leaflet/issues/765#issuecomment-16897812 .

— Reply to this email directly or view it on GitHub.

evilsizord commented 10 years ago

Here's an example of a workaround for delegated events. Each popup is dynamically created, and has a link that has an event attached. It follows mourner's suggestion:

You need to rewrite your code to pass DOM elements instead of HTML strings to bindPopup method, adding events when creating the DOM elements for popup contents.

http://jsfiddle.net/M5Ntr/46/

ghost commented 10 years ago

I have problems using colorbox to show the images that are in a popup, is for this reason too? (pass DOM elements instead of HTML strings). Thanks.

marker.bindPopup("<a href='../pic1' class='pics'><img src='../pic1' height='50px' width='50px'/></a> ");
jQuery('a.pics').colorbox(); 

this does not work

mourner commented 10 years ago

@elPela depends on how the colorbox plugin works... Try opening the popup first and then calling the plugin.

ghost commented 10 years ago

Thanks @mourner!!! can you give me an example of how can I do that? because in my code the popup opens automatically when you click the marker, many thanks!!!

mourner commented 10 years ago

Well, you could do that in a popupopen event listener... Or you can try creating DOM elements, initializing the plugin on them and then passing them to the bindPopup method.

ghost commented 10 years ago

Great @mourner, I will give it a try, any example on the web to help? thanks for your time, leaflet is awesome!

ghost commented 10 years ago

@mourner I tried this and not works!, any help please? thank you...

var div_popup = L.DomUtil.create('div', 'abcpopup');
    div_popup.innerHTML = "<a href='../pic1' class='pics'><img src='../pic1' height='50px' width='50px'/></a> "; 
    jQuery('a.pics').colorbox();        
    marker.bindPopup(div_popup, {maxWidth: '400'}); 
mourner commented 10 years ago

At the moment you call jQuery('a.pics'), there's no a.pics in the DOM. Try doing jQuery(div_popup).find('a.pics').colorbox(); or something like that.

evilsizord commented 10 years ago

Hi elPela, in your example colorbox is not even loading correctly because I think it requires jQuery before 1.9. If you look in Firebug you will see there's an error because $.browser is missing from jQuery 1.10. Try using an earlier jquery.

solarcellsky commented 10 years ago

You can use jQuery migration plugin to resolve it. — Sent from Mailbox for iPhone

On Wed, Jul 17, 2013 at 10:06 PM, evilsizord notifications@github.com wrote:

Hi elPela, in your example colorbox is not even loading correctly because I think it requires jQuery before 1.9. If you look in Firebug you will see there's an error because $.browser is missing from jQuery 1.10. Try using an earlier jquery.

Reply to this email directly or view it on GitHub: https://github.com/Leaflet/Leaflet/issues/765#issuecomment-21114936

ghost commented 10 years ago

Finally I make it work updating leaflet to the last version (0.6.3). Thank you so much for your help @mourner, I really appreciate it, congratulation for leaflet!!!

domenukk commented 10 years ago

Hi, I find this to be a bug.

I'm using a jQuery Plugin (minicolors, a color picker) inside the popup. The pugin sets namespaced events to $(document). So far, so good. The problem: while it work's reasonably well outside of the popup, due to leaflets architecture, the $(document).on event is never triggered as the popup stops propagation.

Now I have to catch all events myself inside the popup and then trigger the jQuery events myself. That's a fix that shouldn't be needed.

Other than that, Leaflet is outstanding. Thank you for it. :)

mourner commented 10 years ago

@domenukk did you really try the latest version? It doesn't stop click propagation in popup at least.

domenukk commented 10 years ago

Version 7-dev? No, I'm still on 6.4. Sorry and thanks for the info.

mourner commented 10 years ago

It should be working on 0.6.4. If it's not for you, provide a minimal test case on JSFiddle that reproduces this.

domenukk commented 10 years ago

@mourner
Hi, here's the fiddle: http://jsfiddle.net/M5Ntr/80/ the colorpicker on the bottom is working, the popup one is not. To make it work, I catch mousedown, mousemove und moseup events inside the popup and then pass them to $(document).triggerHandler atm. But it doesn't feel quite right.

mourner commented 10 years ago

OK, so popup propagates click but not mousedown/move/up.

domenukk commented 10 years ago

Ah okay that makes sense. Well I guess you don't have to support all possible events... After all it's working with a workaround. So for future reference, I've fixed the fiddle. http://jsfiddle.net/M5Ntr/88/

alrick commented 10 years ago

@mourner Hi!

You said did you really try the latest version? It doesn't stop click propagation in popup at least. From which version? I'm trying to check if my problem comes from this or not?

Thanks

mourner commented 10 years ago

The latest stable, 0.6.4

alrick commented 10 years ago

Thanks, I'm using leaflet with mapbox.js which use the 0.6.2, I'll try to upgrade to 0.6.4 myself.

m314 commented 7 years ago

Here's an updated version of http://jsfiddle.net/M5Ntr/46/ using Leaflet 1.0 and JQuery 3.1: https://jsfiddle.net/myjkby8w/

Jimi9 commented 1 year ago

Hello everybody,

I am facing an issue, I have a list of markers, each one populated with a popup. When I try to run a function in the tag of the popup this one instead of running the function when clicked it runs the function for every marker and after that it doesn't allow to use it any longer.

As you can see here my popup is realized like so:

        function changeStatus(stationId, newStatus) {
            //console.log('ciao',stationId)
            const point = points.find(point => point.name === stationId);

            // If the point is found
            if (point) {
                // Update the status of the point
                point.status = newStatus;

                // Find the marker for the point
                const marker = point.marker;
                //console.log('Before updating icon:', marker.options.icon.options.iconUrl);
                //console.log('changStat',stationId,newStatus)
                // Update the marker's icon based on the new status
                switch (newStatus) {
                    case 0:
                        marker.setIcon(
                            L.icon({
                                iconUrl: greyIcon,
                                iconSize: [18, 18],
                            })
                        );
                        break;
                    case 1:
                        marker.setIcon(
                            L.icon({
                                iconUrl: blackIcon,
                                iconSize: [18, 18],
                            })
                        );
                        break;
                    case 2:
                        marker.setIcon(
                            L.icon({
                                iconUrl: greenIcon,
                                iconSize: [18, 18],
                            })
                        );
                        break;
                    case 3:
                        marker.setIcon(
                            L.icon({
                                iconUrl: redIcon,
                                iconSize: [18, 18],
                            })
                        );
                        break;
                }

                //console.log('After updating icon:', marker.options.icon.options.iconUrl);
                point.marker = marker;

                // Update the popup content based on the new status
                marker.setPopupContent(`<br><strong>Name:</strong> ${point.name}
  <br><strong>Latitude:</strong> ${point.coordinates[0]}
  <br><strong>Longitude:</strong> ${point.coordinates[1]}

  <br><strong>Status:</strong><span id="status-popup" style="color: ${newStatus == 3 ? 'red' : newStatus == 2 ? 'green' : newStatus == 1 ? 'black' : 'grey'};">
    ${newStatus == 3 ? 'Shake detected' : newStatus == 2 ? 'OK' : newStatus == 1 ? 'Error (Data delay)' : 'Error'}</span>

  <br><strong>Link: </strong> <a id="linkId" href="#" @click="${fetchXMLData}" target='_blank' >Web-interface</a>`);//point.link
            } //http://${vm.$root.apiUrl}/admin/ in case you want to redirect to admin page

        }

Why is this happening and not following the click event? many thanks