change OpenMapSurfer_Roads basemap to terrain #537

boney-bun · 2019-03-26T07:33:55Z

NyakudyaA · 2019-03-26T07:41:23Z

@boney-bun does this remove the base maps even at Layer detail page, Because this also affects the thumbnail generation where the basemap is empty

codecov · 2019-03-26T07:51:18Z

Codecov Report

Merging #537 into 2.8.x will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            2.8.x     #537   +/-   ##
=======================================
  Coverage   57.75%   57.75%           
=======================================
  Files          49       49           
  Lines        2720     2720           
  Branches      301      301           
=======================================
  Hits         1571     1571           
  Misses       1068     1068           
  Partials       81       81

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08db72a...0a89ffe. Read the comment docs.

lucernae

Why we don't use OSM as default basemap?
I'm not sure why you add Terrain in this page?
Could you use the same logic from Layer details to include set of basemaps taken from the settings file?

NyakudyaA · 2019-03-26T10:00:45Z

Why we don't use OSM as default basemap?
I'm not sure why you add Terrain in this page?
Could you use the same logic from Layer details to include set of basemaps taken from the settings file?

Yes @lucernae we just need to have a basemap that we know always works. OSM basemap would be nice to have it as the default and the terrain can still be added as a second option for people who do want to change

lucernae · 2019-03-26T10:25:29Z

Yes @lucernae we just need to have a basemap that we know always works. OSM basemap would be nice to have it as the default and the terrain can still be added as a second option for people who do want to change

Yeah, argh sorry for not being clear :D. If I read my comment above again, it seems I'm complaining about the Terrain map. What I mean is, we should add the Terrain in the settings, not in the page. Then take the default basemap from whatever the first basemap in the list from the settings.

In last year's sprint, we already fixed this problem by offloading the basemap to settings, so we made this:

https://github.com/kartoza/geonode/blob/2.8.x-qgis_server/geonode/client/templates/leaflet/layers/layer_leaflet_map.html#L142 (in kartoza/geonode repo)

The layer detail page takes extra basemaps from the settings module.

I suggest that:

We take the same approach for analysis related page (because it isn't handled by geonode) in this geosafe repo.
Customize the basemaps to everywhere from settings module.
Add new settings field to choose default basemaps from a given basemaps defined in settings
Refactor layer and map page of GeoNode to choose default basemaps from settings.

The settings that I was talking about:

https://github.com/kartoza/geonode/blob/2.8.x-qgis_server/geonode/settings.py#L1015

It would be nice if we can handle all these

boney-bun · 2019-03-26T10:40:16Z

yes, the default is osm. terrain is just an additional basemap to replace the removed one.
i have another PR in custom repo.
i thought we will just use the custom one.
i guess i will close the PR in the custom repo and move it to geonode repo.
i also will revise this PR.

boney-bun · 2019-03-26T17:48:05Z

the PR in action:

the basemaps are taken from settings.
the default basemap is always osm.

@lucernae @NyakudyaA could you review again?

NyakudyaA

Nice @boney-bun , Does it also cover the layer details page where the default will be OSM

lucernae

LGTM @boney-bun , thank you :).

Could you check my suggestion too in your local environment first.

The aim is to have the default basemap automatically taken from the first TILES config if defined, if not, use default osm.

lucernae · 2019-03-27T07:38:06Z

templates/geosafe/analysis/create_base.html

+                        attribution: '{{ tile.2|safe }}'
+                });
+                {% endfor %}
+            {% endif %}


Suggested change

{% endif %}

default_basemap = base_maps["{{ LEAFLET_CONFIG.TILES.0 }}"];

{% endif %}

changes it into:
default_basemap = base_maps["{{ LEAFLET_CONFIG.TILES.0.0 }}"];

lucernae · 2019-03-27T07:38:44Z

templates/geosafe/analysis/create_base.html

