Exciting! Ping me if you want a code review once this nears a shippable state.

Any particular reason for moving away from ES6 syntax?

@kelvinabrokwa classes make hiding private vars via closures pretty hard, but most of the other changes are just my default writing style coming through. In time I'd like to find which syntax is best for perf and go with that in the places it maters. Please feel free to suggests syntax changes you think would help.

I've spent the day trying to outline the api this branch is working towards into the api.md doc. I'd love some input on this as I continue to work. It is a pretty big shift in how Draw works so making sure it lets you do things like select a feature without requiring too much of the developer is key.

https://github.com/mapbox/mapbox-gl-draw/blob/b97cd9fc3a1cb24aeff596690d75215bbbde8e38/API.md

Thanks!

/cc @lucaswoj @kirach @drewbo @kelvinabrokwa @averas @tmcw @scothis @mayagao

Great progress @mcwhittemore! I will give it a more thorough review but one thing that immediately caught my attention is the somewhat ambiguous description of id's. It says in the description of the add method that it will return feature.id, but in the examples later on it's clear that the id is present on the geometry rather than the top level feature. Is there any particular reason you've decided to maintain id's on the geometries of the features rather than the features themselves btw? If the Draw API always maintains and exposes features regardless of what GeoJSON you provide it's perhaps more intuitive to maintain the id's on the features.

To minimise confusion it's perhaps also best to either base the examples on features, or rename the variables to "geojson" etc., where you in fact are not passing in features.

@mcwhittemore Will the new styling for points allow for sprites? I think from a user perspective, lets say you are editing a layer which is styled with a icon/sprite, you'd probably expect when you add a new point that it looks the same and therefore creates an icon/sprite rather than a circle.

one thing that immediately caught my attention is the somewhat ambiguous description of id's. It says in the description of the add method that it will return feature.id, but in the examples later on it's clear that the id is present on the geometry rather than the top level feature.

@averas thanks for point this out. I'll try to clear up ids later today and ping you for feedback when I think I have a good handle on it.

To minimise confusion it's perhaps also best to either base the examples on features, or rename the variables to "geojson" etc., where you in fact are not passing in features.

@averas yea, I was feeling feature fatigue when I wrote this. GeoJSON is a good move. I think we're going to need to solve this in the code too.

Will the new styling for points allow for sprites?

@joedjc Yep. I'm pretty sure this is currently possible as well. Draw doesn't ship with sprites though, so you'd need to make sure they are included at the map level.

This below style should put an icon on all Point features in the future version of gl-draw. In the current version you'd need to name the layer to overwrite the point layer which also means your LineString and Polygon handles would be this icon.

{
    "id": "coffee-shops",
    "type": "symbol",
    "filter": ["all",
      ["==", "$type", "Point"],
      ["==", "meta", "feature"]],
    "layout": {
        "icon-image": "icon_name"
    },
    "interactive": true
}

@mcwhittemore very cool stuff. A few things I noticed:

• Is there a way to get all active features now? (formerly getSelected()) If so, this makes the .trash method more of convenience function
• Would prefer all events to be lower case, no underscore, to match mapbox-gl (e.g. boxzoomend)
• Would prefer modes to be either enums (Draw.modes.DEFAULT) or separate methods (Draw.startDirectSelect(id), Draw.startDrawing(Draw.types.LINE)) although then I'm not positive what draw.changemode should emit.
• There's no mode which doesn't select a feature when clicked (formerly permanent as an option but I agree with the move to remove these properties from features). This could be accomplished by moving a feature out of the draw source but I can't decide if that should be handled by the library or developer
• draw.active is an ambiguous event name. Would prefer two events (draw.activate and draw.deactivate or something like that) or one event with a broader name draw.changeInActiveness (maybe a little verbose...)

@drewbo

Is there a way to get all active features now? (formerly getSelected()) If so, this makes the .trash method more of convenience function

I don't currently have a getSelected(). I think the getters should only return features drawn or provided by the user. Since you also select vertices and maybe edges (in the future) I think getSelected or even getActive is confusing.

Assuming we had a getter that returned the user provided feature the mode is currently focused on you still wouldn't be able to preform trash because trash is telling the current mode to run its deletion code. For direct_select this would be the selected vertices, not the active feature. For the draw modes, its more of a cancel than a delete.

Would prefer all events to be lower case, no underscore

👍

There's no mode which doesn't select a feature when clicked

Yea. I don't think Draw should be in this game. If the apis for this aren't good for mapbox-gl-js we should fix it there or create a small module that makes this easy.

draw.active is an ambiguous event name

Agree. There isn't an event for feature changes. Maybe we can solve these two problems with one event?

@mcwhittemore this could really use a linting pass

Just a few bugs

• last mid-point is missing on polygons in direct_select mode
• starting a mode with the control buttons doesn't always work
• trash can is not showing
• trash in direct select mode throws an error
• trash doesn't do anything for the draw modes...
• single click in direct_select mode should bring you back to select mode when no coords are selected
• double click zooms...
• shift select is default mode doesn't work
• selecting solid lines is hard
• select dotted lines is really hard
• key bindings don't work
• dragging often drag the map like its an image...

• get events working
• have control options control bindings
• remove tests that cover dropped apis
• work on clustering 'invisible features'