Use buttons to vote in the final voting

As mentioned in commits 5311daadf and bb958daf0, using links combined with JavaScript to generate POST requests to the server has a few issues. We're also improving the keyboard access. Previously, the links were focusable and clickable with the keyboard. Now we're disabling the buttons when voting isn't allowed. Since these elements can no longer be focused, we're adding an element with `tabindex="0"` so the "participation not allowed" message is shown, like we do in most places. Note we're slightly changing one test because now when hovering over the button on Chrome, the "participation not allowed" text isn't shown; it's only shown when hovering on the parts of the `div.ballot` element outside the button. Since we're already rewriting the behavior of the "participation not allowed" text in a different pull request, we aren't going to fix this behavior.
2021-09-30 22:07:14 +02:00
parent f970ddcb86
commit 7a25c61ae6
5 changed files with 66 additions and 61 deletions
--- a/app/components/budgets/investments/ballot_component.html.erb
+++ b/app/components/budgets/investments/ballot_component.html.erb
@@ -8,14 +8,15 @@
        <%= investment.formatted_price %>
      </p>
      <% if investment.should_show_ballots? %>
-        <%= link_to t("budgets.ballots.show.remove"),
-                    budget_ballot_line_path(id: investment.id,
-                                            budget_id: investment.budget_id,
-                                            investments_ids: investment_ids),
-                    class: "button button-remove-support expanded",
-                    method: :delete,
-                    remote: true,
-                    "aria-label": remove_vote_aria_label %>
+        <%= button_to budget_ballot_line_path(id: investment.id,
+                                              budget_id: investment.budget_id,
+                                              investments_ids: investment_ids),
+                      class: "button button-remove-support expanded",
+                      method: :delete,
+                      remote: true,
+                      "aria-label": remove_vote_aria_label do %>
+          <%= t("budgets.ballots.show.remove") %>
+        <% end %>
      <% end %>
    </div>
  <% else %>
@@ -24,30 +25,34 @@
        <%= investment.formatted_price %>
      </p>
      <% if investment.should_show_ballots? %>
-        <%= link_to t("budgets.investments.investment.add"),
-                    budget_ballot_lines_path(investment_id: investment.id,
-                                             budget_id: investment.budget_id,
-                                             investments_ids: investment_ids),
-                    class: "button button-support expanded",
-                    title: t("budgets.investments.investment.support_title"),
-                    method: :post,
-                    remote: true,
-                    "aria-label": vote_aria_label %>
+        <%= button_to budget_ballot_lines_path(investment_id: investment.id,
+                                               budget_id: investment.budget_id,
+                                               investments_ids: investment_ids),
+                      class: "button button-support expanded",
+                      title: t("budgets.investments.investment.support_title"),
+                      method: :post,
+                      remote: true,
+                      disabled: reason.present?,
+                      "aria-label": vote_aria_label do %>
+          <%= t("budgets.investments.investment.add") %>
+        <% end %>
      <% end %>
    </div>
  <% end %>

  <% if reason.present? && !voted? %>
-    <div class="participation-not-allowed" style="display:none">
-      <p>
-        <small>
-          <%= sanitize(t("budgets.ballots.reasons_for_not_balloting.#{reason}",
-                verify_account: link_to_verify_account, signin: link_to_signin,
-                signup: link_to_signup, my_heading: link_to_my_heading,
-                change_ballot: link_to_change_ballot,
-                heading_link: heading_link(assigned_heading, budget))) %>
-        </small>
-      </p>
+    <div tabindex="0">
+      <div class="participation-not-allowed" style="display:none">
+        <p>
+          <small>
+            <%= sanitize(t("budgets.ballots.reasons_for_not_balloting.#{reason}",
+                  verify_account: link_to_verify_account, signin: link_to_signin,
+                  signup: link_to_signup, my_heading: link_to_my_heading,
+                  change_ballot: link_to_change_ballot,
+                  heading_link: heading_link(assigned_heading, budget))) %>
+          </small>
+        </p>
+      </div>
    </div>
  <% end %>
 </div>
--- a/spec/components/budgets/investments/ballot_component_spec.rb
+++ b/spec/components/budgets/investments/ballot_component_spec.rb
@@ -1,7 +1,7 @@
 require "rails_helper"

 describe Budgets::Investments::BallotComponent do
-  describe "vote investment link" do
+  describe "vote investment button" do
    let(:budget) { create(:budget, :balloting) }
    let(:investment) { create(:budget_investment, :selected, title: "New Sports Center", budget: budget) }
    let(:component) do
@@ -18,7 +18,7 @@ describe Budgets::Investments::BallotComponent do

      render_inline component

-      expect(page).to have_link "Vote"
+      expect(page).to have_button "Vote", disabled: true
      expect(page).to have_content "Only verified users can vote on investments; verify your account."
    end

@@ -27,21 +27,21 @@ describe Budgets::Investments::BallotComponent do

      render_inline component

-      expect(page).to have_link count: 1
-      expect(page).to have_link "Vote", title: "Support this project"
-      expect(page).to have_link "Vote New Sports Center"
-      expect(page).not_to have_link "Remove vote"
+      expect(page).to have_button count: 1
+      expect(page).to have_button "Vote", title: "Support this project"
+      expect(page).to have_button "Vote New Sports Center"
+      expect(page).not_to have_button "Remove vote", disabled: :all
    end

