@tomhughes commented on this pull request.

This looks pretty good - just a couple of minor suggestions that I've made 
inline.
> @@ -15,14 +15,7 @@ I18n.default_locale = <%= I18n.default_locale.to_json %>;
 I18n.fallbacks = true;
 
 window.onload = function () {
-  var query = (window.location.search || "?").slice(1),
-      args = {};
-
-  var pairs = query.split("&");
-  for (var i = 0; i < pairs.length; i++) {
-    var parts = pairs[i].split("=");
-    args[parts[0]] = decodeURIComponent(parts[1] || "");
-  }
+  var args = Object.fromEntries(new 
URLSearchParams(window.location.search).entries());

Do you actually need `.entries()` here? I believe the `URLSearchParams` object 
itself should be iterable?

> -    new L.OSM.CycleMap(thunderforestOptions).addTo(map);
-  } else if (args.layer === "transportmap") {
-    new L.OSM.TransportMap(thunderforestOptions).addTo(map);
-  } else if (args.layer === "hot") {
-    new L.OSM.HOT().addTo(map);
-  } else {
-    new L.OSM.Mapnik(mapnikOptions).addTo(map);
-  }
+  var baseLayers = {
+    cyclosm: ["CyclOSM"],
+    cyclemap: ["CycleMap", thunderforestOptions],
+    transportmap: ["TransportMap", thunderforestOptions],
+    hot: ["HOT"],
+    mapnik: ["Mapnik", mapnikOptions]
+  };
+  baseLayers["cycle map"] = baseLayers.cyclemap;

I think I'd just include this in the object definition after the `cyclemap` 
line. I realise it means duplicating the right hand side but I'm not sure 
that's any worse than this and I think it makes it a bit easier to see what's 
going on - also `baseLayers` could then be `const` I think?

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

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

Reply via email to