-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GeoJSON defaults #2256
GeoJSON defaults #2256
Conversation
Fixes #2167 |
@@ -449,19 +475,156 @@ define([ | |||
* Creates a new instance and asynchronously loads the provided url. | |||
* | |||
* @param {Object} url The url to be processed. | |||
* @param {Object} [options] An object with the following properties: | |||
* @param {Number} [options.markerSize] The default size of the map pin created for each point, in pixels. |
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.
It would be helpful to list the defaults here I think.
Tests pass, Sandcastle example works nicely, and the code looks reasonable (admittedly I didn't look at the code in excruciating detail). So all good by me other than the one minor comment. I just reviewed this pull request as a whole instead of your other one separately. I'll give others a chance to review if they want before merging. I'll probably use this feature in National Map right away, so thanks for doing it! |
* Fixed a bug in `PolylineGeometry` that would cause the geometry to be split across the IDL for 3D only scenes. [#1197](https://github.com/AnalyticalGraphicsInc/cesium/issues/1197) | ||
* Deprecated the `sourceUri` parameter to `GeoJsonDatasource.load`, use `options.sourceUri` instead. |
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.
This should go in a Deprecated section at the top, I believe.
Okay, updates are in. |
NOTE: This builds on top of #2255, so review/merge that first
GeoJsonDataSource.load
,GeoJsonDataSource.loadUrl
, andGeoJsonDataSource.fromUrl
that takes the same set of parameters as above in order to allow basic per-instance styling.sourceUri
parameter toGeoJsonDatasource.load
, useoptions.sourceUri
instead.waitsForPromise
. Also added tests for the new functionality.