consul/nairobi
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use a <ul> tag for a list of comments

Browse Source 
We were using a <ul> 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.
This commit is contained in:
Javi Martín
2020-05-10 20:14:15 +02:00
parent bedd879025
commit 4627372a62
16 changed files with 83 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
app/assets/javascripts/comments.js
View File

@@ -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("<li><ul id='" + parent_id + "_children' class='no-bullet comment-children'></ul></li>");
}
$("#" + 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;

20
app/assets/stylesheets/layout.scss
View File

@@ -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 {

23
app/views/comments/_comment.html.erb
View File

@@ -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 %>
<ul id="<%= dom_id(comment) %>" class="comment no-bullet small-12">
<li class="comment-body">
<div id="<%= dom_id(comment) %>" class="comment small-12">
<div class="comment-body">
<% if comment.hidden? || comment.user.hidden? %>
<% if comment.children.size > 0 %>
<div class="callout secondary">
@@ -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 %>
<span class="far fa-minus-square"></span>
<span class="js-child-toggle" style="display: none;"><%= t("comments.comment.responses_show", count: comment.children.size) %></span>
<span class="js-child-toggle"><%= t("comments.comment.responses_collapse", count: comment.children.size) %></span>
@@ -113,17 +113,8 @@
<% end %>
</div>
<% end %>
</li>
<% unless child_comments_of(comment).empty? %>
<li>
<ul id="<%= dom_id(comment) %>_children" class="no-bullet comment-children">
<% child_comments_of(comment).each do |child| %>
<li>
<%= render "comments/comment", { comment: child, valuation: valuation } %>
</li>
<% end %>
</ul>
</li>
<% end %>
</ul>
</div>
<%= render "comments/comment_list", comments: child_comments_of(comment), valuation: valuation %>
</div>
<% end %>

7
app/views/comments/_comment_list.html.erb Normal file
View File

@@ -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 %>

4
app/views/comments/_comment_tree.html.erb
View File

@@ -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 %>
</div>
</div>

2
app/views/comments/create.js.erb
View File

@@ -1,4 +1,4 @@
var comment_html = "<%= j(render @comment) %>"
var comment_html = "<li><%= j(render @comment) %></li>"
<% if @comment.root? -%>
var commentable_id = "<%= dom_id(@commentable) %>";

4
app/views/debates/_comments.html.erb
View File

@@ -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 %>
</div>
</div>

4
app/views/legislation/proposals/_comments.html.erb
View File

@@ -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 %>
</div>
</div>

4
app/views/polls/_comments.html.erb
View File

@@ -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 %>
</div>
</div>

4
app/views/proposals/_comments.html.erb
View File

@@ -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 %>
</div>
</div>

5
app/views/topics/_comments.html.erb
View File

@@ -4,10 +4,7 @@
<div id="comments" class="small-12 column">
<%= 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? %>

18
spec/system/comments/budget_investments_spec.rb
View File

@@ -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

18
spec/system/comments/debates_spec.rb
View File

@@ -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

18
spec/system/comments/legislation_annotations_spec.rb
View File

@@ -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

18
spec/system/comments/legislation_questions_spec.rb
View File

@@ -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

18
spec/system/comments/proposals_spec.rb
View File

@@ -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