Use order links to sort comments and topics

We use order links in many places in the web. However, in the comments
section and the list of community topics, we were displaying a
`<select>` element, and changing the location when users select an
option.

This has several disadvantages.

First, and most important, it's terrible for keyboard users. `<select>`
fields allow using the arrow keys to navigate through their options, and
typing a letter will select the first option starting with that letter.
This will trigger the "change" event and so users will navigate through
a new page while they were probably just checking the available options
[1]. For these reasons, WCAG Success Criterion 3.2.2 [2] states:

> Changing the setting of any user interface component does not
> automatically cause a change of context unless the user has been
> advised of the behavior before using the component.

Second, the form didn't have a submit button. This might confuse screen
reader users, who might not know how that form is supposed to be
submitted.

Finally, dropdowns have usability issues of their own [3], particularly
on mobile phones [4]

The easiest solution is to use the same links we generally use to allow
users select an order, so using them here we make the user experience
more consistent. They offer one disadvantage, though; on small screens
and certain languages, these links might take too much space and not
look that great. This issue affects pretty much every place where we use
order or filter links, so we might revisit it in the future.

Note we're moving the links to order comments after the form to add a
new comment. In my opinion, having an element such as a form to add a
new comment between the element to select the desired order of the
comments and the comments themselves is a bit confusing.

[1] https://webaim.org/techniques/forms/controls#javascript
[2] https://www.w3.org/WAI/WCAG21/Understanding/on-input.html
[3] https://www.youtube.com/watch?v=CUkMCQR4TpY
[4] https://www.lukew.com/ff/entry.asp?1950

app/assets/stylesheets/community.scss

@@ -4,10 +4,6 @@
     pointer-events: none;
   }
 
-  .wide-order-selector {
-    margin-top: 0;
-  }
-
   .panel {
     min-height: auto;
     margin: 0.375rem 0;

app/assets/stylesheets/layout.scss

@@ -1856,6 +1856,11 @@ table {
     }
   }
 
+  .order-links {
+    margin-bottom: $line-height;
+    margin-top: $line-height;
+  }
+
   .comment {
     line-height: $list-lineheight;
     margin: $line-height / 4 0;

app/assets/stylesheets/participation.scss

@@ -393,6 +393,11 @@
       font-size: rem-calc(15);
       margin-bottom: rem-calc(15);
     }
+
+    .order-links > li {
+      font-size: inherit;
+      margin-bottom: 0;
+    }
   }
 
   .geozone {

app/components/shared/comments_component.html.erb

@@ -3,15 +3,13 @@
     <div id="comments" class="small-12 column">
       <%= content %>
 
-      <%= render "shared/wide_order_selector", i18n_namespace: "comments" %>
-
       <% if current_user %>
         <%= render "comments/form", { commentable: record, parent_id: nil } %>
       <% else %>
-        <br>
         <%= render "shared/login_to_comment" %>
       <% end %>
 
+      <%= render "shared/order_links", i18n_namespace: "comments" %>
       <%= render "comments/comment_list", comments: comment_tree.root_comments %>
       <%= paginate comment_tree.root_comments %>
     </div>

app/views/comments/_comment_tree.html.erb

@@ -11,8 +11,6 @@
           </h2>
         <% end %>
 
-        <%= render "shared/wide_order_selector", i18n_namespace: "comments" %>
-
         <% if user_signed_in? %>
           <% if comments_closed_for_commentable?(commentable) %>
             <br>
@@ -30,10 +28,10 @@
                                           valuation: valuation } %>
           <% end %>
         <% else %>
-          <br>
           <%= render "shared/login_to_comment" %>
         <% end %>
 
+        <%= render "shared/order_links", i18n_namespace: "comments" %>
         <%= render "comments/comment_list", comments: comment_tree.root_comments, valuation: valuation %>
         <%= paginate comment_tree.root_comments %>
       </div>

app/views/shared/_order_links.html.erb

@@ -1,4 +1,4 @@
-<ul class="no-bullet submenu">
+<ul class="no-bullet submenu order-links">
   <% @valid_orders.each do |order| %>
     <li class="inline-block">
       <%= link_to current_path_with_query_params(order: order, page: 1), class: order == @current_order ? "is-active" : "" do %>

