Deleting Vertices - Wrong Points Removed on Trash

[mikeomeara1]

mapbox-gl-js version: 1.0 mapbox-gl-draw version: 1.0.9

Steps to Trigger Behavior

1. Go to: https://docs.mapbox.com/mapbox-gl-js/example/mapbox-gl-draw/
2. Draw a polygon with more than ~20 points
3. Select more than 10 vertices starting at the 10th point or later
4. Click the "Trash" button
5. At least one (in cases with polygons that have many vertices - many) of the selected points still remain while some un-selected points are removed.

Expected Behavior

All selected points are removed when in direct_select mode and the trash event is executed.

Actual Behavior

Some selected points will still remain. Some un-selected points are removed.

Example 1 - Large number of points

Yields the following after trash:

Example 2 - Small number of points

After Trash:

Remediation

I'm not certain, but I believe this has something to do with the coord_path assigned to every vertex. When the trash event fires on direct_select it does:

state.selectedCoordPaths.sort().reverse().forEach(id => state.feature.removeCoordinate(id));

I believe that the sort() function doesn't take into account that coord_path is a string and when the values get greater than "0.10" the sort function puts the vertices in the wrong order, thus removes the wrong ones.

[mikeomeara1]

After troubleshooting, it appears if you change the following line in direct_select.js (line 168) from:

state.selectedCoordPaths.sort().reverse().forEach(id => state.feature.removeCoordinate(id));

To:

 state.selectedCoordPaths.map(p => {return {coord_path: p, sort_val: parseInt(p.split('.')[p.split(".").length -1], 10)}})
    .sort((a, b) => a.sort_val - b.sort_val)
    .map(p => p.coord_path)
    .reverse()
    .forEach(id => state.feature.removeCoordinate(id));

It appears to resolve the issue (as ugly as it may be). After I've had a chance to test a bit further, we'll deploy this fix to our fork for community review.

If anyone has optimizations or other ideas...they'd be most welcome.

[kkaefer]

@mikeomeara1 thanks for the detailed analysis, it was very helpful in creating a fix in #943.