fix for #1239 add public gps tiles layer for debugging purposes #1406
Conversation
|
this fixes #1239 |
| subdomains: 'abc' | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
As a vendored library this needs to be patched upstream (at https://github.com/jfirebaugh/leaflet-osm) first.
That rule basically applies to anything under the vendor directory - you can look in Vendorfile to see where they come from.
There was a problem hiding this comment.
Could it theoretically also be rolled up with the usage in https://github.com/openstreetmap/openstreetmap-website/pull/1406/files#diff-14dc1f5c2800c4a4d2accb07b96c97a0R59 though?
There was a problem hiding this comment.
So I basically create a new pull request with https://github.com/jfirebaugh/leaflet-osm first and remove that part here then?
There was a problem hiding this comment.
@derickr well that builds on this - they're two components of the same things although I agree the split is a bit arbitrary.
@IOOI-SqAR open the upstream PR but there's not need to remove it here as we still need it, we just don't want out version diverging from upstream.
There was a problem hiding this comment.
I have just created openstreetmap/leaflet-osm#15
| @@ -2211,6 +2211,7 @@ en: | |||
| header: Map Layers | |||
| notes: Map Notes | |||
| data: Map Data | |||
| gps: GPS | |||
There was a problem hiding this comment.
I think something like "Public GPS Traces" would be a better label here?
| @@ -176,6 +176,7 @@ L.OSM.layers = function(options) { | |||
|
|
|||
| addOverlay(map.noteLayer, 'notes', OSM.MAX_NOTE_REQUEST_AREA); | |||
| addOverlay(map.dataLayer, 'data', OSM.MAX_REQUEST_AREA); | |||
| addOverlay(map.gpsLayer, 'gps', OSM.MAX_REQUEST_AREA); | |||
There was a problem hiding this comment.
We either need to pass a larger value for the second argument here (because I think the GPS tiles don't have a limit) or we need to add javascripts.site.map_gps_zoom_in_tooltip to the resources.
There was a problem hiding this comment.
Using Number.POSITIVE_INFINITY for the second argument is probably best I suspect.
| this.gpsLayer = new L.OSM.GPS({ | ||
| attribution: copyright, | ||
| code: "G", | ||
| keyid: "gps", |
There was a problem hiding this comment.
Only base layers need keyid as there is no support for showing keys for overlays.
The code member is fine, but it needs code adding at https://github.com/openstreetmap/openstreetmap-website/blob/master/app/assets/javascripts/index.js#L160 to actually add the layer if that code is present when the page is loaded.
There was a problem hiding this comment.
just one question: is the attribution key needed here? If I add it (as it currently is) I see the copyright twice one I check that box
There was a problem hiding this comment.
Good question and I believe the answer is no, because attribution is only shown for base layers.
|
openstreetmap/leaflet-osm#15 is now merged |
|
Thanks for adding. This looks great. 💯 |
|
I don't get the tiles delivered: Naturally with this problem, it works when I call the site via plain-http. |
|
Same issue here with that link.
|
|
Yes that was the wrong URL to use for https support - should be fixed in 757ef87. |
|
confirmed, thanks. |
this pull request adds a checkbox in the debug area of the layers selection to show public GPS traces overlayed to the map. This comes in handy in sparsely mapped areas where GPS tracks might exist but no mapping has been done yet.
