@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/2486890...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
