Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Direction of drag for selecting multiple images #327

Closed
jywarren opened this issue Jul 5, 2019 · 11 comments · Fixed by #334 or #344
Closed

Direction of drag for selecting multiple images #327

jywarren opened this issue Jul 5, 2019 · 11 comments · Fixed by #334 or #344

Comments

@jywarren
Copy link
Member

jywarren commented Jul 5, 2019

Hi! I'm really enjoying the multiple image select in the latest release. I noticed that I seem to be able to select only by dragging from lower-right to upper-left, but no other direction (i.e. not from NE>SW or SW>NE or NW>SE). I was wondering if we could adapt the selection math to address this?

image

Potentially related to #29 and #244 -- thanks!

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

That's odd. The bounds code seems to be direction invariant here:

_addSelections: function(e) {
var box = e.boxZoomBounds,
i = 0;
this.eachLayer(function(layer) {
var edit = layer.editing;
for (i = 0; i < 4; i++) {
if (box.contains(layer.getCorner(i))) {
edit._deselect();
L.DomUtil.addClass(layer.getElement(), 'selected');
if (!this.toolbar) { this._addToolbar(); }
break;
}
}
}, this);
},

Wonder what's up.

@sashadev-sky
Copy link
Member

I think it may have something to do with flipping axis or something else very hard to debug 🙄I will check it out right now and see if I can work around it by flipping the axis myself and checking again

@sashadev-sky
Copy link
Member

@jywarren you can drag from any direction, but you must make the area a lot larger to do this from any direction other than bottom-left:

bug

@sashadev-sky
Copy link
Member

I think it has to do with this line:

L.DomUtil.setPosition(this._box, bounds.min);

and this:
https://github.com/Leaflet/Leaflet/pull/5204/files

@jywarren
Copy link
Member Author

jywarren commented Jul 11, 2019 via email

@sashadev-sky
Copy link
Member

@jywarren ah! Seeing that too now. It seems to happen at closer zoom levels. I will double check the calculations for these zoom levels now. It is either that or deselection is being triggered right after selection, which was the original reason why the image could only be selected from "bottom - right". These offsets get pretty confusing - it would be nice if we could try to write a wiki outlining which methods (project, point, ltlng, containerPointToLatLng, etc.) to use when for future contributors!

@sashadev-sky sashadev-sky reopened this Jul 11, 2019
@sashadev-sky
Copy link
Member

Point 1: you must not release shift-key until after you release drag or deselection will fire. is this ok? The other alternative was to make deselection happen on mousedown on the map instead, but I didn't like how that looked as much in action

@sashadev-sky
Copy link
Member

Point 2: the bug seems to reproduce for me when I zoom in a lot and zoom out. Once it appears it does not go away under any zoom until a page refresh. Confirming the bug is due to the bounds not being properly read, and not an event handling issue.

@sashadev-sky
Copy link
Member

zoom level 10. attempting to select the topLeft corner of the img on the very left. the Lng is pretty far out of bounds (see console)
Screen Shot 2019-07-11 at 7 58 37 PM

@sashadev-sky
Copy link
Member

@jywarren ok I did a bunch of runs and the behavior I am seeing is that the bug actually occurs after panning the map left or right (which would make sense with the idea of a lng mismatch) and does not have to do with zoom. We just tend to need to pan when we zoom.

@sashadev-sky
Copy link
Member

I believe its fixed now! adding tests. Might also start this wiki page starting with normalizing after a pan

@sashadev-sky sashadev-sky mentioned this issue Jul 12, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants