@hlfan requested changes on this pull request.
Why even still keep the `map-control` SVGs around?
> @@ -11,16 +11,13 @@ L.OSM.note = function (options) {
.attr("title", OSM.i18n.t("javascripts.site.createnote_tooltip"))
.appendTo($container);
- $(L.SVG.create("svg"))
- .append($(L.SVG.create("use")).attr("href", "#icon-note"))
- .attr("class", "h-100 w-100")
- .appendTo(link);
+ $("<i>").addClass("bi bi-chat-square-fill").appendTo(link);
To stay consistent with the context menu:
```suggestion
$("<i>").addClass("bi bi-chat-square-text-fill").appendTo(link);
```
Also, could that be chained onto `link` with `.append()`?
> @@ -11,16 +11,13 @@ L.OSM.query = function (options) {
.attr("title", OSM.i18n.t("javascripts.site.queryfeature_tooltip"))
.appendTo($container);
- $(L.SVG.create("svg"))
- .append($(L.SVG.create("use")).attr("href", "#icon-query"))
- .attr("class", "h-100 w-100")
- .appendTo(link);
+ $("<i>").addClass("bi bi-question-square-fill").appendTo(link);
I goofed with `question-fill`, I meant this:
```suggestion
$("<i>").addClass("bi bi-question-lg").appendTo(link);
```
Similarly in the context menu.
> @@ -2,7 +2,7 @@
<h1><%= t ".title" %></h1>
<% end %>
-<%= render :partial => "layouts/control_icons", :locals => { :icons =>
%w[zoomin zoomout geolocate] } %>
+<%= render :partial => "layouts/control_icons", :locals => { :icons =>
%w[plus-lg dash-lg cursor-fill] } %>
```suggestion
```
I'd go remove the `_control_icons` partial and the SVG files too while at it.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6695#pullrequestreview-3647256685
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6695/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
