From da11c0d7bab9fb54c8b5cee8a9e1d0ed13b41ca9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 12 Jun 2021 14:34:27 +0200
Subject: [PATCH] Fix empty support link for screen readers users

When identified users accessed the investments page, we were using the
`aria-hidden` attribute to hide this link from screen readers since
unidentified users can't support investments.

However, the link was still focusable using keyboard navigation. This
resulted in screen reader users reaching the link and being able to
click it, but getting no feedback at all about where they were or what
they were clicking on.

This is why the fourth ARIA rule says: "Do not use role="presentation" or
aria-hidden="true" on a focusable element" [1].

So we're replacing the link with a non-interactive element instead, like
we do in other places like proposals.

The accessibility of this part of the page for unidentified users still
has some issues; here we're only improving it up to a certain point.

[1] https://www.w3.org/TR/using-aria/#4thrule
---
 .../investments/votes_component.html.erb      | 45 +++++++++++--------
 .../budgets/investments/votes_component.rb    |  4 --
 .../investments/votes_component_spec.rb       | 31 +++++++++++++
 3 files changed, 57 insertions(+), 23 deletions(-)
 create mode 100644 spec/components/budgets/investments/votes_component_spec.rb

diff --git a/app/components/budgets/investments/votes_component.html.erb b/app/components/budgets/investments/votes_component.html.erb
index 8b84af637..70ffbfe96 100644
--- a/app/components/budgets/investments/votes_component.html.erb
+++ b/app/components/budgets/investments/votes_component.html.erb
@@ -10,29 +10,36 @@
         <%= t("budgets.investments.investment.already_supported") %>
       </div>
     <% elsif investment.should_show_votes? %>
-      <%= link_to t("budgets.investments.investment.give_support"), vote_url,
-          class: "button button-support small expanded",
-          title: t("budgets.investments.investment.support_title"),
-          method: "post",
-          remote: !display_support_alert?,
-          data: ({ confirm: confirm_vote_message } if display_support_alert?),
-          "aria-hidden" => css_for_aria_hidden %>
+      <% if current_user %>
+        <%= link_to t("budgets.investments.investment.give_support"), vote_url,
+            class: "button button-support small expanded",
+            title: t("budgets.investments.investment.support_title"),
+            method: "post",
+            remote: !display_support_alert?,
+            data: ({ confirm: confirm_vote_message } if display_support_alert?) %>
+      <% else %>
+        <div class="button button-support small expanded">
+          <%= t("budgets.investments.investment.give_support") %>
+        </div>
+      <% end %>
     <% end %>
   </div>
 
   <% if reason.present? && !user_voted_for? %>
-    <div class="js-participation-not-allowed participation-not-allowed" style="display:none" aria-hidden="false">
-      <p>
-        <small>
-          <%= sanitize(t("votes.budget_investments.#{reason}",
-                count: investment.group.max_votable_headings,
-                verify_account: link_to_verify_account,
-                signin: link_to_signin,
-                signup: link_to_signup,
-                supported_headings: (current_user && current_user.headings_voted_within_group(investment.group).map(&:name).sort.to_sentence)
-               )) %>
-        </small>
-      </p>
+    <div tabindex="0">
+      <div class="js-participation-not-allowed participation-not-allowed" style="display:none" aria-hidden="false">
+        <p>
+          <small>
+            <%= sanitize(t("votes.budget_investments.#{reason}",
+                  count: investment.group.max_votable_headings,
+                  verify_account: link_to_verify_account,
+                  signin: link_to_signin,
+                  signup: link_to_signup,
+                  supported_headings: (current_user && current_user.headings_voted_within_group(investment.group).map(&:name).sort.to_sentence)
+                 )) %>
+          </small>
+        </p>
+      </div>
     </div>
   <% end %>
 
diff --git a/app/components/budgets/investments/votes_component.rb b/app/components/budgets/investments/votes_component.rb
index 319cef81e..1b3440d6c 100644
--- a/app/components/budgets/investments/votes_component.rb
+++ b/app/components/budgets/investments/votes_component.rb
@@ -32,8 +32,4 @@ class Budgets::Investments::VotesComponent < ApplicationComponent
     def confirm_vote_message
       t("budgets.investments.investment.confirm_group", count: investment.group.max_votable_headings)
     end
-
-    def css_for_aria_hidden
-      reason.present? ? "true" : ""
-    end
 end
diff --git a/spec/components/budgets/investments/votes_component_spec.rb b/spec/components/budgets/investments/votes_component_spec.rb
new file mode 100644
index 000000000..b352fa56f
--- /dev/null
+++ b/spec/components/budgets/investments/votes_component_spec.rb
@@ -0,0 +1,31 @@
+require "rails_helper"
+
+describe Budgets::Investments::VotesComponent, type: :component do
+  describe "vote link" do
+    context "when investment shows votes" do
+      let(:investment) { create(:budget_investment) }
+      let(:component) do
+        Budgets::Investments::VotesComponent.new(investment, investment_votes: [], vote_url: "/")
+      end
+
+      before { allow(investment).to receive(:should_show_votes?).and_return(true) }
+
+      it "displays a link to support the investment to identified users" do
+        allow(controller).to receive(:current_user).and_return(create(:user))
+
+        render_inline component
+
+        expect(page).to have_link "Support"
+      end
+
+      it "does not display link to support the investment to unidentified users" do
+        allow(controller).to receive(:current_user).and_return(nil)
+
+        render_inline component
+
+        expect(page).not_to have_link "Support"
+        expect(page).to have_content "Support"
+      end
+    end
+  end
+end