[openstreetmap/openstreetmap-website] Write browse element icons as <img> (PR #5385)

[Tom Hughes]

@tomhughes requested changes on this pull request.

So to summarise on the question of what happens when there are multiple icons the old logic is that priority is defined by the order of the rules  in the CSS which is mostly alphabetical except that the generic building rule is with the way rules which come after the node rules although the node/way split is really nonsense.

The new logic handles everything alphabetically, except that now the last rule wins rather than the first, and building is now ordered with other things so loses to shop but beats amenity.

My principle suggestion is that a key/value rule should always win over a key only rule as it's more specific.

If we want total backwards compatibility then the first matching rule should win - an way to do that would be to walk `target_tags` in reverse order when looking for a match.

I'm not sure that really matters though - if there are multiple key/value matches then there's no real right or wrong answer as to which one to choose.

> @@ -1,15 +1,43 @@
 module BrowseHelper
+  def element_icon(type, object)
+    icon_data = { :filename => "#{type}.svg" }
+
+    unless object.redacted?
+      target_tags = object.tags.find_all { |k, _v| BROWSE_ICONS.key? k.to_sym }.sort
+      title = target_tags.map { |k, v| "#{k}=#{v}" }.to_sentence unless target_tags.empty?
+
+      target_tags.each do |k, v|
+        k = k.to_sym
+        v = v.to_sym
+        if v != :* && BROWSE_ICONS[k].key?(v)
+          icon_data = BROWSE_ICONS[k][v]
+        elsif BROWSE_ICONS[k].key?(:*)

I'd suggest not allowing a wildcard key only match to override a more specific key=value match here:

```suggestion
        elsif icon_data.nil? && BROWSE_ICONS[k].key?(:*)
```

>                <%= element_single_current_link "node", wn.node %>
               <% related_ways = wn.node.ways.reject { |w| w.id == wn.way_id } %>
+              <% icon_connector = " " %>

Why use a variable for this when it doesn't ever seem to change?

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

Message ID: <openstreetmap/openstreetmap-website/pull/5385/review/2486890217 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20241208/dac8dbae/attachment.htm>