-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
@frederoni, thanks for your PR! By analyzing the history of the files in this pull request, we identified @incanus, @1ec5 and @jfirebaugh to be potential reviewers. |
@@ -159,6 +159,7 @@ class Map : private util::noncopyable { | |||
// Layers | |||
style::Layer* getLayer(const std::string& layerID); | |||
void addLayer(std::unique_ptr<style::Layer>, const optional<std::string>& beforeLayerID = {}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I remove the optional beforeLayerID
and combine beforeOrAfter in the insert method?
f066a0f
to
c5e9f9b
Compare
c5e9f9b
to
a16eac0
Compare
I don't think an additional variant of |
In fact, I question whether the SDKs themselves should have another variant. Typically array-like datastructures offer only an "insert before" operation. In cases where "insert after" is desired, you increment your index by one. |
I think you're right about |
Another vote here for the Cocoa API-like |
As a cartographic convention, it's much more common to say "add a layer beneath labels" than "add a layer above X", for some value of X. We can of course support both in an SDK for a platform where "above" is common in other contexts, I'm just not really convinced it makes sense for map layers. |
Not sure we should confuse cartographers with app developers. One of the most common use cases will likely be to find a road or a park and insert a layer on top of it. If we don't support this, you'll have to find an arbitrary layer above the layer you're working with and insert it beneath it. Doesn't sound very intuitive to me. |
For comparison, MapKit only allows you to insert an overlay at one of two preset levels: |
I only pointed out MapKit's overlay levels for comparison, as a window into the most common use cases. Since our SDK is much more freeform, allowing arbitrary styles with arbitrary layers, an equivalent set of options would necessarily be limited to a certain set of Mapbox styles, something we haven't done in the SDK to date. |
WIP
Insert layer above another layer