@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/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
