0.6.0-rc1

[mcwhittemore]

Closes #257
Closes #249 - selection is not longer a global concept in Draw so forcing feature to be selected is no longer viable.
Closes #247
Closes #245 - draw.changed is emitted with ever render
Closes #243
Closes #218 - draw.changed is emitted with ever render. Adding vertices causes a render.
Closes #128 - fixed in [email protected]
Closes #109 - permanent features have been removed

---

This PR is moving us away from a tool where features describe how events affect them to a system where modes describe how they effect features. Currently you can draw a feature, select a feature and change a features coordinates at almost any time. This is communicated to the end-user via two styles (selected and unselected). Moving forward each of these actions will be available via different modes and only one mode will be running at a time.

Overview of modes

Below is a list of the current modes and what they can do. I also need to document what styling options this gives the end-developer both in CSS and the map style.

The draw modes are always available via the draw controls or via api.startDrawing

I think all modes should be enterable via the api.

Features Select

• Select a feature
• Multi select features
• Drag selected features
• Delete selected features
• Move to Direct Select by doubling clicking on a feature

Direct Select

• Select a vertex
• Multi select vertices
• Move selected vertices
• Delete selected vertices
• Move to Features Select by double clicking any where

Draw LineString

Lets you draw a LineString. Exits to Features Select

Draw Polygon

Lets you draw a Polygon. Exits to Features Select

Draw Point

Lets you place a point. Exits to Features Select

Notes

This is still a long way out

The goals of this PR are:

• rework the event system to work via modes. Each mode represents a type of interaction the user has with the map data. Dragging, browsing, drawing a line, etc.
• move away from bucket based styling and into attribute styling. Hopefully this will let users have their own style for the different states and let users create as many states as they want.
• make private apis private
• standardize api verbs (add/remove, vs add/destroy) as much as we can.
• speed things up

[lucaswoj]

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

[kelvinabrokwa]

s/Taraget/Target/g

[kelvinabrokwa]

Any particular reason for moving away from ES6 syntax?

[mcwhittemore]

@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.

[mcwhittemore]

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

[averas]

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.

[joedjc]

@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.

[kelvinabrokwa]

s/return/returns (tiny typo)

[mcwhittemore]

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
}

[drewbo]

@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...)

[mcwhittemore]

@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?

[kelvinabrokwa]

@mcwhittemore are you going to hit this on this PR?

[mcwhittemore]

Yea. The overwrite should be really simple and having a hot and cold source should help with performance.

[kelvinabrokwa]

single quote

[kelvinabrokwa]

@mcwhittemore this could really use a linting pass

[mcwhittemore]

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...

[mcwhittemore]

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

[lucaswoj]

What does it mean for a benchmark to "pass" or "fail"? I encourage you to think about benchmark scores as something we work to improve over time rather than something that passes or fails.