From: Tom Hughes <tom@compton.nu>
Date: Sun, 25 Feb 2024 09:28:48 +0000 (+0000)
Subject: Merge remote-tracking branch 'upstream/pull/4463'
X-Git-Tag: live~814
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/746bfd0a4838268eaf745e85e0e27b5acf9cf64a?hp=-c

Merge remote-tracking branch 'upstream/pull/4463'
---

746bfd0a4838268eaf745e85e0e27b5acf9cf64a
diff --combined app/assets/stylesheets/common.scss
index b5bd5adec,18eab1316..1f7c45db5
--- a/app/assets/stylesheets/common.scss
+++ b/app/assets/stylesheets/common.scss
@@@ -647,11 -647,6 +647,6 @@@ tr.turn:hover 
      }
    }
  
-   span.action-button:hover {
-     cursor: pointer;
-     text-decoration: underline;
-   }
- 
    .note-description {
      overflow: hidden;
      margin: 0 0 10px 10px;
@@@ -827,11 -822,14 +822,11 @@@
      height: 400px;
      display: none;
    }
 -  .comments {
 -    max-width: 740px;
 +  .diary-comment .col-auto {
 +    width: 62px;
    }
 -  .diary-comment {
 -    border-top: 1px dashed $grey;
 -    &:first-child {
 -      border-top: 1px solid $grey;
 -    }
 +  .diary-comment .col {
 +    max-width: 690px;
    }
  }
  
diff --combined app/views/browse/changeset.html.erb
index 501712717,88d90d0f7..ac5382b14
--- a/app/views/browse/changeset.html.erb
+++ b/app/views/browse/changeset.html.erb
@@@ -18,54 -18,41 +18,41 @@@
      <% if current_user %>
        <div class="col-auto">
          <% if @changeset.subscribers.exists?(current_user.id) %>
-           <button class="action-button btn btn-sm btn-primary" name="unsubscribe" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.unsubscribe") %></button>
+           <button class="btn btn-sm btn-primary" name="unsubscribe" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.unsubscribe") %></button>
          <% else %>
-           <button class="action-button btn btn-sm btn-primary" name="subscribe" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.subscribe") %></button>
+           <button class="btn btn-sm btn-primary" name="subscribe" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>"><%= t("javascripts.changesets.show.subscribe") %></button>
          <% end %>
        </div>
      <% end %>
    </div>
  
    <% if @comments.length > 0 %>
-     <div class='changeset-comments'>
-       <form action="#">
-         <ul class="list-unstyled">
-           <% @comments.each do |comment| %>
-             <% if comment.visible %>
-               <li id="c<%= comment.id %>">
-                 <small class='text-muted'>
-                   <%= t(".comment_by_html",
-                         :time_ago => friendly_date_ago(comment.created_at),
-                         :user => link_to(comment.author.display_name, user_path(comment.author))) %>
-                   <% if current_user and current_user.moderator? %>
-                     — <span class="action-button" data-comment-id="<%= comment.id %>" data-method="POST" data-url="<%= changeset_comment_hide_url(comment.id) %>"><%= t("javascripts.changesets.show.hide_comment") %></span>
-                   <% end %>
-                 </small>
-                 <div class="mx-2">
-                   <%= comment.body.to_html %>
-                 </div>
-               </li>
-             <% elsif current_user and current_user.moderator? %>
-               <li id="c<%= comment.id %>">
-                 <small class='text-muted'>
-                   <%= t(".hidden_comment_by_html",
-                         :time_ago => friendly_date_ago(comment.created_at),
-                         :user => link_to(comment.author.display_name, user_path(comment.author))) %>
-                   — <span class="action-button text-muted" data-comment-id="<%= comment.id %>" data-method="POST" data-url="<%= changeset_comment_unhide_url(comment.id) %>"><%= t("javascripts.changesets.show.unhide_comment") %></span>
-                  </small>
-                 <div class="mx-2">
-                   <%= comment.body.to_html %>
-                 </div>
-               </li>
+     <ul class="list-unstyled">
+       <% @comments.each do |comment| %>
+         <% next unless comment.visible || current_user&.moderator? %>
+         <li id="c<%= comment.id %>">
+           <small class='text-muted'>
+             <%= t comment.visible ? ".comment_by_html" : ".hidden_comment_by_html",
+                   :time_ago => friendly_date_ago(comment.created_at),
+                   :user => link_to(comment.author.display_name, user_path(comment.author)) %>
+             <% if current_user&.moderator? %>
+               —
+               <%= tag.button t("javascripts.changesets.show.#{comment.visible ? 'hide' : 'unhide'}_comment"),
+                              :class => "btn btn-sm small btn-link link-secondary p-0 align-baseline",
+                              :data => { :method => "POST",
+                                         :url => comment.visible ? changeset_comment_hide_url(comment) : changeset_comment_unhide_url(comment) } %>
              <% end %>
-           <% end %>
-         </ul>
-       </form>
-     </div>
+           </small>
+           <div class="mx-2">
+             <%= comment.body.to_html %>
+           </div>
+         </li>
+       <% end %>
+     </ul>
    <% end %>
  
    <% unless current_user %>
-     <p class="notice">
+     <p>
        <%= link_to(t(".join_discussion"), login_path(:referer => request.fullpath)) %>
      </p>
    <% end %>
@@@ -79,11 -66,11 +66,11 @@@
          <div id="comment-error" class="alert alert-danger p-2 mb-3" hidden>
          </div>
          <div>
-           <input type="submit" name="comment" value="<%= t("javascripts.changesets.show.comment") %>" data-changeset-id="<%= @changeset.id %>" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled="1" class="btn btn-sm btn-primary" />
+           <button name="comment" data-method="POST" data-url="<%= changeset_comment_url(@changeset) %>" disabled class="btn btn-sm btn-primary"><%= t("javascripts.changesets.show.comment") %></button>
          </div>
        </form>
      <% else %>
-       <p class="notice">
+       <p>
          <%= t(".still_open") %>
        </p>
      <% end %>
@@@ -93,11 -80,7 +80,11 @@@
      <%= render :partial => "paging_nav", :locals => { :heading => type_and_paginated_count("way", @way_pages), :pages => @way_pages, :page_param => "way_page" } %>
      <ul class="list-unstyled">
        <% @ways.each do |way| %>
 -        <li><%= link_to printable_name(way, :version => true), { :action => "way", :id => way.way_id.to_s }, { :class => link_class("way", way), :title => link_title(way) } %></li>
 +        <%= element_list_item "way", way do %>
 +          <%= t "printable_name.current_and_old_links_html",
 +                :current_link => link_to(printable_element_name(way), way_path(way.way_id)),
 +                :old_link => link_to(printable_element_version(way), old_way_path(way.way_id, way.version)) %>
 +        <% end %>
        <% end %>
      </ul>
    <% end %>
@@@ -106,11 -89,7 +93,11 @@@
      <%= render :partial => "paging_nav", :locals => { :heading => type_and_paginated_count("relation", @relation_pages), :pages => @relation_pages, :page_param => "relation_page" } %>
      <ul class="list-unstyled">
        <% @relations.each do |relation| %>
 -        <li><%= link_to printable_name(relation, :version => true), { :action => "relation", :id => relation.relation_id.to_s }, { :class => link_class("relation", relation), :title => link_title(relation) } %></li>
 +        <%= element_list_item "relation", relation do %>
 +          <%= t "printable_name.current_and_old_links_html",
 +                :current_link => link_to(printable_element_name(relation), relation_path(relation.relation_id)),
 +                :old_link => link_to(printable_element_version(relation), old_relation_path(relation.relation_id, relation.version)) %>
 +        <% end %>
        <% end %>
      </ul>
    <% end %>
@@@ -119,22 -98,12 +106,22 @@@
      <%= render :partial => "paging_nav", :locals => { :heading => type_and_paginated_count("node", @node_pages), :pages => @node_pages, :page_param => "node_page" } %>
      <ul class="list-unstyled">
        <% @nodes.each do |node| %>
 -        <li><%= link_to printable_name(node, :version => true), { :action => "node", :id => node.node_id.to_s }, { :class => link_class("node", node), :title => link_title(node), :rel => link_follow(node) } %></li>
 +        <%= element_list_item "node", node do %>
 +          <%= t "printable_name.current_and_old_links_html",
 +                :current_link => link_to(printable_element_name(node), node_path(node.node_id), { :rel => link_follow(node) }),
 +                :old_link => link_to(printable_element_version(node), old_node_path(node.node_id, node.version), { :rel => link_follow(node) }) %>
 +        <% end %>
        <% end %>
      </ul>
    <% end %>
  </div>
  
 +<div class='secondary-actions'>
 +  <%= link_to(t(".changesetxml"), :controller => "api/changesets", :action => "show") %>
 +  &middot;
 +  <%= link_to(t(".osmchangexml"), :controller => "api/changesets", :action => "download") %>
 +</div>
 +
  <% if @next_by_user || @prev_by_user %>
    <div class='secondary-actions'>
      <% if @prev_by_user %>
@@@ -155,3 -124,9 +142,3 @@@
      <% end %>
    </div>
  <% end %>
 -
 -<div class='secondary-actions'>
 -  <%= link_to(t(".changesetxml"), :controller => "api/changesets", :action => "show") %>
 -  &middot;
 -  <%= link_to(t(".osmchangexml"), :controller => "api/changesets", :action => "download") %>
 -</div>
diff --combined test/controllers/browse_controller_test.rb
index 1023d76ae,0709d544e..2bb743636
--- a/test/controllers/browse_controller_test.rb
+++ b/test/controllers/browse_controller_test.rb
@@@ -142,29 -142,6 +142,15 @@@ class BrowseControllerTest < ActionDisp
      browse_check :changeset_path, changeset.id, "browse/changeset"
    end
  
-   def test_read_changeset_hidden_comments
-     changeset = create(:changeset)
-     create_list(:changeset_comment, 3, :changeset => changeset)
-     create(:changeset_comment, :visible => false, :changeset => changeset)
- 
-     browse_check :changeset_path, changeset.id, "browse/changeset"
-     assert_select "div.changeset-comments ul li", :count => 3
- 
-     session_for(create(:moderator_user))
- 
-     browse_check :changeset_path, changeset.id, "browse/changeset"
-     assert_select "div.changeset-comments ul li", :count => 4
-   end
- 
 +  def test_read_changeset_element_links
 +    changeset = create(:changeset)
 +    node = create(:node, :with_history, :changeset => changeset)
 +
 +    browse_check :changeset_path, changeset.id, "browse/changeset"
 +    assert_dom "a[href='#{node_path node}']", :count => 1
 +    assert_dom "a[href='#{old_node_path node, 1}']", :count => 1
 +  end
 +
    ##
    #  Methods to check redaction.
    #