From fbaa5c23886d992e35400e2fe1454dd2c240c96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 01:13:14 +0200 Subject: [PATCH 01/12] Remove unused commentable tree partial It was created in commit 83d254ad, but it was never used, since the commit creating it removed the code rendering the `budgets/investments/comments` partial, which this partial was supposed to replace. --- app/views/comments/_commentable_tree.html.erb | 33 ------------------- 1 file changed, 33 deletions(-) delete mode 100644 app/views/comments/_commentable_tree.html.erb diff --git a/app/views/comments/_commentable_tree.html.erb b/app/views/comments/_commentable_tree.html.erb deleted file mode 100644 index 8ec0fb268..000000000 --- a/app/views/comments/_commentable_tree.html.erb +++ /dev/null @@ -1,33 +0,0 @@ -<% valuation = local_assigns.fetch(:valuation, false) %> -<% allow_comments = local_assigns.fetch(:allow_comments, true) %> -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@investment), @comment_tree.comments, @comment_tree.comment_authors, @investment.comments_count, @comment_flags] do %> -
-
-
-

- <%= t("debates.show.comments_title") %> - (<%= @investment.comments_count %>) -

- - <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> - - <% if user_signed_in? && allow_comments %> - <%= render "comments/form", { commentable: @investment, - parent_id: nil, - toggeable: false, - valuation: valuation } %> - <% else %> -
- <%= render "shared/login_to_comment" %> - <% end %> - - <% @comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", { comment: comment, - valuation: valuation, - allow_comments: allow_comments } %> - <% end %> - <%= paginate @comment_tree.root_comments %> -
-
-
-<% end %> From 573f861ad154d224085b07d6074fe3c7e985e4bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 02:34:20 +0200 Subject: [PATCH 02/12] Don't use comment_flags to cache comments Flagging a comment automatically updates the comment, so the cache expires anyway, making the `comment_flags` variable redundant. --- app/views/budgets/investments/show.html.erb | 1 - app/views/comments/_comment.html.erb | 3 +-- app/views/comments/_comment_tree.html.erb | 3 +-- app/views/debates/_comments.html.erb | 2 +- app/views/legislation/annotations/show.html.erb | 1 - app/views/legislation/proposals/_comments.html.erb | 2 +- app/views/legislation/questions/show.html.erb | 1 - app/views/polls/_comments.html.erb | 2 +- app/views/proposals/_comments.html.erb | 2 +- app/views/topics/_comments.html.erb | 2 +- .../valuation/budget_investments/_valuation_comments.html.erb | 1 - 11 files changed, 7 insertions(+), 13 deletions(-) diff --git a/app/views/budgets/investments/show.html.erb b/app/views/budgets/investments/show.html.erb index de38661a5..b0eee0ec3 100644 --- a/app/views/budgets/investments/show.html.erb +++ b/app/views/budgets/investments/show.html.erb @@ -19,7 +19,6 @@
<%= render "/comments/comment_tree", comment_tree: @comment_tree, - comment_flags: @comment_flags, display_comments_count: false %>
diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 0f51daac0..2448dc211 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,10 +1,9 @@ -<% comment_flags ||= @comment_flags %> <% valuation = local_assigns.fetch(:valuation, false) %> <% allow_votes = local_assigns.fetch(:allow_votes, true) %> <% allow_actions = local_assigns.fetch(:allow_actions, true) %> <% allow_comments = local_assigns.fetch(:allow_comments, true) %> <% admin_layout = local_assigns.fetch(:admin_layout, false) %> -<% cache [locale_and_user_status(comment), comment, commentable_cache_key(comment.commentable), comment.author, (comment_flags[comment.id] if comment_flags), (admin_layout if admin_layout)] do %> +<% cache [locale_and_user_status(comment), comment, commentable_cache_key(comment.commentable), comment.author, (admin_layout if admin_layout)] do %>
  • <% if comment.hidden? || comment.user.hidden? %> diff --git a/app/views/comments/_comment_tree.html.erb b/app/views/comments/_comment_tree.html.erb index 78acd65c4..03d23c141 100644 --- a/app/views/comments/_comment_tree.html.erb +++ b/app/views/comments/_comment_tree.html.erb @@ -2,7 +2,7 @@ <% valuation = local_assigns.fetch(:valuation, false) %> <% allow_comments = local_assigns.fetch(:allow_comments, true) %> <% admin_layout = local_assigns.fetch(:admin_layout, false) %> -<% cache [locale_and_user_status, comment_tree.order, commentable_cache_key(commentable), comment_tree.comments, comment_tree.comment_authors, commentable.comments_count, comment_flags, admin_layout] do %> +<% cache [locale_and_user_status, comment_tree.order, commentable_cache_key(commentable), comment_tree.comments, comment_tree.comment_authors, commentable.comments_count, admin_layout] do %>
    @@ -39,7 +39,6 @@ <% comment_tree.root_comments.each do |comment| %> <%= render "comments/comment", { comment: comment, - comment_flags: comment_flags, valuation: valuation, allow_votes: !valuation, allow_actions: !valuation, diff --git a/app/views/debates/_comments.html.erb b/app/views/debates/_comments.html.erb index aec0a1414..cf7f553d0 100644 --- a/app/views/debates/_comments.html.erb +++ b/app/views/debates/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@debate), @comment_tree.comments, @comment_tree.comment_authors, @debate.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@debate), @comment_tree.comments, @comment_tree.comment_authors, @debate.comments_count] do %>

    diff --git a/app/views/legislation/annotations/show.html.erb b/app/views/legislation/annotations/show.html.erb index f8aeeb655..d5297bc50 100644 --- a/app/views/legislation/annotations/show.html.erb +++ b/app/views/legislation/annotations/show.html.erb @@ -43,7 +43,6 @@

    <%= render "/comments/comment_tree", comment_tree: @comment_tree, - comment_flags: @comment_flags, display_comments_count: true %>
    diff --git a/app/views/legislation/proposals/_comments.html.erb b/app/views/legislation/proposals/_comments.html.erb index a3b0c635e..61efe69e2 100644 --- a/app/views/legislation/proposals/_comments.html.erb +++ b/app/views/legislation/proposals/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@proposal), @comment_tree.comments, @comment_tree.comment_authors, @proposal.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@proposal), @comment_tree.comments, @comment_tree.comment_authors, @proposal.comments_count] do %>
    <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> diff --git a/app/views/legislation/questions/show.html.erb b/app/views/legislation/questions/show.html.erb index e8bd1a4f0..dec291438 100644 --- a/app/views/legislation/questions/show.html.erb +++ b/app/views/legislation/questions/show.html.erb @@ -43,6 +43,5 @@
    <%= render "/comments/comment_tree", comment_tree: @comment_tree, - comment_flags: @comment_flags, display_comments_count: true %>
    diff --git a/app/views/polls/_comments.html.erb b/app/views/polls/_comments.html.erb index 5ac2dfb64..edab112a7 100644 --- a/app/views/polls/_comments.html.erb +++ b/app/views/polls/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@poll), @comment_tree.comments, @comment_tree.comment_authors, @poll.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@poll), @comment_tree.comments, @comment_tree.comment_authors, @poll.comments_count] do %>
    <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> diff --git a/app/views/proposals/_comments.html.erb b/app/views/proposals/_comments.html.erb index a3b0c635e..61efe69e2 100644 --- a/app/views/proposals/_comments.html.erb +++ b/app/views/proposals/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@proposal), @comment_tree.comments, @comment_tree.comment_authors, @proposal.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@proposal), @comment_tree.comments, @comment_tree.comment_authors, @proposal.comments_count] do %>
    <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> diff --git a/app/views/topics/_comments.html.erb b/app/views/topics/_comments.html.erb index a5ca165fe..e6e73e79e 100644 --- a/app/views/topics/_comments.html.erb +++ b/app/views/topics/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, @current_order, commentable_cache_key(@topic), @comment_tree.comments, @comment_tree.comment_authors, @topic.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@topic), @comment_tree.comments, @comment_tree.comment_authors, @topic.comments_count] do %>
    diff --git a/app/views/valuation/budget_investments/_valuation_comments.html.erb b/app/views/valuation/budget_investments/_valuation_comments.html.erb index f6b4c9a53..b852f93e9 100644 --- a/app/views/valuation/budget_investments/_valuation_comments.html.erb +++ b/app/views/valuation/budget_investments/_valuation_comments.html.erb @@ -1,7 +1,6 @@

    <%= t("valuation.budget_investments.valuation_comments") %>

    <% unless @comment_tree.nil? %> <%= render "/comments/comment_tree", comment_tree: @comment_tree, - comment_flags: @comment_flags, display_comments_count: false, valuation: true, allow_comments: can?(:comment_valuation, @investment), From 06b7c6dbd396cb0245125a6422683c769679a0e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 02:57:48 +0200 Subject: [PATCH 03/12] Simplify comment partial variables We were passing around many variables to condition the way we display the comment. However, in the end we only had one place where these variables were used: valuation. So we can make everything depend on the valuation variable. --- app/views/comments/_comment.html.erb | 20 ++++++------------- app/views/comments/_comment_tree.html.erb | 13 +++--------- .../_valuation_comments.html.erb | 4 +--- 3 files changed, 10 insertions(+), 27 deletions(-) diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 2448dc211..d8bf131b3 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,9 +1,5 @@ <% valuation = local_assigns.fetch(:valuation, false) %> -<% allow_votes = local_assigns.fetch(:allow_votes, true) %> -<% allow_actions = local_assigns.fetch(:allow_actions, true) %> -<% allow_comments = local_assigns.fetch(:allow_comments, true) %> -<% admin_layout = local_assigns.fetch(:admin_layout, false) %> -<% cache [locale_and_user_status(comment), comment, commentable_cache_key(comment.commentable), comment.author, (admin_layout if admin_layout)] do %> +<% cache [locale_and_user_status(comment), comment, commentable_cache_key(comment.commentable), comment.author] do %>
    • <% if comment.hidden? || comment.user.hidden? %> @@ -32,7 +28,7 @@ <% if comment.as_administrator? %> <%= t("comments.comment.admin") %> - <% if admin_layout %> + <% if valuation %> <%= Administrator.find(comment.administrator_id).description_or_name %> <% else %> #<%= comment.administrator_id %> @@ -83,7 +79,7 @@
    - <% if allow_votes %> + <% unless valuation %>
    <%= render "comments/votes", comment: comment %>
    @@ -106,11 +102,11 @@ <%= link_to(comment_link_text(comment), "", class: "js-add-comment-link", data: { "id": dom_id(comment) }) %> - <% if allow_actions %> + <% unless valuation %> <%= render "comments/actions", { comment: comment } %> <% end %> - <% if allow_comments %> + <% if !valuation || can?(:comment_valuation, comment.commentable) %> <%= render "comments/form", { commentable: comment.commentable, parent_id: comment.id, toggeable: true, @@ -125,11 +121,7 @@
      <% child_comments_of(comment).each do |child| %>
    • - <%= render "comments/comment", { comment: child, - valuation: valuation, - allow_votes: allow_votes, - allow_actions: allow_actions, - allow_comments: allow_comments } %> + <%= render "comments/comment", { comment: child, valuation: valuation } %>
      • <% end %>
    diff --git a/app/views/comments/_comment_tree.html.erb b/app/views/comments/_comment_tree.html.erb index 03d23c141..88d3d0f0b 100644 --- a/app/views/comments/_comment_tree.html.erb +++ b/app/views/comments/_comment_tree.html.erb @@ -1,8 +1,6 @@ <% commentable = comment_tree.commentable %> <% valuation = local_assigns.fetch(:valuation, false) %> -<% allow_comments = local_assigns.fetch(:allow_comments, true) %> -<% admin_layout = local_assigns.fetch(:admin_layout, false) %> -<% cache [locale_and_user_status, comment_tree.order, commentable_cache_key(commentable), comment_tree.comments, comment_tree.comment_authors, commentable.comments_count, admin_layout] do %> +<% cache [locale_and_user_status, comment_tree.order, commentable_cache_key(commentable), comment_tree.comments, comment_tree.comment_authors, commentable.comments_count] do %>
    @@ -26,7 +24,7 @@
    <%= sanitize(t("comments.verified_only", verify_account: link_to_verify_account)) %>
    - <% elsif allow_comments %> + <% elsif !valuation || can?(:comment_valuation, commentable) %> <%= render "comments/form", { commentable: commentable, parent_id: nil, toggeable: false, @@ -38,12 +36,7 @@ <% end %> <% comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", { comment: comment, - valuation: valuation, - allow_votes: !valuation, - allow_actions: !valuation, - allow_comments: allow_comments, - admin_layout: admin_layout } %> + <%= render "comments/comment", { comment: comment, valuation: valuation } %> <% end %> <%= paginate comment_tree.root_comments %>
    diff --git a/app/views/valuation/budget_investments/_valuation_comments.html.erb b/app/views/valuation/budget_investments/_valuation_comments.html.erb index b852f93e9..39f9995dc 100644 --- a/app/views/valuation/budget_investments/_valuation_comments.html.erb +++ b/app/views/valuation/budget_investments/_valuation_comments.html.erb @@ -2,7 +2,5 @@ <% unless @comment_tree.nil? %> <%= render "/comments/comment_tree", comment_tree: @comment_tree, display_comments_count: false, - valuation: true, - allow_comments: can?(:comment_valuation, @investment), - admin_layout: true %> + valuation: true %> <% end %> From 255e56c0f2516e4a7ea9e5ceeadc8d310c5af9e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 15:21:36 +0200 Subject: [PATCH 04/12] Simplify toggleClass call This method accepts several classes as arguments, so there's no need to call it twice. --- app/assets/javascripts/comments.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/comments.js b/app/assets/javascripts/comments.js index e8127b705..8ec7d28c4 100644 --- a/app/assets/javascripts/comments.js +++ b/app/assets/javascripts/comments.js @@ -40,7 +40,7 @@ $("#js-comment-form-" + id).toggle(); }, toggle_arrow: function(id) { - $("span#" + id + "_arrow").toggleClass("fa-minus-square").toggleClass("fa-plus-square"); + $("span#" + id + "_arrow").toggleClass("fa-minus-square fa-plus-square"); }, initialize: function() { $("body").on("click", ".js-add-comment-link", function() { From 41b9d2ba017beca7cae050bfd68cc504e55118a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 21:04:09 +0200 Subject: [PATCH 05/12] Simplify code to show/collapse comment replies Tests are also a bit easier to read, even though we need to use the `text:` option to find links because otherwise the text in the hidden `` tags will cause `click_link` to miss the link we want to click. Here's an explanation by one of Capybara's authors: https://github.com/teamcapybara/capybara/issues/2347#issuecomment-626373440 --- app/assets/javascripts/comments.js | 9 ++----- app/views/comments/_comment.html.erb | 2 +- .../comments/budget_investments_spec.rb | 12 +++++++--- .../budget_investments_valuation_spec.rb | 14 +++++++---- spec/system/comments/debates_spec.rb | 12 +++++++--- .../comments/legislation_annotations_spec.rb | 12 +++++++--- .../comments/legislation_questions_spec.rb | 12 +++++++--- spec/system/comments/polls_spec.rb | 12 +++++++--- spec/system/comments/proposals_spec.rb | 12 +++++++--- spec/system/comments/topics_spec.rb | 24 ++++++++++++++----- 10 files changed, 85 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/comments.js b/app/assets/javascripts/comments.js index 8ec7d28c4..250efc340 100644 --- a/app/assets/javascripts/comments.js +++ b/app/assets/javascripts/comments.js @@ -39,9 +39,6 @@ toggle_form: function(id) { $("#js-comment-form-" + id).toggle(); }, - toggle_arrow: function(id) { - $("span#" + id + "_arrow").toggleClass("fa-minus-square fa-plus-square"); - }, initialize: function() { $("body").on("click", ".js-add-comment-link", function() { App.Comments.toggle_form($(this).data().id); @@ -49,10 +46,8 @@ }); $("body").on("click", ".js-toggle-children", function() { - var children_container_id; - children_container_id = ($(this).data().id) + "_children"; - $("#" + children_container_id).toggle("slow"); - App.Comments.toggle_arrow(children_container_id); + $("#" + $(this).data().id + "_children").toggle("slow"); + $(this).children(".far").toggleClass("fa-minus-square fa-plus-square"); $(this).children(".js-child-toggle").toggle(); return false; }); diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index d8bf131b3..991759e29 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -89,7 +89,7 @@ <%= link_to "", class: "js-toggle-children relative", data: { "id": "#{dom_id(comment)}" } do %> <%= t("shared.hide") %> - + <%= t("comments.comment.responses_collapse", count: comment.children.size) %> <% end %> diff --git a/spec/system/comments/budget_investments_spec.rb b/spec/system/comments/budget_investments_spec.rb index 2970da0df..8e58c0773 100644 --- a/spec/system/comments/budget_investments_spec.rb +++ b/spec/system/comments/budget_investments_spec.rb @@ -66,20 +66,26 @@ describe "Commenting Budget::Investments" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/budget_investments_valuation_spec.rb b/spec/system/comments/budget_investments_valuation_spec.rb index 9e43dda3b..c4d3c7990 100644 --- a/spec/system/comments/budget_investments_valuation_spec.rb +++ b/spec/system/comments/budget_investments_valuation_spec.rb @@ -61,7 +61,7 @@ describe "Internal valuation comments on Budget::Investments" do scenario "Collapsable comments", :js do parent_comment = create(:comment, :valuation, author: valuator_user, body: "Main comment", commentable: investment) - child_comment = create(:comment, :valuation, author: valuator_user, body: "First child", + child_comment = create(:comment, :valuation, author: valuator_user, body: "First subcomment", commentable: investment, parent: parent_comment) grandchild_comment = create(:comment, :valuation, author: valuator_user, parent: child_comment, @@ -73,20 +73,26 @@ describe "Internal valuation comments on Budget::Investments" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index 1b5db3283..78af230b1 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -63,20 +63,26 @@ describe "Commenting debates" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index 18b9f40d2..4354da1f8 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -71,20 +71,26 @@ describe "Commenting legislation questions" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: parent_comment.body do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/legislation_questions_spec.rb b/spec/system/comments/legislation_questions_spec.rb index 67510cae2..09cc165dd 100644 --- a/spec/system/comments/legislation_questions_spec.rb +++ b/spec/system/comments/legislation_questions_spec.rb @@ -69,20 +69,26 @@ describe "Commenting legislation questions" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/polls_spec.rb b/spec/system/comments/polls_spec.rb index ad4db74ec..65fd9af57 100644 --- a/spec/system/comments/polls_spec.rb +++ b/spec/system/comments/polls_spec.rb @@ -64,20 +64,26 @@ describe "Commenting polls" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/proposals_spec.rb b/spec/system/comments/proposals_spec.rb index e1b74613b..84aa7c219 100644 --- a/spec/system/comments/proposals_spec.rb +++ b/spec/system/comments/proposals_spec.rb @@ -62,20 +62,26 @@ describe "Commenting proposals" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") diff --git a/spec/system/comments/topics_spec.rb b/spec/system/comments/topics_spec.rb index 4afc13ff1..4d5d48d46 100644 --- a/spec/system/comments/topics_spec.rb +++ b/spec/system/comments/topics_spec.rb @@ -67,20 +67,26 @@ describe "Commenting topics from proposals" do expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).to have_content("1 response (collapse)") expect(page).to have_content("1 response (show)") expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content("1 response (collapse)", count: 2) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).to have_content("1 response (show)") @@ -625,17 +631,23 @@ describe "Commenting topics from budget investments" do expect(page).to have_css(".comment", count: 3) - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (collapse)" + end expect(page).to have_css(".comment", count: 2) expect(page).not_to have_content grandchild_comment.body - find("#comment_#{child_comment.id}_children_arrow").click + within ".comment .comment", text: "First subcomment" do + click_link text: "1 response (show)" + end expect(page).to have_css(".comment", count: 3) expect(page).to have_content grandchild_comment.body - find("#comment_#{parent_comment.id}_children_arrow").click + within ".comment", text: "Main comment" do + click_link text: "1 response (collapse)", match: :first + end expect(page).to have_css(".comment", count: 1) expect(page).not_to have_content child_comment.body From bedd8790257b6ae0a194715f358379ad1ec1275a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 15:46:40 +0200 Subject: [PATCH 06/12] Remove redundant show/hide screen reader texts These texts are redundant since we added the same texts for everyone in commit 6b85ed87. --- app/views/comments/_comment.html.erb | 2 -- config/locales/en/general.yml | 1 - config/locales/es/general.yml | 1 - 3 files changed, 4 deletions(-) diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 991759e29..fec693a48 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -87,8 +87,6 @@ <% if comment.children.size > 0 %> <%= link_to "", class: "js-toggle-children relative", data: { "id": "#{dom_id(comment)}" } do %> - - <%= t("shared.hide") %> <%= t("comments.comment.responses_collapse", count: comment.children.size) %> diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index feadd4cd5..24fb4fe6f 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -723,7 +723,6 @@ en: notice: "Now you are following this citizen proposal!
    We will notify you of changes as they occur so that you are up-to-date." destroy: notice: "You have stopped following this citizen proposal!
    You will no longer receive notifications related to this proposal." - hide: Hide print: print_button: Print this info search: Search diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index fb88107af..92de921d8 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -721,7 +721,6 @@ es: notice: "¡Ahora estás siguiendo esta propuesta ciudadana!
    Te notificaremos los cambios a medida que se produzcan para que estés al día." destroy: notice: "¡Has dejado de seguir esta propuesta ciudadana!
    Ya no recibirás más notificaciones relacionadas con esta propuesta." - hide: Ocultar print: print_button: Imprimir esta información search: Buscar From 4627372a6277e4da4cbcef7f4f55b8b4ac1ea4e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 20:14:15 +0200 Subject: [PATCH 07/12] Use a
      tag for a list of comments We were using a
        tag for a single comment, where the first element of the list was the comment itself and the second element was the list of replies. IMHO it makes more sense to have a list of all comments, where every element is a comment and inside it there's a list of replies. We're also rendering the list even if it has no children so it's easier to add comments through JavaScript. Then we use the :empty CSS selector to hide the list if it's empty. However, since ERB adds whitespace if we structure our code the usual way and current browsers don't recognize elements with whitespace as empty, we have to use the `tag` helper so no whitespace is added. --- app/assets/javascripts/comments.js | 9 +++----- app/assets/stylesheets/layout.scss | 20 +++++++++++----- app/views/comments/_comment.html.erb | 23 ++++++------------- app/views/comments/_comment_list.html.erb | 7 ++++++ app/views/comments/_comment_tree.html.erb | 4 +--- app/views/comments/create.js.erb | 2 +- app/views/debates/_comments.html.erb | 4 +--- .../legislation/proposals/_comments.html.erb | 4 +--- app/views/polls/_comments.html.erb | 4 +--- app/views/proposals/_comments.html.erb | 4 +--- app/views/topics/_comments.html.erb | 5 +--- .../comments/budget_investments_spec.rb | 18 +++++++-------- spec/system/comments/debates_spec.rb | 18 +++++++-------- .../comments/legislation_annotations_spec.rb | 18 +++++++-------- .../comments/legislation_questions_spec.rb | 18 +++++++-------- spec/system/comments/proposals_spec.rb | 18 +++++++-------- 16 files changed, 83 insertions(+), 93 deletions(-) create mode 100644 app/views/comments/_comment_list.html.erb diff --git a/app/assets/javascripts/comments.js b/app/assets/javascripts/comments.js index 250efc340..f2c5411ca 100644 --- a/app/assets/javascripts/comments.js +++ b/app/assets/javascripts/comments.js @@ -2,14 +2,11 @@ "use strict"; App.Comments = { add_comment: function(parent_id, response_html) { - $(response_html).insertAfter($("#js-comment-form-" + parent_id)); + $(".comment-list:first").prepend($(response_html)); this.update_comments_count(); }, add_reply: function(parent_id, response_html) { - if ($("#" + parent_id + " .comment-children").length === 0) { - $("#" + parent_id).append("
          • "); - } - $("#" + parent_id + " .comment-children:first").prepend($(response_html)); + $("#" + parent_id + " .comment-list:first").prepend($(response_html)); this.update_comments_count(); }, update_comments_count: function() { @@ -46,7 +43,7 @@ }); $("body").on("click", ".js-toggle-children", function() { - $("#" + $(this).data().id + "_children").toggle("slow"); + $(this).closest(".comment").find(".comment-list:first").toggle("slow"); $(this).children(".far").toggleClass("fa-minus-square fa-plus-square"); $(this).children(".js-child-toggle").toggle(); return false; diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 82aee720f..977c6fe6b 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2084,6 +2084,7 @@ table { } .comment { + line-height: $list-lineheight; margin: $line-height / 4 0; position: relative; @@ -2171,12 +2172,19 @@ table { } } -.comment-children { - border-left: 1px dashed $border; - display: inline-block; - margin-left: rem-calc(16); - padding-left: rem-calc(8); - width: 100%; +.comment-list { + margin: $line-height / 4 0; + + .comment-list { + border-left: 1px dashed $border; + display: inline-block; + padding-left: rem-calc(8); + width: 100%; + } + + &:empty { + display: none; + } } .comment-info { diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index fec693a48..443c69a01 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,7 +1,7 @@ <% valuation = local_assigns.fetch(:valuation, false) %> <% cache [locale_and_user_status(comment), comment, commentable_cache_key(comment.commentable), comment.author] do %> -
            -
          • +
            +
            <% if comment.hidden? || comment.user.hidden? %> <% if comment.children.size > 0 %>
            @@ -86,7 +86,7 @@ <% end %> <% if comment.children.size > 0 %> - <%= link_to "", class: "js-toggle-children relative", data: { "id": "#{dom_id(comment)}" } do %> + <%= link_to "", class: "js-toggle-children relative" do %> <%= t("comments.comment.responses_collapse", count: comment.children.size) %> @@ -113,17 +113,8 @@ <% end %>
            <% end %> -
            • - <% unless child_comments_of(comment).empty? %> -
          • -
              - <% child_comments_of(comment).each do |child| %> -
            • - <%= render "comments/comment", { comment: child, valuation: valuation } %> -
              • - <% end %> -
            -
            • - <% end %> -
          +
      + + <%= render "comments/comment_list", comments: child_comments_of(comment), valuation: valuation %> +
      <% end %> diff --git a/app/views/comments/_comment_list.html.erb b/app/views/comments/_comment_list.html.erb new file mode 100644 index 000000000..26138c7f5 --- /dev/null +++ b/app/views/comments/_comment_list.html.erb @@ -0,0 +1,7 @@ +<% valuation = local_assigns.fetch(:valuation, false) %> + +<%= tag.ul class: "no-bullet comment-list" do %> + <% comments.each do |comment| %> + <%= tag.li render("comments/comment", { comment: comment, valuation: valuation }) %> + <% end %> +<% end %> diff --git a/app/views/comments/_comment_tree.html.erb b/app/views/comments/_comment_tree.html.erb index 88d3d0f0b..055f94e4d 100644 --- a/app/views/comments/_comment_tree.html.erb +++ b/app/views/comments/_comment_tree.html.erb @@ -35,9 +35,7 @@ <%= render "shared/login_to_comment" %> <% end %> - <% comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", { comment: comment, valuation: valuation } %> - <% end %> + <%= render "comments/comment_list", comments: comment_tree.root_comments, valuation: valuation %> <%= paginate comment_tree.root_comments %>
      diff --git a/app/views/comments/create.js.erb b/app/views/comments/create.js.erb index aa2bd9b1c..ff8c068bf 100644 --- a/app/views/comments/create.js.erb +++ b/app/views/comments/create.js.erb @@ -1,4 +1,4 @@ -var comment_html = "<%= j(render @comment) %>" +var comment_html = "
    • <%= j(render @comment) %>
      • " <% if @comment.root? -%> var commentable_id = "<%= dom_id(@commentable) %>"; diff --git a/app/views/debates/_comments.html.erb b/app/views/debates/_comments.html.erb index cf7f553d0..46cf3fb57 100644 --- a/app/views/debates/_comments.html.erb +++ b/app/views/debates/_comments.html.erb @@ -15,9 +15,7 @@ <%= render "shared/login_to_comment" %> <% end %> - <% @comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", comment: comment %> - <% end %> + <%= render "comments/comment_list", comments: @comment_tree.root_comments %> <%= paginate @comment_tree.root_comments %>
    diff --git a/app/views/legislation/proposals/_comments.html.erb b/app/views/legislation/proposals/_comments.html.erb index 61efe69e2..a7471102d 100644 --- a/app/views/legislation/proposals/_comments.html.erb +++ b/app/views/legislation/proposals/_comments.html.erb @@ -10,9 +10,7 @@ <%= render "shared/login_to_comment" %> <% end %> - <% @comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", comment: comment %> - <% end %> + <%= render "comments/comment_list", comments: @comment_tree.root_comments %> <%= paginate @comment_tree.root_comments %> diff --git a/app/views/polls/_comments.html.erb b/app/views/polls/_comments.html.erb index edab112a7..d3309d79d 100644 --- a/app/views/polls/_comments.html.erb +++ b/app/views/polls/_comments.html.erb @@ -10,9 +10,7 @@ <%= render "shared/login_to_comment" %> <% end %> - <% @comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", comment: comment %> - <% end %> + <%= render "comments/comment_list", comments: @comment_tree.root_comments %> <%= paginate @comment_tree.root_comments %> diff --git a/app/views/proposals/_comments.html.erb b/app/views/proposals/_comments.html.erb index 61efe69e2..a7471102d 100644 --- a/app/views/proposals/_comments.html.erb +++ b/app/views/proposals/_comments.html.erb @@ -10,9 +10,7 @@ <%= render "shared/login_to_comment" %> <% end %> - <% @comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", comment: comment %> - <% end %> + <%= render "comments/comment_list", comments: @comment_tree.root_comments %> <%= paginate @comment_tree.root_comments %> diff --git a/app/views/topics/_comments.html.erb b/app/views/topics/_comments.html.erb index e6e73e79e..485096378 100644 --- a/app/views/topics/_comments.html.erb +++ b/app/views/topics/_comments.html.erb @@ -4,10 +4,7 @@
    <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> - <% @comment_tree.root_comments.each do |comment| %> - <%= render "comments/comment", comment: comment %> - <% end %> - + <%= render "comments/comment_list", comments: @comment_tree.root_comments %> <%= paginate @comment_tree.root_comments %> <% if user_signed_in? %> diff --git a/spec/system/comments/budget_investments_spec.rb b/spec/system/comments/budget_investments_spec.rb index 8e58c0773..3c922aee7 100644 --- a/spec/system/comments/budget_investments_spec.rb +++ b/spec/system/comments/budget_investments_spec.rb @@ -23,22 +23,22 @@ describe "Commenting Budget::Investments" do end scenario "Show" do - parent_comment = create(:comment, commentable: investment) - first_child = create(:comment, commentable: investment, parent: parent_comment) - second_child = create(:comment, commentable: investment, parent: parent_comment) + parent_comment = create(:comment, commentable: investment, body: "Parent") + create(:comment, commentable: investment, parent: parent_comment, body: "First subcomment") + create(:comment, commentable: investment, parent: parent_comment, body: "Last subcomment") visit comment_path(parent_comment) expect(page).to have_css(".comment", count: 3) - expect(page).to have_content parent_comment.body - expect(page).to have_content first_child.body - expect(page).to have_content second_child.body + expect(page).to have_content "Parent" + expect(page).to have_content "First subcomment" + expect(page).to have_content "Last subcomment" expect(page).to have_link "Go back to #{investment.title}", href: budget_investment_path(investment.budget, investment) - expect(page).to have_selector("ul#comment_#{parent_comment.id}>li", count: 2) - expect(page).to have_selector("ul#comment_#{first_child.id}>li", count: 1) - expect(page).to have_selector("ul#comment_#{second_child.id}>li", count: 1) + within ".comment", text: "Parent" do + expect(page).to have_selector(".comment", count: 2) + end end scenario "Link to comment show" do diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index 78af230b1..b9943c10c 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -20,22 +20,22 @@ describe "Commenting debates" do end scenario "Show" do - parent_comment = create(:comment, commentable: debate) - first_child = create(:comment, commentable: debate, parent: parent_comment) - second_child = create(:comment, commentable: debate, parent: parent_comment) + parent_comment = create(:comment, commentable: debate, body: "Parent") + create(:comment, commentable: debate, parent: parent_comment, body: "First subcomment") + create(:comment, commentable: debate, parent: parent_comment, body: "Last subcomment") visit comment_path(parent_comment) expect(page).to have_css(".comment", count: 3) - expect(page).to have_content parent_comment.body - expect(page).to have_content first_child.body - expect(page).to have_content second_child.body + expect(page).to have_content "Parent" + expect(page).to have_content "First subcomment" + expect(page).to have_content "Last subcomment" expect(page).to have_link "Go back to #{debate.title}", href: debate_path(debate) - expect(page).to have_selector("ul#comment_#{parent_comment.id}>li", count: 2) - expect(page).to have_selector("ul#comment_#{first_child.id}>li", count: 1) - expect(page).to have_selector("ul#comment_#{second_child.id}>li", count: 1) + within ".comment", text: "Parent" do + expect(page).to have_selector(".comment", count: 2) + end end scenario "Link to comment show" do diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index 4354da1f8..0ed8ef015 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -22,25 +22,25 @@ describe "Commenting legislation questions" do end scenario "Show" do - parent_comment = create(:comment, commentable: legislation_annotation) - first_child = create(:comment, commentable: legislation_annotation, parent: parent_comment) - second_child = create(:comment, commentable: legislation_annotation, parent: parent_comment) href = legislation_process_draft_version_annotation_path(legislation_annotation.draft_version.process, legislation_annotation.draft_version, legislation_annotation) + parent_comment = create(:comment, commentable: legislation_annotation, body: "Parent") + create(:comment, commentable: legislation_annotation, parent: parent_comment, body: "First subcomment") + create(:comment, commentable: legislation_annotation, parent: parent_comment, body: "Last subcomment") visit comment_path(parent_comment) expect(page).to have_css(".comment", count: 3) - expect(page).to have_content parent_comment.body - expect(page).to have_content first_child.body - expect(page).to have_content second_child.body + expect(page).to have_content "Parent" + expect(page).to have_content "First subcomment" + expect(page).to have_content "Last subcomment" expect(page).to have_link "Go back to #{legislation_annotation.title}", href: href - expect(page).to have_selector("ul#comment_#{parent_comment.id}>li", count: 2) - expect(page).to have_selector("ul#comment_#{first_child.id}>li", count: 1) - expect(page).to have_selector("ul#comment_#{second_child.id}>li", count: 1) + within ".comment", text: "Parent" do + expect(page).to have_selector(".comment", count: 2) + end end scenario "Link to comment show" do diff --git a/spec/system/comments/legislation_questions_spec.rb b/spec/system/comments/legislation_questions_spec.rb index 09cc165dd..368c56190 100644 --- a/spec/system/comments/legislation_questions_spec.rb +++ b/spec/system/comments/legislation_questions_spec.rb @@ -25,23 +25,23 @@ describe "Commenting legislation questions" do end scenario "Show" do - parent_comment = create(:comment, commentable: legislation_question) - first_child = create(:comment, commentable: legislation_question, parent: parent_comment) - second_child = create(:comment, commentable: legislation_question, parent: parent_comment) href = legislation_process_question_path(legislation_question.process, legislation_question) + parent_comment = create(:comment, commentable: legislation_question, body: "Parent") + create(:comment, commentable: legislation_question, parent: parent_comment, body: "First subcomment") + create(:comment, commentable: legislation_question, parent: parent_comment, body: "Last subcomment") visit comment_path(parent_comment) expect(page).to have_css(".comment", count: 3) - expect(page).to have_content parent_comment.body - expect(page).to have_content first_child.body - expect(page).to have_content second_child.body + expect(page).to have_content "Parent" + expect(page).to have_content "First subcomment" + expect(page).to have_content "Last subcomment" expect(page).to have_link "Go back to #{legislation_question.title}", href: href - expect(page).to have_selector("ul#comment_#{parent_comment.id}>li", count: 2) - expect(page).to have_selector("ul#comment_#{first_child.id}>li", count: 1) - expect(page).to have_selector("ul#comment_#{second_child.id}>li", count: 1) + within ".comment", text: "Parent" do + expect(page).to have_selector(".comment", count: 2) + end end scenario "Link to comment show" do diff --git a/spec/system/comments/proposals_spec.rb b/spec/system/comments/proposals_spec.rb index 84aa7c219..919e59a06 100644 --- a/spec/system/comments/proposals_spec.rb +++ b/spec/system/comments/proposals_spec.rb @@ -20,21 +20,21 @@ describe "Commenting proposals" do end scenario "Show" do - parent_comment = create(:comment, commentable: proposal) - first_child = create(:comment, commentable: proposal, parent: parent_comment) - second_child = create(:comment, commentable: proposal, parent: parent_comment) + parent_comment = create(:comment, commentable: proposal, body: "Parent") + create(:comment, commentable: proposal, parent: parent_comment, body: "First subcomment") + create(:comment, commentable: proposal, parent: parent_comment, body: "Last subcomment") visit comment_path(parent_comment) expect(page).to have_css(".comment", count: 3) - expect(page).to have_content parent_comment.body - expect(page).to have_content first_child.body - expect(page).to have_content second_child.body + expect(page).to have_content "Parent" + expect(page).to have_content "First subcomment" + expect(page).to have_content "Last subcomment" expect(page).to have_link "Go back to #{proposal.title}", href: proposal_path(proposal) - expect(page).to have_selector("ul#comment_#{parent_comment.id}>li", count: 2) - expect(page).to have_selector("ul#comment_#{first_child.id}>li", count: 1) - expect(page).to have_selector("ul#comment_#{second_child.id}>li", count: 1) + within ".comment", text: "Parent" do + expect(page).to have_selector(".comment", count: 2) + end end scenario "Link to comment show" do From faefb529724b0214e3dfdbbad9c8592b8dcf4a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 10 May 2020 20:47:35 +0200 Subject: [PATCH 08/12] Use the same code to add comments and replies --- app/assets/javascripts/comments.js | 28 +++++++++++----------------- app/views/comments/create.js.erb | 13 +++++-------- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/comments.js b/app/assets/javascripts/comments.js index f2c5411ca..05229bce5 100644 --- a/app/assets/javascripts/comments.js +++ b/app/assets/javascripts/comments.js @@ -1,12 +1,8 @@ (function() { "use strict"; App.Comments = { - add_comment: function(parent_id, response_html) { - $(".comment-list:first").prepend($(response_html)); - this.update_comments_count(); - }, - add_reply: function(parent_id, response_html) { - $("#" + parent_id + " .comment-list:first").prepend($(response_html)); + add_comment: function(parent_selector, response_html) { + $(parent_selector + " .comment-list:first").prepend($(response_html)); this.update_comments_count(); }, update_comments_count: function() { @@ -21,17 +17,15 @@ display_error: function(field_with_errors, error_html) { $(error_html).insertAfter($("" + field_with_errors)); }, - reset_and_hide_form: function(id) { - var form_container, input; - form_container = $("#js-comment-form-" + id); - input = form_container.find("form textarea"); - input.val(""); - form_container.hide(); - }, - reset_form: function(id) { - var input; - input = $("#js-comment-form-" + id + " form textarea"); - input.val(""); + reset_form: function(parent_selector) { + var form_container; + + form_container = $(parent_selector + " .comment-form:first"); + form_container.find("textarea").val(""); + + if (parent_selector !== "") { + form_container.hide(); + } }, toggle_form: function(id) { $("#js-comment-form-" + id).toggle(); diff --git a/app/views/comments/create.js.erb b/app/views/comments/create.js.erb index ff8c068bf..1492140f0 100644 --- a/app/views/comments/create.js.erb +++ b/app/views/comments/create.js.erb @@ -1,11 +1,8 @@ -var comment_html = "
  • <%= j(render @comment) %>
    • " - <% if @comment.root? -%> - var commentable_id = "<%= dom_id(@commentable) %>"; - App.Comments.reset_form(commentable_id); - App.Comments.add_comment(commentable_id, comment_html); + var parent_id = ""; <% else -%> - var parent_id = "<%= "comment_#{@comment.parent_id}" %>"; - App.Comments.reset_and_hide_form(parent_id); - App.Comments.add_reply(parent_id, comment_html); + var parent_id = "#" + "<%= "comment_#{@comment.parent_id}" %>"; <% end -%> + +App.Comments.reset_form(parent_id); +App.Comments.add_comment(parent_id, "
  • <%= j(render @comment) %>
    • "); From 5d362ced1f1e4b16d1c78857fbd4dcc6eb5eb297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 11 May 2020 00:27:27 +0200 Subject: [PATCH 09/12] Fix duplicate HTML IDs in comment forms Since there were many on the page, the resulting HTML was invalid. --- app/views/comments/_form.html.erb | 12 ++++++------ spec/shared/system/notifiable_in_app.rb | 4 ++-- spec/support/common_actions/notifications.rb | 8 ++++++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/app/views/comments/_form.html.erb b/app/views/comments/_form.html.erb index 6b3c9d85f..39721877c 100644 --- a/app/views/comments/_form.html.erb +++ b/app/views/comments/_form.html.erb @@ -2,18 +2,18 @@ <% cache [locale_and_user_status, parent_id, commentable_cache_key(commentable), valuation] do %> <% css_id = parent_or_commentable_dom_id(parent_id, commentable) %>
    class="comment-form"> - <%= form_for Comment.new, remote: true do |f| %> + <%= form_for Comment.new, remote: true, html: { id: "new_comment_#{css_id}" } do |f| %> <%= f.text_area :body, id: "comment-body-#{css_id}", maxlength: Comment.body_max_length, label: leave_comment_text(commentable) %> - <%= f.hidden_field :commentable_type, value: commentable.class.name %> - <%= f.hidden_field :commentable_id, value: commentable.id %> - <%= f.hidden_field :parent_id, value: parent_id %> - <%= f.hidden_field :valuation, value: valuation %> + <%= f.hidden_field :commentable_type, value: commentable.class.name, id: "comment_commentable_type_#{css_id}" %> + <%= f.hidden_field :commentable_id, value: commentable.id, id: "comment_commentable_id_#{css_id}" %> + <%= f.hidden_field :parent_id, value: parent_id, id: "comment_parent_id_#{css_id}" %> + <%= f.hidden_field :valuation, value: valuation, id: "comment_valuation_#{css_id}" %> - <%= f.submit comment_button_text(parent_id, commentable), class: "button", id: "publish_comment" %> + <%= f.submit comment_button_text(parent_id, commentable), class: "button", id: "publish_comment_#{css_id}" %> <% if can? :comment_as_moderator, commentable %>
    diff --git a/spec/shared/system/notifiable_in_app.rb b/spec/shared/system/notifiable_in_app.rb index c141b98c4..4cbfdb394 100644 --- a/spec/shared/system/notifiable_in_app.rb +++ b/spec/shared/system/notifiable_in_app.rb @@ -30,7 +30,7 @@ shared_examples "notifiable in-app" do |factory_name| visit path_for(notifiable) fill_in comment_body(notifiable), with: "Number #{n + 1} is the best!" - click_button "publish_comment" + click_button submit_comment_text(notifiable) within "#comments" do expect(page).to have_content "Number #{n + 1} is the best!" end @@ -102,7 +102,7 @@ shared_examples "notifiable in-app" do |factory_name| visit path_for(notifiable) fill_in comment_body(notifiable), with: "I commented on my own notifiable" - click_button "publish_comment" + click_button submit_comment_text(notifiable) within "#comments" do expect(page).to have_content "I commented on my own notifiable" end diff --git a/spec/support/common_actions/notifications.rb b/spec/support/common_actions/notifications.rb index 9dbb38091..60f1673aa 100644 --- a/spec/support/common_actions/notifications.rb +++ b/spec/support/common_actions/notifications.rb @@ -13,6 +13,14 @@ module Notifications "comment-body-#{resource.class.name.parameterize(separator: "_").to_sym}_#{resource.id}" end + def submit_comment_text(resource) + if resource.class.name == "Legislation::Question" + "Publish answer" + else + "Publish comment" + end + end + def create_proposal_notification(proposal) login_as(proposal.author) visit root_path From 61a63ddd59847aaecafad7b8edcacb5ed5408313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 11 May 2020 00:51:20 +0200 Subject: [PATCH 10/12] Use label text instead of ID to fill in comments This way tests are easier to read (and easier to write). --- spec/shared/system/notifiable_in_app.rb | 6 ++--- spec/support/common_actions/comments.rb | 2 +- spec/support/common_actions/notifications.rb | 6 ++++- .../comments/budget_investments_spec.rb | 16 ++++++------- spec/system/comments/debates_spec.rb | 14 +++++------ .../comments/legislation_annotations_spec.rb | 18 +++++++------- .../comments/legislation_questions_spec.rb | 14 +++++------ spec/system/comments/polls_spec.rb | 12 +++++----- spec/system/comments/proposals_spec.rb | 12 +++++----- spec/system/comments/topics_spec.rb | 24 +++++++++---------- spec/system/emails_spec.rb | 2 +- 11 files changed, 65 insertions(+), 61 deletions(-) diff --git a/spec/shared/system/notifiable_in_app.rb b/spec/shared/system/notifiable_in_app.rb index 4cbfdb394..afddae9d3 100644 --- a/spec/shared/system/notifiable_in_app.rb +++ b/spec/shared/system/notifiable_in_app.rb @@ -53,7 +53,7 @@ shared_examples "notifiable in-app" do |factory_name| click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I replied to your comment" + fill_in comment_body(notifiable), with: "I replied to your comment" click_button "Publish reply" end @@ -79,7 +79,7 @@ shared_examples "notifiable in-app" do |factory_name| within("#comment_#{comment.id}_reply") { click_link "Reply" } within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Reply number #{n}" + fill_in comment_body(notifiable), with: "Reply number #{n}" click_button "Publish reply" end @@ -121,7 +121,7 @@ shared_examples "notifiable in-app" do |factory_name| click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I replied to my own comment" + fill_in comment_body(notifiable), with: "I replied to my own comment" click_button "Publish reply" end diff --git a/spec/support/common_actions/comments.rb b/spec/support/common_actions/comments.rb index 1c8a60b7c..a53c17cd9 100644 --- a/spec/support/common_actions/comments.rb +++ b/spec/support/common_actions/comments.rb @@ -17,7 +17,7 @@ module Comments click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end expect(page).to have_content "It will be done next week." diff --git a/spec/support/common_actions/notifications.rb b/spec/support/common_actions/notifications.rb index 60f1673aa..569010555 100644 --- a/spec/support/common_actions/notifications.rb +++ b/spec/support/common_actions/notifications.rb @@ -10,7 +10,11 @@ module Notifications end def comment_body(resource) - "comment-body-#{resource.class.name.parameterize(separator: "_").to_sym}_#{resource.id}" + if resource.class.name == "Legislation::Question" + "Leave your answer" + else + "Leave your comment" + end end def submit_comment_text(resource) diff --git a/spec/system/comments/budget_investments_spec.rb b/spec/system/comments/budget_investments_spec.rb index 3c922aee7..7364c4698 100644 --- a/spec/system/comments/budget_investments_spec.rb +++ b/spec/system/comments/budget_investments_spec.rb @@ -199,7 +199,7 @@ describe "Commenting Budget::Investments" do login_as(user) visit budget_investment_path(investment.budget, investment) - fill_in "comment-body-budget_investment_#{investment.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#tab-comments-label" do @@ -231,7 +231,7 @@ describe "Commenting Budget::Investments" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -334,7 +334,7 @@ describe "Commenting Budget::Investments" do login_as(moderator.user) visit budget_investment_path(investment.budget, investment) - fill_in "comment-body-budget_investment_#{investment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-budget_investment_#{investment.id}" click_button "Publish comment" @@ -358,7 +358,7 @@ describe "Commenting Budget::Investments" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -391,7 +391,7 @@ describe "Commenting Budget::Investments" do login_as(admin.user) visit budget_investment_path(investment.budget, investment) - fill_in "comment-body-budget_investment_#{investment.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-budget_investment_#{investment.id}" click_button "Publish comment" @@ -410,7 +410,7 @@ describe "Commenting Budget::Investments" do visit admin_budget_budget_investment_path(investment.budget, investment) - fill_in "comment-body-budget_investment_#{investment.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-budget_investment_#{investment.id}" click_button "Publish comment" @@ -434,7 +434,7 @@ describe "Commenting Budget::Investments" do login_as(admin.user) visit admin_budget_budget_investment_path(investment.budget, investment) - fill_in "comment-body-budget_investment_#{investment.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-budget_investment_#{investment.id}" click_button "Publish comment" @@ -458,7 +458,7 @@ describe "Commenting Budget::Investments" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb index b9943c10c..2b8bbc3e8 100644 --- a/spec/system/comments/debates_spec.rb +++ b/spec/system/comments/debates_spec.rb @@ -216,7 +216,7 @@ describe "Commenting debates" do login_as(user) visit debate_path(debate) - fill_in "comment-body-debate_#{debate.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#comments" do @@ -245,7 +245,7 @@ describe "Commenting debates" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -371,7 +371,7 @@ describe "Commenting debates" do login_as(user) visit debate_path(debate) - fill_in "comment-body-debate_#{debate.id}", with: "Testing submit button!" + fill_in "Leave your comment", with: "Testing submit button!" click_button "Publish comment" # The button"s text should now be "..." @@ -388,7 +388,7 @@ describe "Commenting debates" do login_as(moderator.user) visit debate_path(debate) - fill_in "comment-body-debate_#{debate.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-debate_#{debate.id}" click_button "Publish comment" @@ -412,7 +412,7 @@ describe "Commenting debates" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -444,7 +444,7 @@ describe "Commenting debates" do login_as(admin.user) visit debate_path(debate) - fill_in "comment-body-debate_#{debate.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-debate_#{debate.id}" click_button "Publish comment" @@ -468,7 +468,7 @@ describe "Commenting debates" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index 0ed8ef015..bb1788660 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -233,7 +233,7 @@ describe "Commenting legislation questions" do legislation_annotation.draft_version, legislation_annotation) - fill_in "comment-body-legislation_annotation_#{legislation_annotation.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#comments" do @@ -267,7 +267,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -384,7 +384,7 @@ describe "Commenting legislation questions" do legislation_annotation.draft_version, legislation_annotation) - fill_in "comment-body-legislation_annotation_#{legislation_annotation.id}", with: "Testing submit button!" + fill_in "Leave your comment", with: "Testing submit button!" click_button "Publish comment" # The button's text should now be "..." @@ -403,7 +403,7 @@ describe "Commenting legislation questions" do legislation_annotation.draft_version, legislation_annotation) - fill_in "comment-body-legislation_annotation_#{legislation_annotation.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-legislation_annotation_#{legislation_annotation.id}" click_button "Publish comment" @@ -430,7 +430,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -466,7 +466,7 @@ describe "Commenting legislation questions" do legislation_annotation.draft_version, legislation_annotation) - fill_in "comment-body-legislation_annotation_#{legislation_annotation.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-legislation_annotation_#{legislation_annotation.id}" click_button "Publish comment" @@ -493,7 +493,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end @@ -667,7 +667,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "replying in single annotation thread" + fill_in "Leave your comment", with: "replying in single annotation thread" click_button "Publish reply" end @@ -706,7 +706,7 @@ describe "Commenting legislation questions" do end within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "replying in multiple annotation thread" + fill_in "Leave your comment", with: "replying in multiple annotation thread" click_button "Publish reply" end diff --git a/spec/system/comments/legislation_questions_spec.rb b/spec/system/comments/legislation_questions_spec.rb index 368c56190..87167555f 100644 --- a/spec/system/comments/legislation_questions_spec.rb +++ b/spec/system/comments/legislation_questions_spec.rb @@ -202,7 +202,7 @@ describe "Commenting legislation questions" do login_as(user) visit legislation_process_question_path(legislation_question.process, legislation_question) - fill_in "comment-body-legislation_question_#{legislation_question.id}", with: "Have you thought about...?" + fill_in "Leave your answer", with: "Have you thought about...?" click_button "Publish answer" within "#comments" do @@ -249,7 +249,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your answer", with: "It will be done next week." click_button "Publish reply" end @@ -348,7 +348,7 @@ describe "Commenting legislation questions" do login_as(user) visit legislation_process_question_path(legislation_question.process, legislation_question) - fill_in "comment-body-legislation_question_#{legislation_question.id}", with: "Testing submit button!" + fill_in "Leave your answer", with: "Testing submit button!" click_button "Publish answer" # The button's text should now be "..." @@ -365,7 +365,7 @@ describe "Commenting legislation questions" do login_as(moderator.user) visit legislation_process_question_path(legislation_question.process, legislation_question) - fill_in "comment-body-legislation_question_#{legislation_question.id}", with: "I am moderating!" + fill_in "Leave your answer", with: "I am moderating!" check "comment-as-moderator-legislation_question_#{legislation_question.id}" click_button "Publish answer" @@ -389,7 +389,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your answer", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -421,7 +421,7 @@ describe "Commenting legislation questions" do login_as(admin.user) visit legislation_process_question_path(legislation_question.process, legislation_question) - fill_in "comment-body-legislation_question_#{legislation_question.id}", with: "I am your Admin!" + fill_in "Leave your answer", with: "I am your Admin!" check "comment-as-administrator-legislation_question_#{legislation_question.id}" click_button "Publish answer" @@ -445,7 +445,7 @@ describe "Commenting legislation questions" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your answer", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end diff --git a/spec/system/comments/polls_spec.rb b/spec/system/comments/polls_spec.rb index 65fd9af57..298e8cb1f 100644 --- a/spec/system/comments/polls_spec.rb +++ b/spec/system/comments/polls_spec.rb @@ -197,7 +197,7 @@ describe "Commenting polls" do login_as(user) visit poll_path(poll) - fill_in "comment-body-poll_#{poll.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#comments" do @@ -229,7 +229,7 @@ describe "Commenting polls" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -340,7 +340,7 @@ describe "Commenting polls" do login_as(moderator.user) visit poll_path(poll) - fill_in "comment-body-poll_#{poll.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-poll_#{poll.id}" click_button "Publish comment" @@ -366,7 +366,7 @@ describe "Commenting polls" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -402,7 +402,7 @@ describe "Commenting polls" do login_as(admin.user) visit poll_path(poll) - fill_in "comment-body-poll_#{poll.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-poll_#{poll.id}" click_button "Publish comment" @@ -428,7 +428,7 @@ describe "Commenting polls" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end diff --git a/spec/system/comments/proposals_spec.rb b/spec/system/comments/proposals_spec.rb index 919e59a06..d23aad2b4 100644 --- a/spec/system/comments/proposals_spec.rb +++ b/spec/system/comments/proposals_spec.rb @@ -195,7 +195,7 @@ describe "Commenting proposals" do login_as(user) visit proposal_path(proposal) - fill_in "comment-body-proposal_#{proposal.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#comments" do @@ -227,7 +227,7 @@ describe "Commenting proposals" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -330,7 +330,7 @@ describe "Commenting proposals" do login_as(moderator.user) visit proposal_path(proposal) - fill_in "comment-body-proposal_#{proposal.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-proposal_#{proposal.id}" click_button "Publish comment" @@ -354,7 +354,7 @@ describe "Commenting proposals" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -386,7 +386,7 @@ describe "Commenting proposals" do login_as(admin.user) visit proposal_path(proposal) - fill_in "comment-body-proposal_#{proposal.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-proposal_#{proposal.id}" click_button "Publish comment" @@ -410,7 +410,7 @@ describe "Commenting proposals" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end diff --git a/spec/system/comments/topics_spec.rb b/spec/system/comments/topics_spec.rb index 4d5d48d46..a00698047 100644 --- a/spec/system/comments/topics_spec.rb +++ b/spec/system/comments/topics_spec.rb @@ -216,7 +216,7 @@ describe "Commenting topics from proposals" do login_as(user) visit community_topic_path(community, topic) - fill_in "comment-body-topic_#{topic.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#comments" do @@ -253,7 +253,7 @@ describe "Commenting topics from proposals" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -369,7 +369,7 @@ describe "Commenting topics from proposals" do login_as(moderator.user) visit community_topic_path(community, topic) - fill_in "comment-body-topic_#{topic.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-topic_#{topic.id}" click_button "Publish comment" @@ -395,7 +395,7 @@ describe "Commenting topics from proposals" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -431,7 +431,7 @@ describe "Commenting topics from proposals" do login_as(admin.user) visit community_topic_path(community, topic) - fill_in "comment-body-topic_#{topic.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-topic_#{topic.id}" click_button "Publish comment" @@ -457,7 +457,7 @@ describe "Commenting topics from proposals" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end @@ -776,7 +776,7 @@ describe "Commenting topics from budget investments" do login_as(user) visit community_topic_path(community, topic) - fill_in "comment-body-topic_#{topic.id}", with: "Have you thought about...?" + fill_in "Leave your comment", with: "Have you thought about...?" click_button "Publish comment" within "#comments" do @@ -813,7 +813,7 @@ describe "Commenting topics from budget investments" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end @@ -929,7 +929,7 @@ describe "Commenting topics from budget investments" do login_as(moderator.user) visit community_topic_path(community, topic) - fill_in "comment-body-topic_#{topic.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-topic_#{topic.id}" click_button "Publish comment" @@ -955,7 +955,7 @@ describe "Commenting topics from budget investments" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "I am moderating!" + fill_in "Leave your comment", with: "I am moderating!" check "comment-as-moderator-comment_#{comment.id}" click_button "Publish reply" end @@ -991,7 +991,7 @@ describe "Commenting topics from budget investments" do login_as(admin.user) visit community_topic_path(community, topic) - fill_in "comment-body-topic_#{topic.id}", with: "I am your Admin!" + fill_in "Leave your comment", with: "I am your Admin!" check "comment-as-administrator-topic_#{topic.id}" click_button "Publish comment" @@ -1017,7 +1017,7 @@ describe "Commenting topics from budget investments" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "Top of the world!" + fill_in "Leave your comment", with: "Top of the world!" check "comment-as-administrator-comment_#{comment.id}" click_button "Publish reply" end diff --git a/spec/system/emails_spec.rb b/spec/system/emails_spec.rb index 7b23eea21..345cc33cc 100644 --- a/spec/system/emails_spec.rb +++ b/spec/system/emails_spec.rb @@ -443,7 +443,7 @@ describe "Emails" do click_link "Reply" within "#js-comment-form-comment_#{comment.id}" do - fill_in "comment-body-comment_#{comment.id}", with: "It will be done next week." + fill_in "Leave your comment", with: "It will be done next week." click_button "Publish reply" end From ae41becd3afc1cddbd0ffd13f06ae19fe04fa068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 11 May 2020 17:17:12 +0200 Subject: [PATCH 11/12] Use CSS to hide reply forms We were using inline styles and passing local variables around, while the rule we were following is very simple: it's only hidden if it's a form to reply to a comment. --- app/assets/stylesheets/layout.scss | 5 +++++ app/views/comments/_comment.html.erb | 1 - app/views/comments/_comment_tree.html.erb | 1 - app/views/comments/_form.html.erb | 2 +- app/views/debates/_comments.html.erb | 2 +- app/views/legislation/proposals/_comments.html.erb | 2 +- app/views/polls/_comments.html.erb | 2 +- app/views/proposals/_comments.html.erb | 2 +- app/views/topics/_comments.html.erb | 2 +- 9 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 977c6fe6b..3c195b1d8 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2180,6 +2180,11 @@ table { display: inline-block; padding-left: rem-calc(8); width: 100%; + + } + + .comment-form { + display: none; } &:empty { diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 443c69a01..382555b62 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -107,7 +107,6 @@ <% if !valuation || can?(:comment_valuation, comment.commentable) %> <%= render "comments/form", { commentable: comment.commentable, parent_id: comment.id, - toggeable: true, valuation: valuation } %> <% end %> <% end %> diff --git a/app/views/comments/_comment_tree.html.erb b/app/views/comments/_comment_tree.html.erb index 055f94e4d..c8014178c 100644 --- a/app/views/comments/_comment_tree.html.erb +++ b/app/views/comments/_comment_tree.html.erb @@ -27,7 +27,6 @@ <% elsif !valuation || can?(:comment_valuation, commentable) %> <%= render "comments/form", { commentable: commentable, parent_id: nil, - toggeable: false, valuation: valuation } %> <% end %> <% else %> diff --git a/app/views/comments/_form.html.erb b/app/views/comments/_form.html.erb index 39721877c..41b5e61eb 100644 --- a/app/views/comments/_form.html.erb +++ b/app/views/comments/_form.html.erb @@ -1,7 +1,7 @@ <% valuation = local_assigns.fetch(:valuation, false) %> <% cache [locale_and_user_status, parent_id, commentable_cache_key(commentable), valuation] do %> <% css_id = parent_or_commentable_dom_id(parent_id, commentable) %> -
    class="comment-form"> +
    <%= form_for Comment.new, remote: true, html: { id: "new_comment_#{css_id}" } do |f| %> <%= f.text_area :body, id: "comment-body-#{css_id}", diff --git a/app/views/debates/_comments.html.erb b/app/views/debates/_comments.html.erb index 46cf3fb57..9b0348930 100644 --- a/app/views/debates/_comments.html.erb +++ b/app/views/debates/_comments.html.erb @@ -9,7 +9,7 @@ <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> <% if user_signed_in? %> - <%= render "comments/form", { commentable: @debate, parent_id: nil, toggeable: false } %> + <%= render "comments/form", { commentable: @debate, parent_id: nil } %> <% else %>
    <%= render "shared/login_to_comment" %> diff --git a/app/views/legislation/proposals/_comments.html.erb b/app/views/legislation/proposals/_comments.html.erb index a7471102d..a8e657835 100644 --- a/app/views/legislation/proposals/_comments.html.erb +++ b/app/views/legislation/proposals/_comments.html.erb @@ -4,7 +4,7 @@ <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> <% if user_signed_in? %> - <%= render "comments/form", { commentable: @proposal, parent_id: nil, toggeable: false } %> + <%= render "comments/form", { commentable: @proposal, parent_id: nil } %> <% else %>
    <%= render "shared/login_to_comment" %> diff --git a/app/views/polls/_comments.html.erb b/app/views/polls/_comments.html.erb index d3309d79d..b99b9f351 100644 --- a/app/views/polls/_comments.html.erb +++ b/app/views/polls/_comments.html.erb @@ -4,7 +4,7 @@ <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> <% if user_signed_in? %> - <%= render "comments/form", { commentable: @poll, parent_id: nil, toggeable: false } %> + <%= render "comments/form", { commentable: @poll, parent_id: nil } %> <% else %>
    <%= render "shared/login_to_comment" %> diff --git a/app/views/proposals/_comments.html.erb b/app/views/proposals/_comments.html.erb index a7471102d..a8e657835 100644 --- a/app/views/proposals/_comments.html.erb +++ b/app/views/proposals/_comments.html.erb @@ -4,7 +4,7 @@ <%= render "shared/wide_order_selector", i18n_namespace: "comments" %> <% if user_signed_in? %> - <%= render "comments/form", { commentable: @proposal, parent_id: nil, toggeable: false } %> + <%= render "comments/form", { commentable: @proposal, parent_id: nil } %> <% else %>
    <%= render "shared/login_to_comment" %> diff --git a/app/views/topics/_comments.html.erb b/app/views/topics/_comments.html.erb index 485096378..abcaba601 100644 --- a/app/views/topics/_comments.html.erb +++ b/app/views/topics/_comments.html.erb @@ -8,7 +8,7 @@ <%= paginate @comment_tree.root_comments %> <% if user_signed_in? %> - <%= render "comments/form", { commentable: @topic, parent_id: nil, toggeable: false } %> + <%= render "comments/form", { commentable: @topic, parent_id: nil } %> <% else %> <%= render "shared/login_to_comment" %> <% end %> From f89bf0c52c139d4aca1c2820853aed7cf0772a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 11 May 2020 17:38:40 +0200 Subject: [PATCH 12/12] Bump initialjs-rails from 0.2.0.5 to 0.2.0.8 Version 0.2.0.5 was causing comments to have invalid HTML because the avatars had `` tags with an empty `src` attribute. --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 2c2c3e208..f3661c06c 100644 --- a/Gemfile +++ b/Gemfile @@ -25,7 +25,7 @@ gem "globalize-accessors", "~> 0.2.1" gem "graphiql-rails", "~> 1.4.1" gem "graphql", "~> 1.7.8" gem "groupdate", "~> 3.2.0" -gem "initialjs-rails", "~> 0.2.0.5" +gem "initialjs-rails", "~> 0.2.0.8" gem "invisible_captcha", "~> 0.10.0" gem "jquery-fileupload-rails" gem "jquery-rails", "~> 4.3.3" diff --git a/Gemfile.lock b/Gemfile.lock index 1d64d39ba..4494283b4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -282,7 +282,7 @@ GEM rainbow (>= 2.2.2, < 4.0) terminal-table (>= 1.5.1) ice_nine (0.11.2) - initialjs-rails (0.2.0.5) + initialjs-rails (0.2.0.8) railties (>= 3.1, < 6.0) invisible_captcha (0.10.0) rails (>= 3.2.0) @@ -642,7 +642,7 @@ DEPENDENCIES graphql (~> 1.7.8) groupdate (~> 3.2.0) i18n-tasks (~> 0.9.29) - initialjs-rails (~> 0.2.0.5) + initialjs-rails (~> 0.2.0.8) invisible_captcha (~> 0.10.0) jquery-fileupload-rails jquery-rails (~> 4.3.3)