When adding annotations, add a gcs options. #654
Conversation
New annotations should default to the interface gcs, not the map gcs. Note that this breaks the current interface for annotationLayer.addAnnotation.
|
@danlamanna This will make it so you don't have to call |
Current coverage is 86.89% (diff: 100%)@@ master #654 diff @@
==========================================
Files 87 87
Lines 8586 8591 +5
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 7460 7465 +5
Misses 1126 1126
Partials 0 0
|
| @@ -348,7 +358,7 @@ var annotationLayer = function (args) { | |||
| state: geo_annotation.state.create, | |||
| layer: this | |||
| }); | |||
| this.addAnnotation(m_this.currentAnnotation); | |||
| this.addAnnotation(m_this.currentAnnotation, null); | |||
There was a problem hiding this comment.
it could be little confusing in the way we are using null or undefined. I would suggest that if it is null or undefined, we fall back to map.gcs(). Should we create keys perhaps? map_gcs, map_ingcs? Thoughts?
There was a problem hiding this comment.
This is done throughout the other modules, so that none is the special value that can be used internally to mean use the map.gcs() while not forcing a user to constantly pass gcs parameters when what they mostly want is the map.ingcs().
I would not make the undefined value go to map.gcs(), as that is certainly not what a user wants (they want the interface gcs, not the display gcs).
Using map_gcs instead of none would be okay, but this really should be a constant defined somewhere. In which case, I lament that we don't have one constants module. The user already has to import events and actions to get constants.
There was a problem hiding this comment.
I agree with all of what you said above. May we can add constants and use map_gcs as a replacement for none in a separate PR. Care to create a issue for it? I would think that having a entry for map_ingcs will be useful too.
|
thanks @manthey |
New annotations should default to the interface gcs, not the map gcs. Note that this breaks the current interface for annotationLayer.addAnnotation.