@tomhughes requested changes on this pull request.


> @@ -107,6 +107,8 @@ attachments_dir: ":rails_root/public/attachments"
 #logstash_path: ""
 # List of memcache servers to use for caching
 #memcache_servers: []
+# URL of Maptiler API for vector maps
+maptiler_url: "https://api.maptiler.com/";

Does this actually need to be a setting? The other layers are all hard coded 
and in fact this is hard coded in the javascript, just not when setting the 
security policy.

What we should have in here is a commented out setting for `maptiler_key` 
though, as we already do for the other map layers that need a key.

> +        openmaptiles_url: "https://openmaptiles.org/";,
+        maptiler_url: "https://www.maptiler.com/";
+      });
+
+      var terms = $("<a>", {
+        href: "https://wiki.osmfoundation.org/wiki/Terms_of_Use";,
+        text: I18n.t("javascripts.map.website_and_api_terms")
+      }).prop("outerHTML");
+
+      this.baseLayers.push(new L.OpenMapTiles({
+        attribution: copyright + ". " + openmaptiles_link + ". " + terms,
+        code: "V",
+        keyid: "openmaptiles_osm",
+        name: I18n.t("javascripts.map.base.openmaptiles_osm")
+      }));
+    }

This should, if at all possible, be done in `config/layers.yml` rather than 
hand coding it - if we need to add new keys to that file and code here to 
handle them that's fine but all layers are supposed to be defined there now.

> @@ -3316,7 +3318,7 @@ en:
       title: "Creating New Redaction"
     show:
       description: "Description:"
-      heading: "Showing Redaction \"%{title}\""
+      heading: 'Showing Redaction "%{title}"'

Why has this changed?

> @@ -9,6 +13,35 @@ L.extend(L.LatLngBounds.prototype, {
   }
 });
 
+if (OSM.MAPTILER_KEY) {
+  
maplibregl.setRTLTextPlugin("https://api.mapbox.com/mapbox-gl-js/plugins/mapbox-gl-rtl-text/v0.2.3/mapbox-gl-rtl-text.js";,
 true);

To answer the question from that comment, you modify 
`app/assets/config/manifest.js` and add:

```js
//= link @mapbox/mapbox-gl-rtl-text/mapbox-gl-rtl-text.js
```

then add to `app/assets/javascripts/osm.js.erb` something like:

```js
RTL_TEXT_PLUGIN: <%= 
javascript_path("@mapbox/mapbox-gl-rtl-text/mapbox-gl-rtl-text.js").to_json %>,
```

and then you can use `OSM.RTL_TEXT_PLUGIN` here in place of the mapbox URL 
string.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4042#pullrequestreview-2635631099
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/4042/review/2635631...@github.com>
_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to