Replace support investment link with a button

Using links combined with JavaScript to generate POST requests to the
browser has a few issues.

An obvious one is that it doesn't work for users without JavaScript
enabled (which lately I've noticed are more common than I thought, even
though I've been one of them for years). These users will reach a 404
page.

Since CONSUL isn't currently designed to work without JavaScript
enabled, let's ignore this one for now.

A not so obvious issue is screen reader users might expect the link to
take them somewhere instead of performing an action (in this case,
sending a form to the server).

There might be more issues I'm unaware of. Quoting DHH [1]:

"Turning ahrefs into POSTs is a bit of an anti-pattern, especially for
a11y reasons. Better to use button_to with a styling."

So we're using a button instead. This way we can also simplify the code
and make the button disabled for unidentified users, which automatically
makes it impossible to focus it using the keyboard.

A possible disadvantage of using `button_to` is it will create a form
tag which will be announced to screen readers as a form landmark. I've
tested it with my screen reader and everything worked fine for me, but
some screen reader users might interact with these forms in a different
way and find it confusing, particularly in the case where the button is
disabled.

With this change, we're only changing links for buttons in one place.
There are other places where we should do similar changes.

[1] See issue 33 in https://github.com/hotwired/turbo-rails/

app/assets/stylesheets/participation.scss

@@ -815,12 +815,20 @@
       font-size: $base-font-size;
       font-weight: bold;
 
+      &:disabled {
+        opacity: 1;
+      }
+
       &:hover {
         background: $budget-hover;
         color: #fff;
         cursor: pointer;
       }
 
+      &:focus {
+        outline: $outline-focus;
+      }
+
       &:active {
         opacity: 0.75;
       }

app/components/budgets/investments/votes_component.html.erb

@@ -10,19 +10,14 @@
         <%= t("budgets.investments.investment.already_supported") %>
       </div>
     <% elsif investment.should_show_votes? %>
-      <% if current_user %>
+      <%= button_to t("budgets.investments.investment.give_support"), vote_url,
-        <%= 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?),
+          disabled: !current_user,
           "aria-label": support_aria_label %>
-      <% else %>
-        <div class="button button-support small expanded">
-          <%= t("budgets.investments.investment.give_support") %>
-        </div>
-      <% end %>
     <% end %>
   </div>
 

spec/components/budgets/investments/votes_component_spec.rb

@@ -10,23 +10,22 @@ describe Budgets::Investments::VotesComponent, type: :component do
 
       before { allow(investment).to receive(:should_show_votes?).and_return(true) }
 
-      it "displays a link to support the investment to identified users" do
+      it "displays a button 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 count: 1
+        expect(page).to have_button count: 1
-        expect(page).to have_link "Support", title: "Support this project"
+        expect(page).to have_button "Support", title: "Support this project", disabled: false
-        expect(page).to have_link "Support Renovate sidewalks in Main Street"
+        expect(page).to have_button "Support Renovate sidewalks in Main Street"
       end
 
-      it "does not display link to support the investment to unidentified users" do
+      it "disables the button 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_button "Support", disabled: true
-        expect(page).to have_content "Support"
       end
     end
   end

spec/system/budgets/investments_spec.rb

@@ -1113,7 +1113,9 @@ describe "Budget Investments" do
         visit budget_investments_path(budget, heading_id: carabanchel.id)
 
         within(".budget-investment", text: "In Carabanchel") do
-          expect(page).to have_css(".in-favor a[data-confirm]")
+          expect(page).to have_button count: 1
+          expect(page).to have_button "Support"
+          expect(page).to have_css "[type='submit'][data-confirm]"
         end
       end
 
@@ -1129,8 +1131,8 @@ describe "Budget Investments" do
         visit budget_investments_path(budget, heading_id: carabanchel.id)
 
         within(".budget-investment", text: "Unsupported in Carabanchel") do
-          expect(page).to have_link "Support"
+          expect(page).to have_button "Support"
-          expect(page).not_to have_css(".in-favor a[data-confirm]")
+          expect(page).not_to have_css "[data-confirm]"
         end
       end
 
@@ -1148,7 +1150,9 @@ describe "Budget Investments" do
         visit budget_investments_path(budget, heading_id: another_heading1.id)
 
         within(".budget-investment", text: "Another investment") do
-          expect(page).to have_css(".in-favor a[data-confirm]")
+          expect(page).to have_button count: 1
+          expect(page).to have_button "Support"
+          expect(page).to have_css "[type='submit'][data-confirm]"
         end
       end
 
@@ -1159,7 +1163,8 @@ describe "Budget Investments" do
         visit budget_investments_path(budget, heading_id: heading.id)
 
         within("#budget_investment_#{all_city_investment.id}") do
