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

Fix: In direct_select mode it's better to prefer selecting vertices rather than midpoints

Open denull opened this issue 6 months ago • 3 comments

When a polygon or a polyline contains two vertices very close to each other, it can be very tricky to click on one of them and not on the midpoint between them.

The result is very frustrating, because this would create yet another vertex (and if user decides to click multiple times, it would produce even more vertices). This issue is even worse on touch devices (because touchBuffer is much bigger than clickBuffer).

This commit fixes the issue by tweaking the sort order of points: if there are more than one vertex near the cursor, we should always try to select an existing vertex instead of creating a new one.

denull avatar Jul 28 '25 18:07 denull

Hey, @denull. Thank you for your contribution! It's a nice improvement! Could you share the video before and after to visually demonstrate the difference in the behaviour?

I'm also considering integration tests, but unfortunately, we haven't set up anything here yet: only some tests that we run in a Node.js environment with a mocked browser. But I guess we can do it later as a follow-up

underoot avatar Jul 31 '25 07:07 underoot

Could you share the video before and after to visually demonstrate the difference in the behaviour?

Sure thing. Here's the current behavior when trying to disentangle vertices of a LineString in a close proximity to each other. Notice the vertex counter constantly going up due to hitting midpoints instead of existing vertices.

https://github.com/user-attachments/assets/b9fb991d-1187-4ad4-a807-ea749a044eb6

And now, the updated version:

https://github.com/user-attachments/assets/fbaee862-a356-4a02-bb1f-058892086e18

Here, we are able to successfully drag vertices apart without creating a ton of new ones.

denull avatar Aug 01 '25 07:08 denull

Btw, yet another potential improvement would involve sorting vertices based on their distance from the actual cursor position — similar to how polygons are sorted based on their area now. But that would probably require adding an extra dependency (cheap-ruler, for example), so I decided not to include it in this PR.

denull avatar Aug 01 '25 07:08 denull