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

app/assets/javascripts/index/changeset.js

@@ -26,14 +26,14 @@ OSM.Changeset = function (map) {
     });
   }
 
-  function updateChangeset(form, method, url, include_data) {
+  function updateChangeset(method, url, include_data) {
     var data;
 
-    $(form).find("#comment-error").prop("hidden", true);
-    $(form).find("input[type=submit]").prop("disabled", true);
+    content.find("#comment-error").prop("hidden", true);
+    content.find("button[data-method][data-url]").prop("disabled", true);
 
     if (include_data) {
-      data = { text: $(form.text).val() };
+      data = { text: content.find("textarea").val() };
     } else {
       data = {};
     }
@@ -47,24 +47,21 @@ OSM.Changeset = function (map) {
         OSM.loadSidebarContent(window.location.pathname, page.load);
       },
       error: function (xhr) {
-        $(form).find("#comment-error").text(xhr.responseText);
-        $(form).find("#comment-error").prop("hidden", false);
-        $(form).find("input[type=submit]").prop("disabled", false);
+        content.find("button[data-method][data-url]").prop("disabled", false);
+        content.find("#comment-error")
+          .text(xhr.responseText)
+          .prop("hidden", false)
+          .get(0).scrollIntoView({ block: "nearest" });
       }
     });
   }
 
   function initialize() {
-    content.find("input[name=comment]").on("click", function (e) {
+    content.find("button[data-method][data-url]").on("click", function (e) {
       e.preventDefault();
       var data = $(e.target).data();
-      updateChangeset(e.target.form, data.method, data.url, true);
-    });
-
-    content.find(".action-button").on("click", function (e) {
-      e.preventDefault();
-      var data = $(e.target).data();
-      updateChangeset(e.target.form, data.method, data.url);
+      var include_data = e.target.name === "comment";
+      updateChangeset(data.method, data.url, include_data);
     });
 
     content.find("textarea").on("input", function (e) {

app/assets/stylesheets/common.scss

@@ -647,11 +647,6 @@ tr.turn:hover {
     }
   }
 
-  span.action-button:hover {
-    cursor: pointer;
-    text-decoration: underline;
-  }
-
   .note-description {
     overflow: hidden;
     margin: 0 0 10px 10px;

app/views/browse/changeset.html.erb

@@ -18,54 +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 @@
         <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 %>

test/application_system_test_case.rb

@@ -33,6 +33,11 @@ def sign_in_as(user)
     end
   end
 
+  def sign_out
+    visit logout_path
+    click_on "Logout", :match => :first
+  end
+
   def within_sidebar(&block)
     within "#sidebar_content", &block
   end

test/controllers/browse_controller_test.rb

@@ -142,20 +142,6 @@ def test_read_private_changeset
     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)