[openstreetmap/openstreetmap-website] Show tag changes in object history view (PR #6448)

[Pablo Brasero]

@pablobm commented on this pull request.

Still looking into how to better resolve the duplication in `browse/_tag_details`, but these are some quick notes.

> @@ -38,4 +38,4 @@
   <% end %>
 </div>
 
-<%= render :partial => "browse/tag_details", :object => common_details.tags %>
+<%= render :partial => "browse/tag_details", :object => common_details.tags, :locals => { :tag_details_object => common_details } %>

How about renaming `tag_details_object` to `current_feature`? I had some difficulty reasoning about what it represented, due to the naming.

> +<% if defined?(tag_with_changes) %>
+  <% change_info = tag_with_changes %>
+  <tr class="<%= tag_change_class(change_info[:type]) %>">
+    <th class='py-1 border-secondary-subtle table-secondary fw-normal' dir='auto'><%= format_key(change_info[:key]) %></th>
+    <td class='py-1 border-secondary-subtle border-start' dir='auto'><%= format_tag_value_with_change(change_info) %></td>
+  </tr>
+<% else %>

I think we can remove this whole branch, as `tag_with_changes` is no more.

> @@ -2,7 +2,16 @@
   <h4><%= t ".tags" %></h4>
   <div class='mb-3 border border-secondary-subtle rounded overflow-hidden'>
     <table class='mb-0 browse-tag-list table align-middle'>
-      <%= render :partial => "browse/tag", :collection => tag_details.sort %>
+      <% if defined?(tag_details_object) && @old_features %>

I remembered now a better way to do this than using `define?`. You can put a `<% tag_details_object ||= nil %>` at the top of the file. If `tag_details_object` is not defined, it will be defined with a `nil` value.

As usual `a ||= b` expands to `a = a || b`. On any assignment, Ruby causes a definition of any variables on the right-hand side that they didn't exist yet, so if `a` doesn't exist, it gets `a = nil` before entering the expression.

(But of course it would be even better if `tag_details_object` was also renamed to `current_feature` as mentioned above 😝).

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

Message ID: <openstreetmap/openstreetmap-website/pull/6448/review/3347233796 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20251016/18dc7a10/attachment.htm>