From 4c23f639becf0af00a4b878960cf7c5572c3e2b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 15 Mar 2021 23:59:18 +0100
Subject: [PATCH] Use heading name as link text

Using the name instead of using the name and the price is IMHO more
consistent with the rest of the application, particularly for screen
reader users. Writing texts clicking those links is also easier.

I think the main reason why we used the price as part of the link was so
the clickable area was bigger. We can accomplish the same result with
CSS.
---
 .../budgets/groups_and_headings.scss          | 35 ++++++++++++++-----
 .../groups_and_headings_component.html.erb    | 17 ++++-----
 .../budgets/groups_and_headings_component.rb  |  7 ++--
 spec/system/budgets/budgets_spec.rb           |  4 +--
 spec/system/budgets/investments_spec.rb       |  2 +-
 5 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/app/assets/stylesheets/budgets/groups_and_headings.scss b/app/assets/stylesheets/budgets/groups_and_headings.scss
index 1be85a2be..6f6b4a2d9 100644
--- a/app/assets/stylesheets/budgets/groups_and_headings.scss
+++ b/app/assets/stylesheets/budgets/groups_and_headings.scss
@@ -13,6 +13,8 @@
     border-radius: rem-calc(3);
     margin-bottom: $line-height / 2;
     margin-left: $spacing;
+    padding: $line-height / 2;
+    position: relative;
     width: 100%;
 
     @include breakpoint(medium) {
@@ -23,22 +25,37 @@
       width: calc(100% / 6 - #{$spacing});
     }
 
-    a {
-      display: block;
-      padding: $line-height / 2;
+    &:focus-within {
+      outline: $outline-focus;
 
-      &:hover {
-        background: $highlight;
-        text-decoration: none;
+      a:focus {
+        outline: none;
       }
     }
 
-    .heading-name {
-      padding: $line-height / 2;
+    a {
+
+      &::after,
+      &::before {
+        bottom: 0;
+        content: "";
+        left: 0;
+        position: absolute;
+        right: 0;
+        top: 0;
+      }
+
+      &:hover {
+        text-decoration: none;
+      }
+
+      &:hover::before {
+        background: $highlight;
+        z-index: -1;
+      }
     }
 
     span {
-      color: $text;
       display: block;
       font-size: $small-font-size;
     }
diff --git a/app/components/budgets/groups_and_headings_component.html.erb b/app/components/budgets/groups_and_headings_component.html.erb
index 6c88b9850..6f44960dd 100644
--- a/app/components/budgets/groups_and_headings_component.html.erb
+++ b/app/components/budgets/groups_and_headings_component.html.erb
@@ -4,16 +4,13 @@
     <ul class="headings-list">
       <% group.headings.sort_by_name.each do |heading| %>
         <li class="heading">
-          <% unless budget.informing? || budget.finished? %>
-            <%= link_to budget_investments_path(budget.id,
-                                                heading_id: heading.id) do %>
-              <%= heading_name_and_price_html(heading) %>
-            <% end %>
-          <% else %>
-            <div class="heading-name">
-              <%= heading_name_and_price_html(heading) %>
-            </div>
-          <% end %>
+          <%= link_to_unless(
+            (budget.informing? || budget.finished?),
+            heading.name,
+            budget_investments_path(budget.id, heading_id: heading.id)
+          ) %>
+
+          <%= price(heading) %>
         </li>
       <% end %>
     </ul>
diff --git a/app/components/budgets/groups_and_headings_component.rb b/app/components/budgets/groups_and_headings_component.rb
index 87d0b247d..b16357e4b 100644
--- a/app/components/budgets/groups_and_headings_component.rb
+++ b/app/components/budgets/groups_and_headings_component.rb
@@ -7,10 +7,7 @@ class Budgets::GroupsAndHeadingsComponent < ApplicationComponent
 
   private
 
-    def heading_name_and_price_html(heading)
-      tag.div do
-        concat(heading.name + " ")
-        concat(tag.span(budget.formatted_heading_price(heading)))
-      end
+    def price(heading)
+      tag.span(budget.formatted_heading_price(heading))
     end
 end
diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb
index 022ad0693..15e142afd 100644
--- a/spec/system/budgets/budgets_spec.rb
+++ b/spec/system/budgets/budgets_spec.rb
@@ -118,7 +118,7 @@ describe "Budgets" do
       visit budgets_path
 
       within("#budget_info") do
-        expect(page).not_to have_link "#{heading.name} €1,000,000"
+        expect(page).not_to have_link heading.name
         expect(page).to have_content "#{heading.name} €1,000,000"
 
         expect(page).not_to have_link("List of all investment projects")
@@ -136,7 +136,7 @@ describe "Budgets" do
       visit budgets_path
 
       within("#budget_info") do
-        expect(page).not_to have_link "#{heading.name} €1,000,000"
+        expect(page).not_to have_link heading.name
         expect(page).to have_content "#{heading.name} €1,000,000"
 
         expect(page).to have_css("div.map")
diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb
index 5640def42..5cdfb9b8d 100644
--- a/spec/system/budgets/investments_spec.rb
+++ b/spec/system/budgets/investments_spec.rb
@@ -1230,7 +1230,7 @@ describe "Budget Investments" do
 
       first(:link, "Participatory budgeting").click
 
-      click_link "More hospitals €666,666"
+      click_link "More hospitals"
 
       within("#budget_investment_#{investment1.id}") do
         expect(page).to have_content investment1.title