Use buttons instead of links in the public area

[javierm]

User Story

As mentioned in commits 5311daa and bb958da, using links combined with JavaScript to generate POST (or, in this case, DELETE) requests to the server has a few issues. Quoting from the first of these commits:

First, screen readers will announce them as "links". People using screen readers usually associate links with "things that get you somewhere" and buttons with "things that perform an action". So when something like "Delete, link" is announced, they'll probably think this is a link which will take them to another page where they can delete a record.

Furthermore, the URL of the link for the "destroy" action might be the same as the URL for the "show" action (only one is accessed with a DELETE request and the other one with a GET request). That means screen readers could announce the link like "Delete, visited link", which is very confusing.

These links won't work when opening links in a new tab, since opening links in a new tab always results in a GET request to the URL the link points to.

Finally, submit buttons work without JavaScript enabled, so they'll work even if the JavaScript in the page hasn't loaded (for whatever reason).

Implementation details

See pull request #5459 for an example, since this pull request does the exact same thing for links in the admin section. Don't use the Admin::ActionComponent class, though; either replace link_to with button_to or create a new Shared::ActionComponent class.

Here's a patch adding a # TODO element to the places where we detected this is needed. Note the patch was based on version 2.1.1, so it's possible that it doesn't apply cleanly if you're using a more recent version of Consul Democracy.

diff --git a/app/components/budgets/ballot/investment_component.html.erb b/app/components/budgets/ballot/investment_component.html.erb
index 58e62f4ea9..8e0d5af93c 100644
--- a/app/components/budgets/ballot/investment_component.html.erb
+++ b/app/components/budgets/ballot/investment_component.html.erb
@@ -3,6 +3,7 @@
   <%= investment_price %>
 
   <% if budget.balloting? %>
+    <%# TODO %>
     <%= link_to delete_path,
                 title: t("budgets.ballots.show.remove"),
                 class: "remove-budget-investment",
diff --git a/app/components/documents/document_component.html.erb b/app/components/documents/document_component.html.erb
index 1de9100811..1a73c49212 100644
--- a/app/components/documents/document_component.html.erb
+++ b/app/components/documents/document_component.html.erb
@@ -8,6 +8,7 @@
   <% end %>
 
   <% if show_destroy_link? && can?(:destroy, document) %>
+    <%# TODO %>
     <%= link_to t("documents.buttons.destroy_document"),
                 document,
                 method: :delete,
diff --git a/app/components/layout/login_items_component.html.erb b/app/components/layout/login_items_component.html.erb
index 048ef300dc..7c70124755 100644
--- a/app/components/layout/login_items_component.html.erb
+++ b/app/components/layout/login_items_component.html.erb
@@ -16,6 +16,7 @@
                                    t("layouts.header.my_account_link") %>
   </li>
   <li>
+    <%# TODO %>
     <%= link_to t("devise_views.menu.login_items.logout"),
                 destroy_user_session_path, rel: "nofollow", method: :delete %>
   </li>
diff --git a/app/helpers/proposals_helper.rb b/app/helpers/proposals_helper.rb
index f2b82e8dcd..f3968922fb 100644
--- a/app/helpers/proposals_helper.rb
+++ b/app/helpers/proposals_helper.rb
@@ -80,6 +80,7 @@ module ProposalsHelper
       path = toggle_selection_admin_legislation_process_proposal_path(proposal.process, proposal)
     end
 
+    # TODO
     link_to button_text, path, remote: true, method: :patch, class: html_class
   end
 
diff --git a/app/views/admin/budget_investments/_select_investment.html.erb b/app/views/admin/budget_investments/_select_investment.html.erb
index 9753b35f9f..ac712e0c37 100644
--- a/app/views/admin/budget_investments/_select_investment.html.erb
+++ b/app/views/admin/budget_investments/_select_investment.html.erb
@@ -67,6 +67,7 @@
 
 <td id="selection" class="small text-center" data-field="selected">
   <% if investment.selected? %>