app/views/topics/_topics.html.erb

@@ -1,7 +1,7 @@
 <% if topics.any? %>
   <div class="row column">
     <div class="order">
-      <%= render "shared/wide_order_selector", i18n_namespace: "comments" %>
+      <%= render "shared/order_links", i18n_namespace: "comments" %>
     </div>
   </div>
 <% else %>

config/locales/en/general.yml

@@ -61,7 +61,6 @@ en:
       newest: Newest first
       oldest: Oldest first
       most_commented: Most commented
-    select_order: Sort by
     show:
       return_to_commentable: "Go back to "
   comments_helper:

config/locales/es/general.yml

@@ -61,7 +61,6 @@ es:
       newest: Más nuevos primero
       oldest: Más antiguos primero
       most_commented: Más comentados
-    select_order: Ordenar por
     show:
       return_to_commentable: "Volver a "
   comments_helper:

spec/system/comments/budget_investments_spec.rb

@@ -108,13 +108,15 @@ describe "Commenting Budget::Investments" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit budget_investment_path(investment.budget, investment, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit budget_investment_path(investment.budget, investment, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/comments/debates_spec.rb

@@ -124,13 +124,15 @@ describe "Commenting debates" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit debate_path(debate, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit debate_path(debate, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/comments/legislation_annotations_spec.rb

@@ -116,19 +116,15 @@ describe "Commenting legislation questions" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit legislation_process_draft_version_annotation_path(legislation_annotation.draft_version.process,
+    click_link "Newest first"
-                                                            legislation_annotation.draft_version,
-                                                            legislation_annotation,
-                                                            order: :newest)
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit legislation_process_draft_version_annotation_path(legislation_annotation.draft_version.process,
+    click_link "Oldest first"
-                                                            legislation_annotation.draft_version,
-                                                            legislation_annotation,
-                                                            order: :oldest)
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/comments/legislation_questions_spec.rb

@@ -110,13 +110,15 @@ describe "Commenting legislation questions" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit legislation_process_question_path(legislation_question.process, legislation_question, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit legislation_process_question_path(legislation_question.process, legislation_question, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/comments/polls_spec.rb

@@ -104,13 +104,15 @@ describe "Commenting polls" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit poll_path(poll, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit poll_path(poll, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/comments/proposals_spec.rb

@@ -104,13 +104,15 @@ describe "Commenting proposals" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit proposal_path(proposal, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit proposal_path(proposal, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/comments/topics_spec.rb

@@ -111,13 +111,15 @@ describe "Commenting topics from proposals" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit community_topic_path(community, topic, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit community_topic_path(community, topic, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end
@@ -653,13 +655,15 @@ describe "Commenting topics from budget investments" do
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
 
-    visit community_topic_path(community, topic, order: :newest)
+    click_link "Newest first"
 
+    expect(page).to have_link "Newest first", class: "is-active"
     expect(c3.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c1.body)
 
-    visit community_topic_path(community, topic, order: :oldest)
+    click_link "Oldest first"
 
+    expect(page).to have_link "Oldest first", class: "is-active"
     expect(c1.body).to appear_before(c2.body)
     expect(c2.body).to appear_before(c3.body)
   end

spec/system/communities_spec.rb

@@ -26,7 +26,7 @@ describe "Communities" do
       expect(page).to have_content "Participants (1)"
     end
 
-    scenario "Should display order selector and topic content when there are topics" do
+    scenario "Should display order links and topic content when there are topics" do
       proposal = create(:proposal)
       community = proposal.community
       topic = create(:topic, community: community)
@@ -34,7 +34,10 @@ describe "Communities" do
 
       visit community_path(community)
 
-      expect(page).to have_selector ".wide-order-selector"
+      expect(page).to have_link "Newest first", class: "is-active"
+      expect(page).to have_link "Most commented"
+      expect(page).to have_link "Oldest first"
+
       within "#topic_#{topic.id}" do
         expect(page).to have_content topic.title
         expect(page).to have_content "#{topic.comments_count} comment"