-    it "is replaced with a link to remove the vote when the user has already voted" do
+    it "is replaced with a button to remove the vote when the user has already voted" do
      sign_in(create(:user, :level_two, ballot_lines: [investment]))

      render_inline component

-      expect(page).to have_link count: 1
-      expect(page).to have_link "Remove vote"
-      expect(page).to have_link "Remove your vote for New Sports Center"
-      expect(page).not_to have_link "Vote"
+      expect(page).to have_button count: 1
+      expect(page).to have_button "Remove vote"
+      expect(page).to have_button "Remove your vote for New Sports Center"
+      expect(page).not_to have_button "Vote", disabled: :all
    end
  end
 end
--- a/spec/support/common_actions/budgets.rb
+++ b/spec/support/common_actions/budgets.rb
@@ -1,17 +1,17 @@
 module Budgets
  def expect_message_organizations_cannot_vote
    expect(page).to have_content "Organization"
-    expect(page).to have_selector(".in-favor a", obscured: true)
+    expect(page).to have_button "Vote", disabled: true, obscured: true
  end

  def hover_over_ballot
    scroll_to find("div.ballot"), align: :bottom
-    find("div.ballot").hover
+    first("div.ballot p").hover
  end

  def add_to_ballot(investment_title)
    within(".budget-investment", text: investment_title) do
-      find(".add a").click
+      click_button "Vote"
      expect(page).to have_content "Remove"
    end
  end
--- a/spec/system/budget_polls/voter_spec.rb
+++ b/spec/system/budget_polls/voter_spec.rb
@@ -81,11 +81,11 @@ describe "BudgetPolls", :with_frozen_time do
      login_as(user)

      visit budget_investment_path(budget, investment)
-      find("div.ballot").hover
+      first("div.ballot p").hover

      within("#budget_investment_#{investment.id}") do
        expect(page).to have_content "You have already participated offline"
-        expect(page).to have_css(".add a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end
    end
  end
@@ -96,7 +96,7 @@ describe "BudgetPolls", :with_frozen_time do
      visit budget_investment_path(budget, investment)

      within("#budget_investment_#{investment.id}") do
-        find(".add a").click
+        click_button "Vote"
        expect(page).to have_content "Remove"
      end
    end
@@ -106,7 +106,7 @@ describe "BudgetPolls", :with_frozen_time do
      visit budget_investment_path(budget, investment)

      within("#budget_investment_#{investment.id}") do
-        find(".add a").click
+        click_button "Vote"
        expect(page).to have_content "Remove"
      end

@@ -123,7 +123,7 @@ describe "BudgetPolls", :with_frozen_time do
      visit budget_investment_path(budget, investment)

      within("#budget_investment_#{investment.id}") do
-        find(".add a").click
+        click_button "Vote"
        expect(page).to have_content "Remove"
      end

--- a/spec/system/budgets/ballots_spec.rb
+++ b/spec/system/budgets/ballots_spec.rb
@@ -147,7 +147,7 @@ describe "Ballots" do
        end

        within("#budget_investment_#{investment.id}") do
-          find(".remove a").click
+          click_button "Remove vote"
        end

        expect(page).to have_css("#total_amount", text: "€0")
@@ -283,8 +283,8 @@ describe "Ballots" do
      visit budget_investments_path(budget, heading_id: california.id)

      within(".budget-investment", text: "Early ShakeAlert") do
-        find(".remove a").click
-        expect(page).to have_link "Vote"
+        click_button "Remove vote"
+        expect(page).to have_button "Vote"
      end

      visit budget_investments_path(budget, heading_id: new_york.id)
@@ -449,7 +449,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_content "You must sign in or sign up to continue."
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end
    end

@@ -515,7 +515,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_content("already voted a different heading")
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end
    end

@@ -531,7 +531,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end
    end

@@ -546,7 +546,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).not_to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: false)
+        expect(page).to have_button "Vote", obscured: false
      end

      add_to_ballot("Build replicants")
@@ -555,7 +555,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end
    end

@@ -571,7 +571,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end

      within("#budget_investment_#{bi1.id}") do
@@ -583,7 +583,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).not_to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: false)
+        expect(page).to have_button "Vote", obscured: false
      end
    end

@@ -599,7 +599,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end

      within("#budget_investment_#{bi1.id}_sidebar") do
@@ -612,7 +612,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).not_to have_content("You have already assigned the available budget")
-        expect(page).to have_selector(".in-favor a", obscured: false)
+        expect(page).to have_button "Vote", obscured: false
      end
    end

@@ -623,7 +623,7 @@ describe "Ballots" do
      login_as user
      visit budget_investments_path(budget, heading_id: new_york)

-      expect(page).to have_css(".in-favor a")
+      expect(page).to have_button "Vote"

      in_browser(:admin) do
        login_as admin_user
@@ -639,7 +639,7 @@ describe "Ballots" do
      end

      within("#budget_investment_#{investment1.id}") do
-        find(".in-favor a").click
+        click_button "Vote"

        expect(page).not_to have_content "Remove"
        expect(page).not_to have_selector(".participation-not-allowed")
@@ -647,7 +647,7 @@ describe "Ballots" do
        hover_over_ballot

        expect(page).to have_selector(".participation-not-allowed")
-        expect(page).to have_selector(".in-favor a", obscured: true)
+        expect(page).to have_button "Vote", disabled: true, obscured: true
      end
    end