[openstreetmap/openstreetmap-website] Migrate the dashboard entry page from leaflet to maplibre (PR #6504)

[Frank Elsinga]

@CommanderStorm commented on this pull request.



> +.maplibregl-ctrl button.maplibregl-ctrl-zoom-in .maplibregl-ctrl-icon {
+  background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='20' height='20' fill='%23ffffff' viewBox='0 0 20 20'%3E%3Cpath d='M16 8H12V4L11 3H10L9 4V8H5L4 9v1l1 1H9v4l1 1h1l1-1V11h4l1-1V9z'/%3E%3C/svg%3E") !important;
+}
+.maplibregl-ctrl button.maplibregl-ctrl-zoom-out .maplibregl-ctrl-icon {
+  background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='20' height='20' fill='%23ffffff' viewBox='0 0 20 20'%3E%3Cpath d='M4 9v1l1 1H16l1-1V9L16 8H5Z'/%3E%3C/svg%3E") !important;
+}
+.maplibregl-ctrl button.maplibregl-ctrl-geolocate .maplibregl-ctrl-icon {
+  background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='20' height='20' fill='%23ffffff' viewBox='0 0 20 20'%3E%3Cpath d='M10 10v6h2L16 6V4H14L4 8v2Z'/%3E%3C/svg%3E") !important;
+}
+.maplibregl-ctrl button.maplibregl-ctrl-geolocate.maplibregl-ctrl-geolocate-active .maplibregl-ctrl-icon {
+  background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='20' height='20' fill='%2376c551' viewBox='0 0 20 20'%3E%3Cpath d='M10 10v6h2L16 6V4H14L4 8v2Z'/%3E%3C/svg%3E") !important;
+}

I am not sure if I follow your rationale fully :sweat_smile:.

What kind of https://github.com/openstreetmap/openstreetmap-website/labels/dx are we optimising for?
Neither [`sprite.svg`](https://github.com/openstreetmap/openstreetmap-website/blob/59d834114145771d228dd0a728b026ed2eb13bd3/app/assets/images/sprite.svg), nor item in the new [`map-controls`](https://github.com/hlfan/openstreetmap-website/tree/master/app/assets/images/map-controls) directory were changed in the last 2 years, so I don't expect this to materially change.
The icons seem to be working.

Your approach means:
- having every control you add to the map has to explicitely change its appearance via DOM interaction
- having to have the svg in scope via adding the following (kind of magic unless you know that these are map icons)
  ```html
  <svg class="d-none"><defs><%= render :partial => "layouts/control_icons", :locals => { :icons => %w[zoomin zoomout geolocate] } %></defs></svg>
  ```
- having to know that the indirect linking of the svg does what it does.
- having to subclass every Control to change the DOM element. 
 `geolocate.onAdd(map)` is not what we want, we want control over placement, i.e. top right in this instance, too.

The existing solution has the cost of this is a lot of abstraction "magic" and code duplication.
In the definting it in CSS way, this is only defined once in the CSS (where the rest of the styling code lives), not on every map control adding to the map.

**Are you sure that I should do this refactoring?**

After a bit of searching, I found `image-url()` which works as expected and solves this too.
Downside: no inlined SVGs.

Given that the map takes some time to intalise, the few ms to the CDN are likely fine.
With caching, likely not an issue.

Alternatively, I could do a custom scss function, but that also is not actually simpler, given how often it would be used.

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

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