Fix setting of offset for popups.#3526
Conversation
|
Yep, my mistake. |
src/layer/Layer.Popup.js
Outdated
There was a problem hiding this comment.
Naive question: why not this._popup, as it's the popup that will be open then (or pass it as parameter on the initial call to _popupAnchor) ?
|
Cool! Please squash into one commit. |
|
@mourner I don't think we can merge yet,I believe this code has introduced a slight horizontal offset that is not intended. |
|
This will sound really really weird but I think @snkashis solution is totally correct and the offset is happening because of browser rounding in the tip of the popup. Currently the popup tip is 17x17 pixels compare this to a 16x16 pixel tip. A radius 1 circle marker is used for reference on my retina MBP. |
|
interesting @patrickarlt . what do you think @mourner , is this mergeable in current state? |
|
Actually I took the default popup as it sits not on the this branch and measured it all. Left edge of popup content to center of popup tip: 163px It looks like something is off by 1px somewhere I'll see if I can figure it out. |
|
@mourner I think this is good to merge this like it is now. It addresses the problem of not using offset with my latest changes. The slight horizontal offset is still an issue in dfbd0fa which is the commit just before you merged my changes in. Check that commit and look at The popup is placed correctly on the market labeled "Static" and incorrectly on the marker labeled "Draggable" |
|
@snkashis please squash into one commit and I'll merge. |
|
I've filed an issue for the horizontal popup offset in #3586 |
|
Okay @mourner , squash done. |
Fix setting of offset for popups.
|
And...I think we need to revert this because of an issue I just noticed on
|
|
Reverted. |
|
Yup this line https://github.com/Leaflet/Leaflet/blob/master/src/layer/Layer.Popup.js#L63.
|
|
The fix for this is pretty simple actually. You just have to save the originally passed offset somewhere like I'll have a PR for this soon. |
|
An offset keeps getting added if the second (optional) argument to |





Fixes #3525