feat: Show neighbouring properties as points on map#429
Conversation
✅ Deploy Preview for oslmap ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
cdccollins
force-pushed
the
feat-show-neighbour-markers
branch
2 times, most recently
from
feb6642 to
dc63bea
Compare
There was a problem hiding this comment.
Thanks for this new feature !! 🎉
Only one small question while reviewing: when I pass geojsonData and turn on showNeighbourMarkers it's not actually displaying the default black cirlces or any other image (it does correctly re-center the map over where the points should be)? I can still see the defualt marker image fine when just displaying the singular showMarker.
Is it the same for you / is there something different about my example data than what you plan to pass in?
{
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"properties": {},
"geometry": {
"coordinates": [
-0.13708768238623747,
51.51141463106259
],
"type": "Point"
}
},
{
"type": "Feature",
"properties": {},
"geometry": {
"coordinates": [
-0.1359069395277004,
51.512461774011285
],
"type": "Point"
}
},
{
"type": "Feature",
"properties": {},
"geometry": {
"coordinates": [
-0.1378256466718426,
51.51303126270315
],
"type": "Point"
}
}
]
}
| featureBuffer = 40; | ||
|
|
||
| @property({ type: Boolean }) | ||
| showNeighbourMarkers = false; |
There was a problem hiding this comment.
nit: since this prop reads from geojsonData, maybe naming as showGeojsonDataMarkers would offer more clarity on when to use it? The component itself doesn't necessarily need to know about the concept/use-case of "neighbours" I think / it works for any collection of dispersed points!
(showMarker → showCentreMarker might be nice clarification for existing prop too now that we're doubling down on this feature - but not sure it's worth a breaking change! Your call, easy enough for us to adjust our map instances on next upgrade if updated 🙂)
| } | ||
|
|
||
| if (this.showNeighbourMarkers) { | ||
| this.geojsonData.features.forEach((feature: any) => { |
There was a problem hiding this comment.
nit: rather than any this should be able to use type GeoJSONFeature from "ol/format/GeoJSON"
Thanks for looking! I think this comes down to my not knowing about geojson stuff... Do you normally pass latitude then longitude, or longitude then latitude? I've assumed that latitude is always first EDIT - ah I see. I've somehow got it backwards somewhere. Will look into |
cdccollins
force-pushed
the
feat-show-neighbour-markers
branch
2 times, most recently
from
1bb5a92 to
693bb73
Compare
jessicamcinchak
left a comment
jessicamcinchak
left a comment
There was a problem hiding this comment.
Aha no worries - you're right it's just an ordering thing - if I switch locally to this then I see markers ✅
showNewMarker(
feature.geometry.coordinates[0],
feature.geometry.coordinates[1],
);
Lat lon ⚔️ lon lat is a whole thing in gis mapping land, it's definitely not just you!! https://observablehq.com/@clhenrick/lat-lon-or-lon-lat
| type: "Point", | ||
| }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Oops let's keep default features [] here please !
|
|
||
| if (this.showGeojsonDataMarkers) { | ||
| this.geojsonData.features.forEach((feature: GeoJSONFeatureCollection) => { | ||
| console.log(feature.geometry.coordinates); |
There was a problem hiding this comment.
sorry two more minor things:
- please remove
console.log() - I think
geojsonDatais the typeGeoJSONFeatureCollectionbut the singular iteratorfeautureis justGeoJSONFeature!
Creates a property showNeighbourMark which when switched on creates a marker for each point passed in on the geojsonData property. This will be useful in BOPS as it can be difficult to know when drawing a polygon whether you have selected the neighbouring properties. BOPS will pass the coordinates of the neighbours via the geojsonData prop.
cdccollins
force-pushed
the
feat-show-neighbour-markers
branch
from
693bb73 to
e74e9b3
Compare
jessicamcinchak
left a comment
jessicamcinchak
left a comment
There was a problem hiding this comment.
Working now & good to go by me - thanks again 🙌
Please squash & merge this whenever you're ready & I'll queue up a new release this afternoon and ping you once published to NPM 🙂
Thanks so much for you help with this! |
2 participants
Creates a property showNeighbourMark which when switched on creates a marker for each point passed in on the geojsonData property.
This will be useful in BOPS as it can be difficult to know when drawing a polygon whether you have selected the neighbouring properties. BOPS will pass the coordinates of the neighbours via the geojsonData prop.