-          expect(page).not_to have_css(".in-favor a[data-confirm]")
+          expect(page).to have_button "Support"
+          expect(page).not_to have_css "[data-confirm]"
         end
       end
     end

spec/system/budgets/votes_spec.rb

@@ -42,7 +42,7 @@ describe "Votes" do
         visit budget_investments_path(budget, heading_id: heading.id)
 
         within(".supports") do
-          click_link "Support"
+          click_button "Support"
 
           expect(page).to have_content "1 support"
           expect(page).to have_content "You have already supported this investment project. "\
@@ -63,10 +63,10 @@ describe "Votes" do
         visit budget_investment_path(budget, investment)
 
         within(".supports") do
-          find(".in-favor a").click
+          click_button "Support"
-          expect(page).to have_content "1 support"
 
-          expect(page).not_to have_selector ".in-favor a"
+          expect(page).not_to have_button "Support", disabled: :all
+          expect(page).to have_content "1 support"
         end
       end
 
@@ -74,7 +74,7 @@ describe "Votes" do
         visit budget_investment_path(budget, investment)
 
         within(".supports") do
-          find(".in-favor a").click
+          click_button "Support"
 
           expect(page).to have_content "1 support"
           expect(page).to have_content "You have already supported this investment project. "\
@@ -120,7 +120,7 @@ describe "Votes" do
         visit budget_investments_path(budget, heading_id: new_york.id)
 
         within("#budget_investment_#{new_york_investment.id}") do
-          accept_confirm { find(".in-favor a").click }
+          accept_confirm { click_button "Support" }
 
           expect(page).to have_content "1 support"
           expect(page).to have_content "You have already supported this investment project. "\
@@ -130,7 +130,7 @@ describe "Votes" do
         visit budget_investments_path(budget, heading_id: san_francisco.id)
 
         within("#budget_investment_#{san_francisco_investment.id}") do
-          find(".in-favor a").click
+          click_button "Support"
 
           expect(page).to have_content "1 support"
           expect(page).to have_content "You have already supported this investment project. "\
@@ -140,7 +140,7 @@ describe "Votes" do
         visit budget_investments_path(budget, heading_id: third_heading.id)
 
         within("#budget_investment_#{third_heading_investment.id}") do
-          find(".in-favor a").click
+          click_button "Support"
 
           expect(page).to have_content "You can only support investment projects in 2 districts. "\
                                        "You have already supported investments in"
@@ -160,19 +160,22 @@ describe "Votes" do
       scenario "From show" do
         visit budget_investment_path(budget, new_york_investment)
 
-        accept_confirm { find(".in-favor a").click }
+        accept_confirm { click_button "Support" }
+
         expect(page).to have_content "1 support"
         expect(page).to have_content "You have already supported this investment project. Share it!"
 
         visit budget_investment_path(budget, san_francisco_investment)
 
-        find(".in-favor a").click
+        click_button "Support"
+
         expect(page).to have_content "1 support"
         expect(page).to have_content "You have already supported this investment project. Share it!"
 
         visit budget_investment_path(budget, third_heading_investment)
 
-        find(".in-favor a").click
+        click_button "Support"
+
         expect(page).to have_content "You can only support investment projects in 2 districts. "\
                                      "You have already supported investments in"
 
@@ -189,7 +192,7 @@ describe "Votes" do
 
       scenario "Confirm message shows the right text" do
         visit budget_investments_path(budget, heading_id: new_york.id)
-        find(".in-favor a").click
+        click_button "Support"
 
         expect(page.driver.send(:find_modal).text).to match "You can only support investments in 2 districts."
       end

spec/system/management/budget_investments_spec.rb

@@ -277,7 +277,7 @@ describe "Budget Investments" do
       expect(page).to have_content(budget_investment.title)
 
       within("#budget-investments") do
-        find("in-favor a").click
+        click_button "Support"
 
         expect(page).to have_content "1 support"
         expect(page).to have_content "You have already supported this investment project. Share it!"
@@ -302,7 +302,7 @@ describe "Budget Investments" do
 
       expect(page).to have_css "h1", exact_text: budget_investment.title
 
-      find(".in-favor a").click
+      click_button "Support"
 
       expect(page).to have_content "1 support"
       expect(page).to have_content "You have already supported this investment project. Share it!"
@@ -325,7 +325,7 @@ describe "Budget Investments" do
 
       expect(page).to have_css "h1", exact_text: "Default heading investment"
 
-      accept_confirm { find(".in-favor a").click }
+      accept_confirm { click_button "Support" }
 
       expect(page).to have_content "1 support"
       expect(page).to have_content "You have already supported this investment project. Share it!"