[openstreetmap/openstreetmap-website] Consolidate leaflet marker creation in OSM.getMarker (PR #5860)

[Tom Hughes]

@tomhughes commented on this pull request.



> @@ -391,13 +391,17 @@ OSM.isDarkMap = function () {
   return window.matchMedia("(prefers-color-scheme: dark)").matches;
 };
 
-OSM.getUserIcon = function (url) {
-  return L.icon({
-    iconUrl: url || OSM.MARKER_RED,
-    iconSize: [25, 41],
-    iconAnchor: [12, 41],
-    popupAnchor: [1, -34],
-    shadowUrl: OSM.MARKER_SHADOW,
-    shadowSize: [41, 41]
-  });
+OSM.getMarker = function ({ icon = "MARKER_RED", shadow = true }) {
+  const height = icon.includes("NOTE") ? 40 : 41;

Should `height` be a parameter rather than trying to guess it from the name? It's a bit odd that we do it for height but make the user pass a flag for shadow?

> -    iconAnchor: [12, 41],
-    popupAnchor: [1, -34],
-    shadowUrl: OSM.MARKER_SHADOW,
-    shadowSize: [41, 41]
-  });
+OSM.getMarker = function ({ icon = "MARKER_RED", shadow = true }) {
+  const height = icon.includes("NOTE") ? 40 : 41;
+  const options = {
+    iconUrl: OSM[icon.toUpperCase()] || OSM.MARKER_RED,
+    iconSize: [25, height],
+    iconAnchor: [12, height],
+    popupAnchor: [1, -34]
+  };
+  if (shadow) {
+    options.shadowUrl = OSM.MARKER_SHADOW;
+    options.shadowSize = [41, 41];

Should this use `height` as the second value? I don't think it actually makes any difference right now as we only switch height for notes which always pass disable shadows but if we're trying to make it generic then maybe it should?

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

Message ID: <openstreetmap/openstreetmap-website/pull/5860/review/2726177209 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20250328/a952c95b/attachment-0001.htm>