+      <%# TODO %>
       <%= link_to_if can?(:toggle_selection, investment),
                      t("admin.budget_investments.index.selected"),
                      toggle_selection_admin_budget_budget_investment_path(
@@ -84,6 +85,7 @@
                      class: "button small expanded" %>
   <% elsif investment.feasible? && investment.valuation_finished? %>
     <% if can?(:toggle_selection, investment) %>
+      <%# TODO %>
       <%= link_to t("admin.budget_investments.index.select"),
                   toggle_selection_admin_budget_budget_investment_path(
                     @budget,
diff --git a/app/views/budgets/investments/_author_actions.html.erb b/app/views/budgets/investments/_author_actions.html.erb
index 92a4aa8638..a6536b693b 100644
--- a/app/views/budgets/investments/_author_actions.html.erb
+++ b/app/views/budgets/investments/_author_actions.html.erb
@@ -7,6 +7,7 @@
                   edit_budget_investment_path(investment.budget, investment),
                   method: :get, class: "button hollow expanded" %>
     <% else %>
+        <%# TODO %>
         <%= link_to image_path(investment.image),
                     method: :delete,
                     class: "button hollow alert expanded" do %>
diff --git a/app/views/comments/_actions.html.erb b/app/views/comments/_actions.html.erb
index 7780d91672..a38918c51a 100644
--- a/app/views/comments/_actions.html.erb
+++ b/app/views/comments/_actions.html.erb
@@ -5,6 +5,7 @@
 <% if can?(:hide, comment) || can?(:hide, comment.user) %>
   <% if comment.author == current_user %>
     <span class="divider">&nbsp;&bull;&nbsp;</span>
+    <%# TODO %>
     <%= link_to t("comments.actions.delete"),
                 hide_comment_path(comment),
                 method: :put, remote: true, class: "delete-comment",
diff --git a/app/views/dashboard/_proposed_action.html.erb b/app/views/dashboard/_proposed_action.html.erb
index 082ddf0edc..aa3591cab1 100644
--- a/app/views/dashboard/_proposed_action.html.erb
+++ b/app/views/dashboard/_proposed_action.html.erb
@@ -1,6 +1,7 @@
 <div id="<%= dom_id(proposed_action) %>">
   <div class="action">
     <% if proposed_action.proposals.where(id: proposal.id).any? %>
+      <%# TODO %>
       <%= link_to unexecute_proposal_dashboard_action_path(proposal, proposed_action),
                   id: "#{dom_id(proposed_action)}_unexecute",
                   method: :post,
@@ -8,6 +9,7 @@
         <span class="icon-check"></span>
       <% end %>
     <% else %>
+      <%# TODO %>
       <%= link_to execute_proposal_dashboard_action_path(proposal, proposed_action),
                   id: "#{dom_id(proposed_action)}_execute",
                   method: :post,
diff --git a/app/views/dashboard/mailing/_mailing_options.html.erb b/app/views/dashboard/mailing/_mailing_options.html.erb
index fce44db6d9..8f04ba48d5 100644
--- a/app/views/dashboard/mailing/_mailing_options.html.erb
+++ b/app/views/dashboard/mailing/_mailing_options.html.erb
@@ -5,6 +5,7 @@
                 class: "button expanded" %>
   <% end %>
 
+  <%# TODO %>
   <%= link_to t("dashboard.mailing.mailing_options.send", address: current_user.email),
               proposal_dashboard_mailing_index_path(proposal),
               method: :post,
diff --git a/app/views/dashboard/polls/_poll.html.erb b/app/views/dashboard/polls/_poll.html.erb
index 26fe74c281..1025a55ddb 100644
--- a/app/views/dashboard/polls/_poll.html.erb
+++ b/app/views/dashboard/polls/_poll.html.erb
@@ -27,6 +27,7 @@
     <% end %>
     <p class="help-text"><%= t("dashboard.polls.poll.show_results_help") %></p>
 
+    <%# TODO %>
     <%= link_to t("dashboard.polls.poll.delete"),
                 proposal_dashboard_poll_path(proposal, poll),
                 method: :delete,
diff --git a/app/views/dashboard/show.html.erb b/app/views/dashboard/show.html.erb
index f698f1ca15..3a32a0c36d 100644
--- a/app/views/dashboard/show.html.erb
+++ b/app/views/dashboard/show.html.erb
@@ -13,6 +13,7 @@
 <% end %>
 
 <% if can?(:publish, proposal) %>
+  <%# TODO %>
   <%= link_to t("dashboard.index.publish"),
               publish_proposal_dashboard_path(proposal),
               class: "button success",
diff --git a/app/views/debates/_actions.html.erb b/app/views/debates/_actions.html.erb
index d3e64148b1..fb9dfbe496 100644
--- a/app/views/debates/_actions.html.erb
+++ b/app/views/debates/_actions.html.erb
@@ -3,12 +3,14 @@
 <% if can? :mark_featured, debate %>
   &nbsp;|&nbsp;
   <% if debate.featured? %>
+    <%# TODO %>
     <%= link_to t("admin.actions.unmark_featured").capitalize, unmark_featured_debate_path(debate),
                 method: :put,
                 data: { confirm: t("admin.actions.confirm_action",
                                    action: t("admin.actions.unmark_featured"),
                                    name: debate.title) } %>
   <% else %>
+    <%# TODO %>
     <%= link_to t("admin.actions.mark_featured").capitalize, mark_featured_debate_path(debate),
                 method: :put,
                 data: { confirm: t("admin.actions.confirm_action",
diff --git a/app/views/devise/_omniauth_form.html.erb b/app/views/devise/_omniauth_form.html.erb
index 8cbe25844d..f734a431e8 100644
--- a/app/views/devise/_omniauth_form.html.erb
+++ b/app/views/devise/_omniauth_form.html.erb
@@ -8,6 +8,7 @@
 
     <% oauth_logins.each do |login| %>
       <div class="small-12 medium-6 large-4 column end">
+        <%# TODO %>
         <%= link_to t("omniauth.#{login}.name"), send("user_#{login}_omniauth_authorize_path"),
                     title: t("omniauth.#{login}.#{action}"),
                     class: "button-#{login.to_s.delete_suffix("_oauth2")} button expanded",
diff --git a/app/views/devise/shared/_links.html.erb b/app/views/devise/shared/_links.html.erb
index dadc44e4db..707e8705fb 100644
--- a/app/views/devise/shared/_links.html.erb
+++ b/app/views/devise/shared/_links.html.erb
@@ -27,6 +27,7 @@
 
   <%- if devise_mapping.omniauthable? && devise_mapping.name == "user" %>
     <%- resource_class.omniauth_providers.each do |provider| %>
+      <%# TODO %>
       <%= link_to t("devise_views.shared.links.signin_with_provider", provider: provider.to_s.titleize), omniauth_authorize_path(resource_name, provider), method: :post %><br>
     <% end -%>
   <% end -%>
diff --git a/app/views/follows/_follow_button.html.erb b/app/views/follows/_follow_button.html.erb
index 3b13058675..6c90ddc9f1 100644
--- a/app/views/follows/_follow_button.html.erb
+++ b/app/views/follows/_follow_button.html.erb
@@ -1,6 +1,7 @@
 <div class="js-follow">
   <span class="followable-content">
     <% if follow.followable.followed_by?(current_user) %>
+      <%# TODO %>
       <%= link_to t("shared.following"),
                   follow_path(follow),
                   method: :delete, remote: true,
@@ -8,6 +9,7 @@
                   class: "button expanded" %>
 
     <% else %>
+      <%# TODO %>
       <%= link_to follow_text(follow.followable),
                   follows_path(followable_id: follow.followable.id,
                                followable_type: follow.followable.class.name),
diff --git a/app/views/management/_account_info.html.erb b/app/views/management/_account_info.html.erb
index ab37d813b1..f978c4f3ee 100644
--- a/app/views/management/_account_info.html.erb
+++ b/app/views/management/_account_info.html.erb
@@ -1,5 +1,6 @@
 <% if managed_user.document_number.present? %>
   <section class="account-info">
+    <%# TODO %>
     <%= link_to(t("management.account_info.change_user"),
                 logout_management_users_path,
                 method: :delete,
diff --git a/app/views/management/users/_erase_user_account.html.erb b/app/views/management/users/_erase_user_account.html.erb
index 5746e103a2..f85c496f71 100644
--- a/app/views/management/users/_erase_user_account.html.erb
+++ b/app/views/management/users/_erase_user_account.html.erb
@@ -5,5 +5,6 @@
     <%= t("management.users.erase_warning") %>
   </div>
 
+  <%# TODO %>
   <%= link_to t("management.users.erase_submit"), erase_management_users_path, method: :delete, class: "button hollow alert", data: { confirm: t("management.users.erase_account_confirm") } %>
 </div>
diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb
index b19be5432a..250642ddad 100644
--- a/app/views/notifications/_notification.html.erb
+++ b/app/views/notifications/_notification.html.erb
@@ -15,12 +15,14 @@
   <% end %>
 
   <% if notification.unread? %>
+    <%# TODO %>
     <%= link_to t("notifications.notification.mark_as_read"),
                 mark_as_read_notification_path(notification),
                 method: :put,
                 remote: true,
                 class: "mark-notification small" %>
   <% else %>
+    <%# TODO %>
     <%= link_to t("notifications.notification.mark_as_unread"),
                 mark_as_unread_notification_path(notification),
                 method: :put,
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb
index 95ef36e03e..881b95288b 100644
--- a/app/views/notifications/index.html.erb
+++ b/app/views/notifications/index.html.erb
@@ -5,6 +5,7 @@
       <%= t("notifications.index.title") %>
     </h1>
 
+    <%# TODO %>
     <%= link_to t("notifications.index.mark_all_as_read"),
                 mark_all_as_read_notifications_path,
                 method: :put,
diff --git a/app/views/proposals/created.html.erb b/app/views/proposals/created.html.erb
index 1c9ed9faea..ae9d0f6e4d 100644
--- a/app/views/proposals/created.html.erb
+++ b/app/views/proposals/created.html.erb
@@ -13,6 +13,7 @@
       <% end %>
 
       <% if can?(:publish, @proposal) %>
+        <%# TODO %>
         <%= link_to t("proposals.created.publish"),
                     publish_proposal_path(@proposal),
                     method: :patch, class: "button" %>
diff --git a/app/views/relationable/_score.html.erb b/app/views/relationable/_score.html.erb
index ef66c7b4ef..0c274ec2e0 100644
--- a/app/views/relationable/_score.html.erb
+++ b/app/views/relationable/_score.html.erb
@@ -1,12 +1,14 @@
 <small><%= t("related_content.is_related") %></small>
 
 <span class="relate-content-score">
+  <%# TODO %>
   <%= link_to t("related_content.score_positive"),
               score_positive_related_content_path(related),
               method: :put,
               remote: true,
               class: "score-positive" %>
 
+  <%# TODO %>
   <%= link_to t("related_content.score_negative"),
               score_negative_related_content_path(related),
               method: :put,
diff --git a/app/views/shared/_flag_actions.html.erb b/app/views/shared/_flag_actions.html.erb
index b2b2e4566d..8418ac32e3 100644
--- a/app/views/shared/_flag_actions.html.erb
+++ b/app/views/shared/_flag_actions.html.erb
@@ -8,6 +8,7 @@
       <span class="icon-flag flag-disable"></span>
     </button>
     <span class="dropdown-pane" id="flag-drop-<%= dom_id(flaggable) %>" data-dropdown data-auto-focus="true">
+      <%# TODO %>
       <%= link_to t("shared.flag"), polymorphic_path(flaggable, action: :flag), method: :put, remote: true %>
     </span>
   <% end %>
@@ -21,6 +22,7 @@
       <span class="icon-flag flag-active"></span>
     </button>
     <span class="dropdown-pane" id="unflag-drop-<%= dom_id(flaggable) %>" data-dropdown data-auto-focus="true">
+      <%# TODO %>
       <%= link_to t("shared.unflag"), polymorphic_path(flaggable, action: :unflag), method: :put, remote: true %>
     </span>
   <% end %>
diff --git a/app/views/shared/_recommended_index.html.erb b/app/views/shared/_recommended_index.html.erb
index 4447a9ae4c..4824281436 100644
--- a/app/views/shared/_recommended_index.html.erb
+++ b/app/views/shared/_recommended_index.html.erb
@@ -5,6 +5,7 @@
     </div>
 
     <div id="recommendations" data-toggler=".hide">
+      <%# TODO %>
       <%= link_to disable_recommendations_path, title: t("shared.recommended_index.hide"),
                                                 class: "float-right-medium small hide-recommendations",
                                                 data: {
diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb
index 767d94efa0..c5567dae2b 100644
--- a/app/views/topics/show.html.erb
+++ b/app/views/topics/show.html.erb
@@ -32,6 +32,7 @@
                     edit_community_topic_path(@community.id, @topic),
                     class: "button hollow expanded" %>
 
+        <%# TODO %>
         <%= link_to t("community.show.topic.destroy"),
                     community_topic_path(@community.id, @topic),
                     method: :delete,
diff --git a/app/views/users/_budget_investment.html.erb b/app/views/users/_budget_investment.html.erb
index 7c9dc0fecd..c8c431f353 100644
--- a/app/views/users/_budget_investment.html.erb
+++ b/app/views/users/_budget_investment.html.erb
@@ -8,6 +8,7 @@
                   class: "button hollow" %>
     <% end %>
     <% if can? :destroy, budget_investment %>
+      <%# TODO %>
       <%= link_to t("shared.delete"), budget_investment_path(budget_investment.budget, budget_investment),
                   method: :delete, class: "button hollow alert",
                   data: { confirm: t("users.show.delete_alert") } %>
diff --git a/app/views/users/registrations/finish_signup.html.erb b/app/views/users/registrations/finish_signup.html.erb
index 11bcca2f8f..038ca81de9 100644
--- a/app/views/users/registrations/finish_signup.html.erb
+++ b/app/views/users/registrations/finish_signup.html.erb
@@ -21,6 +21,7 @@
 
   <%= f.submit t("devise_views.users.registrations.new.submit"), class: "button expanded" %>
   <div class="text-center">
+    <%# TODO %>
     <%= link_to t("devise_views.users.registrations.new.cancel"), destroy_user_session_path, class: "delete", method: :delete %>
   </div>
 <% end %>
diff --git a/app/views/verification/letter/new.html.erb b/app/views/verification/letter/new.html.erb
index 60b38f9ae4..d9d5515c6d 100644
--- a/app/views/verification/letter/new.html.erb
+++ b/app/views/verification/letter/new.html.erb
@@ -42,6 +42,7 @@
           <%= t("verification.letter.new.office") %>
         </li>
         <li>
+          <%# TODO %>
           <%= link_to t("verification.letter.new.send_letter"), letter_path, method: :post %>
         </li>
       </ul>