Fix second click popup issue (#1925) #1963
Conversation
ghost
assigned
|
hmm. The deleted Line was added 5 month ago to fix a bug with a yellow border around the close button. See #1537. |
|
Maybe android? That yellow looks like an android thing. |
|
Maybe lets just replace it with stopPropagation? It would fix the current issue while not affecting the previous close button fix in theory. |
|
Ok. It's an Android Issue with the border and I can reproduce it in the linked fiddle. I'll try to call stopPropagation. Lg fastrde |
|
stopPropagation is already called. The problem is that _skipped not resets when propagation is stopped. |
|
Oh, now I get what you mean. But I think there's no need to introduce a public |
|
(had typo in prev comment, fixed to |
|
Testcase at http://www.fastr.de/tests/leaflet-clickfix.html |
|
Looks good! But it still removes |
|
thx again for the close look. I've concentrated me on L.DomEvent and overlooked the missing line. |
| @@ -143,7 +143,7 @@ L.Popup = L.Class.extend({ | |||
| closeButton.href = '#close'; | |||
| closeButton.innerHTML = '×'; | |||
| L.DomEvent.disableClickPropagation(closeButton); | |||
|
|
|||
|
Nice! Now that this small change built into a lot of commits, could you please rebase it into one commit (only including DomEvent fixes, without whitespace changes in Popup?) |
|
so, now i have learned to rebase ;-) and hope it's ready to merge. |
Fix second click popup issue (#1925)
|
Yep! Thanks a lot Fabian :) |
|
Hi, any idea when's the next release that will include this fix? |
|
Not sure, maybe in a few weeks. |
|
Nothing prevents you from using the master version though. |
…eClickPropagation logic Test case for Leaflet#1963
…eClickPropagation logic Test case for Leaflet#1963
…eClickPropagation logic Test case for Leaflet#1963
…eClickPropagation logic Test case for Leaflet#1963
* DomEventSpec.js: use on/off instead of add/removeListener aliases * DomEventSpec.js: use happen.click instead of custom function (for uniformity) * DomEventSpec.js: minor refactoring Make use of more `sinon` features (for uniformity with other tests) * DomEventSpec.js: tests for disableScrollPropagation and disableClickPropagation DomEvent.js: fix typo * DomEventSpec.js: test to ensure stopPropagation does not break disableClickPropagation logic Test case for #1963
Deleted one line from L.Popup to fix the issue described in Issue #1925
The click events doesn't propagate to the map when you click the close button so the _fakeStop called in disableClickPropagation disable the first real mapClick.
I tested propagation in Firefox 22 and Chrome 28.0.1500.95 and it seems to work.
lg
fastrde