If no enterprises have been geocoded yet make sure Open Street Map displays correctly #5680
Conversation
…ill displays correctly. Before it would display a gray/blank div instead of map because the map latitude, longitude couldn't be calculated without geocoded enterprises. This adds a setting so the default coordinates can be set even if no geocoded enterprises present.
There was a problem hiding this comment.
wonderful, thanks a lot for one more @cillian
I think we need to move some of this code to a service with tests
app/assets/javascripts/darkswarm/directives/open_street_map.js.coffee
Outdated
Show resolved
Hide resolved
… given a set of coordinates. Also removing the hardcoded default latitude/longitude from open_street_map directive because it's probably not very likely that it will be needed.
| averagePositiveAngle - averageNegativeAngle | ||
| else | ||
| averageNegativeAngle - averagePositiveAngle | ||
|
|
||
| buildMarker = (enterprise, latlng, title) -> | ||
| icon = L.icon |
There was a problem hiding this comment.
Where's this L variable coming from? I can't see where it's set...?
There was a problem hiding this comment.
That L is the Leaflet JS library, it gets included here https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/assets/javascripts/darkswarm/all.js.coffee#L11
| zoomLevel = 6 | ||
| map = L.map('open-street-map') | ||
| L.tileLayer.provider(openStreetMapProviderName, openStreetMapProviderOptions).addTo(map) | ||
| map.setView([averageLatitude, averageLongitude], zoomLevel) | ||
| map.setView([initialLatitude(), initialLongitude()], zoomLevel) |
There was a problem hiding this comment.
These two methods are only called in this one place, right? I think I would extract #initialLatitude and #initialLongitude to the service as well, and just have something like this:
map.setView([MapCentreCalculator.initialLatitude(), MapCentreCalculator.initialLongitude()], zoomLevel)There was a problem hiding this comment.
@Matt-Yorkley yes that works well. I have moved those method into the MapCentreCalculator service now.
|
Moving to In Dev until @cillian 's feedback. |
…StreetMap service to the MapCenterCalculator service.
|
|
||
| _calculate: (angleName, coordinates) => | ||
| positiveAngles = [] | ||
| negativeAngles = [] |
There was a problem hiding this comment.
these two arrays can be removed right? We append values to them but I don't see them used. I'm moving this to test ready anyway, but it'd be good the clarify this.
There was a problem hiding this comment.
@sauloperez ah yes, I should have spotted that, it's fixed now in
e6ab2ae
|
Pretty neat @cillian ! |
…e calculator service. I forgot to remove these when I was refactoring this earlier.
|
Verifying the issue before staging this. Google maps centers around melbourne by default with no enterprise. After deployment, google maps still works and OSM now shows the default location in AUS (it's not so zoomed in as in google maps). This is ready to go ✔️ |
What? Why?
Before if Open Street Map was enabled and no geocoded enterprises were present the map wouldn't display correctly, it would display a gray/blank element. These changes make sure the map displays correctly even if no geocoded enterprises present. It also includes a setting to change the default map coordinates.
This is a start for the last item in #5542
What should we test?
Extra testing note from Matt: turn Open Street Map off again in the staging server after you've finished testing this please! 😅
Release notes
Prevent blank Open Street Map when no geocoded enterprises present.
Changelog Category: Fixed