-                zIndex: 0,
-                noWrap: true,
-                attribution: 'Imagery from <a href="http://giscience.uni-hd.de/">GIScience Research Group @ University of Heidelberg</a> &mdash; Map data &copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>'
-            });
            var osm = L.tileLayer('http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {


Suggested change

var osm = L.tileLayer('http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {

var default_basemap = L.tileLayer('http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {

lucernae · 2019-03-27T07:39:10Z

templates/geosafe/analysis/create_base.html

            map = L.map('map', {
-                layers: [OpenMapSurfer_Roads],
+                layers: [osm],


Suggested change

layers: [osm],

layers: [default_basemap],

lucernae · 2019-03-27T07:39:37Z

templates/geosafe/analysis/detail_base.html

-                attribution: 'Imagery from <a href="http://giscience.uni-hd.de/">GIScience Research Group @ University of Heidelberg</a> &mdash; Map data &copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>'
-            });
-            map.addLayer(OpenMapSurfer_Roads);
+            map.addLayer(osm);


Suggested change

map.addLayer(osm);

map.addLayer(default_basemap);

boney-bun · 2019-03-27T09:45:16Z

thanks for the feedback @lucernae @NyakudyaA
i've revised and tested it locally.
i'm going to merge this PR.

change openstreetmap_roads basemap to terrain

2827f5c

fix #kartoza#531

boney-bun requested a review from lucernae March 26, 2019 07:33

ghost assigned boney-bun Mar 26, 2019

ghost added in progress labels Mar 26, 2019

lucernae requested changes Mar 26, 2019

View reviewed changes

load basemaps from settings

c2b4142

boney-bun requested a review from NyakudyaA March 26, 2019 17:47

NyakudyaA reviewed Mar 27, 2019

View reviewed changes

lucernae approved these changes Mar 27, 2019

View reviewed changes

revise after feedback

0a89ffe

boney-bun merged commit 9291061 into kartoza:2.8.x Mar 27, 2019

ghost removed the in progress label Mar 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change OpenMapSurfer_Roads basemap to terrain #537

change OpenMapSurfer_Roads basemap to terrain #537

boney-bun commented Mar 26, 2019

NyakudyaA commented Mar 26, 2019

codecov bot commented Mar 26, 2019 •

edited

Loading

lucernae left a comment

NyakudyaA commented Mar 26, 2019

lucernae commented Mar 26, 2019

boney-bun commented Mar 26, 2019

boney-bun commented Mar 26, 2019

NyakudyaA left a comment

lucernae left a comment

lucernae Mar 27, 2019

boney-bun Mar 27, 2019

lucernae Mar 27, 2019

lucernae Mar 27, 2019

lucernae Mar 27, 2019

boney-bun commented Mar 27, 2019

	{% endif %}
	default_basemap = base_maps["{{ LEAFLET_CONFIG.TILES.0 }}"];
	{% endif %}

	var osm = L.tileLayer('http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
	var default_basemap = L.tileLayer('http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {

change OpenMapSurfer_Roads basemap to terrain #537

change OpenMapSurfer_Roads basemap to terrain #537

Conversation

boney-bun commented Mar 26, 2019

NyakudyaA commented Mar 26, 2019

codecov bot commented Mar 26, 2019 • edited Loading

Codecov Report

lucernae left a comment

Choose a reason for hiding this comment

NyakudyaA commented Mar 26, 2019

lucernae commented Mar 26, 2019

boney-bun commented Mar 26, 2019

boney-bun commented Mar 26, 2019

NyakudyaA left a comment

Choose a reason for hiding this comment

lucernae left a comment

Choose a reason for hiding this comment

lucernae Mar 27, 2019

Choose a reason for hiding this comment

boney-bun Mar 27, 2019

Choose a reason for hiding this comment

lucernae Mar 27, 2019

Choose a reason for hiding this comment

lucernae Mar 27, 2019

Choose a reason for hiding this comment

lucernae Mar 27, 2019

Choose a reason for hiding this comment

boney-bun commented Mar 27, 2019

codecov bot commented Mar 26, 2019 •

edited

Loading