<p></p>
<p><b>@pablobm</b> commented on this pull request.</p>

<p dir="auto">Still looking into how to better resolve the duplication in <code class="notranslate">browse/_tag_details</code>, but these are some quick notes.</p><hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/6448#discussion_r2437629392">app/views/browse/_common_details.html.erb</a>:</p>
<pre style='color:#555'>> @@ -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 } %>
</pre>
<p dir="auto">How about renaming <code class="notranslate">tag_details_object</code> to <code class="notranslate">current_feature</code>? I had some difficulty reasoning about what it represented, due to the naming.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/6448#discussion_r2437630320">app/views/browse/_tag.html.erb</a>:</p>
<pre style='color:#555'>> +<% 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 %>
</pre>
<p dir="auto">I think we can remove this whole branch, as <code class="notranslate">tag_with_changes</code> is no more.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/6448#discussion_r2437638091">app/views/browse/_tag_details.html.erb</a>:</p>
<pre style='color:#555'>> @@ -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 %>
</pre>
<p dir="auto">I remembered now a better way to do this than using <code class="notranslate">define?</code>. You can put a <code class="notranslate"><% tag_details_object ||= nil %></code> at the top of the file. If <code class="notranslate">tag_details_object</code> is not defined, it will be defined with a <code class="notranslate">nil</code> value.</p>
<p dir="auto">As usual <code class="notranslate">a ||= b</code> expands to <code class="notranslate">a = a || b</code>. On any assignment, Ruby causes a definition of any variables on the right-hand side that they didn't exist yet, so if <code class="notranslate">a</code> doesn't exist, it gets <code class="notranslate">a = nil</code> before entering the expression.</p>
<p dir="auto">(But of course it would be even better if <code class="notranslate">tag_details_object</code> was also renamed to <code class="notranslate">current_feature</code> as mentioned above 😝).</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/6448#pullrequestreview-3347233796">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLL5B7MB2AMKVWHUPHD3YAOYVAVCNFSM6AAAAACJGVZNZ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGNBXGIZTGNZZGY">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLKLNEJYRKW7S74CI3D3YAOYVA5CNFSM6AAAAACJGVZNZ2WGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTWHQK6AI.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/6448/review/3347233796</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/6448#pullrequestreview-3347233796",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/6448#pullrequestreview-3347233796",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>