-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: drawGeojsonData
should accept "Feature" and "FeatureCollection" data since introducing drawMany
#491
Conversation
✅ Deploy Preview for oslmap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Looks good to me to switch from pulling in a feature to using a FeatureCollection like in geoJsonData prop.
Only had one question relating to adding a Feature or Features, mostly curious about the structuring of the code.
const geojsonSource = new VectorSource(); | ||
|
||
if (this.geojsonData.type === "FeatureCollection") { | ||
let features = new GeoJSON().readFeatures(this.geojsonData, { | ||
featureProjection: "EPSG:3857", | ||
}); | ||
geojsonSource.addFeatures(features); | ||
geojsonSource.setAttributions(this.geojsonDataCopyright); | ||
} else if (this.geojsonData.type === "Feature") { | ||
let feature = new GeoJSON().readFeature(this.geojsonData, { | ||
featureProjection: "EPSG:3857", | ||
}); | ||
geojsonSource.addFeature(feature); |
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.
Took me a second to realise the differences between these parts. Is the need for two different functions relate to one accessing the Features
array of a FeatureCollection
and the other just taking the Feature
as an object, or is there another reason for having two different functions / if clauses?
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.
Yes exactly OpenLayers provides different methods depending if GeoJSON is single Feature or a FeatureCollection, and we want to keep our public-facing prop flexible to accepting eithe, therefore we do this conditional check !
Required to make drawing data "go back-able" in Planx when
drawMany
is true.This was missed when introducing
drawMany
prop -drawGeojsonData
was only accepting/processing a single"Feature"
and would fail if the passed in prop was a GeoJSON"FeatureCollection"
- now account for either, same asgeojsonData