<p></p>
<p><b>@tomhughes</b> requested changes on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#discussion_r1919005030">app/assets/javascripts/leaflet.map.js</a>:</p>
<pre style='color:#555'>> @@ -32,6 +32,11 @@ L.OSM.Map = L.Map.extend({
           layerOptions.apikey = OSM[value];
         } else if (property === "leafletOsmId") {
           layerConstructor = L.OSM[value];
+        } else if (property === "leafletOsmDarkId" && OSM.isDarkMap() && L.OSM[value]) {
+          layerConstructor = L.OSM[value];
+          layerOptions.className += " dark";
+        } else if (property === "filterClass" && OSM.isDarkMap()) {
</pre>
<p dir="auto">I think something like <code class="notranslate">darkFilterClass</code> would be better as the property name, so that it's clear what the filtering is for.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#discussion_r1919005826">app/assets/javascripts/leaflet.map.js</a>:</p>
<pre style='color:#555'>> @@ -52,10 +57,12 @@ L.OSM.Map = L.Map.extend({
       code: "G"
     });
 
-    this.on("layeradd", function (event) {
-      if (this.baseLayers.indexOf(event.layer) >= 0) {
-        this.setMaxZoom(event.layer.options.maxZoom);
-      }
+    this.on("layeradd", function ({ layer }) {
+      if (this.baseLayers.indexOf(layer) < 0) return;
+      this.setMaxZoom(layer.options.maxZoom);
+      const key = document.querySelector(".key-ui");
</pre>
<p dir="auto">We normally use jQuery rather than native methods like this.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#discussion_r1919006661">app/assets/javascripts/leaflet.map.js</a>:</p>
<pre style='color:#555'>> @@ -52,10 +57,12 @@ L.OSM.Map = L.Map.extend({
       code: "G"
     });
 
-    this.on("layeradd", function (event) {
-      if (this.baseLayers.indexOf(event.layer) >= 0) {
-        this.setMaxZoom(event.layer.options.maxZoom);
-      }
+    this.on("layeradd", function ({ layer }) {
+      if (this.baseLayers.indexOf(layer) < 0) return;
+      this.setMaxZoom(layer.options.maxZoom);
+      const key = document.querySelector(".key-ui");
+      if (!key) return;
+      key.className = "key-ui " + layer.options.className;
</pre>
<p dir="auto">...and if jQuery is used then this can use <code class="notranslate">addClass</code> and not have to worry about preserving pre-existing classes.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#discussion_r1919011214">config/layers.yml</a>:</p>
<pre style='color:#555'>> @@ -31,6 +31,7 @@
   apiKeyId: "THUNDERFOREST_KEY"
   canEmbed: true
   canDownloadImage: true
+  filterClass: "no-filter"
</pre>
<p dir="auto">I assumed that the filter class was going to be for turning on filtering, not for turning it off? My understanding from <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2752684525" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/5426" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/5426/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/5426">#5426</a> is that enabling filtering for a layer will only happen on the basis of agreement with the designer of that layer.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#discussion_r1919012634">app/assets/stylesheets/common.scss</a>:</p>
<pre style='color:#555'>>    }
 
-  .leaflet-tile-container .leaflet-tile {
-    filter: none;
+  .no-filter, .dark {
+    --dark-mode-map-filter: none;
</pre>
<p dir="auto">This all seems very confused - you've removed the default value for the filter variable, and set a default of <code class="notranslate">none</code> when using it, so why do we need to explicitly set it to none here?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#discussion_r1919013243">app/assets/stylesheets/common.scss</a>:</p>
<pre style='color:#555'>> @@ -516,13 +515,13 @@ body.small-nav {
 }
 
 @mixin dark-map-color-scheme {
-  .leaflet-tile-container,
+  .leaflet-layer,
</pre>
<p dir="auto">Why the change in class here?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5505#pullrequestreview-2556912892">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLNAGXPQ5SAMYMWPZZL2K73MNAVCNFSM6AAAAABVIRFJTCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNJWHEYTEOBZGI">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLIIFB5BZETKSSDLT332K73MNA5CNFSM6AAAAABVIRFJTCWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTUYM5SPY.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/5505/review/2556912892</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/5505#pullrequestreview-2556912892",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/5505#pullrequestreview-2556912892",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>