From ee03712df069e0495571532d8e05179f7652b100 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 25 Feb 2025 18:28:14 +0100
Subject: [PATCH 1/4] Move milestones progress bars partial to a component

---
 app/assets/stylesheets/application.scss       |  1 +
 app/assets/stylesheets/milestones.scss        | 37 -------------------
 .../stylesheets/milestones/progress_bars.scss | 36 ++++++++++++++++++
 .../progress_bars_component.html.erb          | 22 +++++++++++
 .../milestones/progress_bars_component.rb     | 28 ++++++++++++++
 app/helpers/milestones_helper.rb              | 16 --------
 .../milestones/_milestones_content.html.erb   |  2 +-
 app/views/milestones/_progress_bars.html.erb  | 24 ------------
 .../progress_bars_component_spec.rb           | 33 +++++++++++++++++
 spec/system/legislation/processes_spec.rb     | 23 +-----------
 10 files changed, 122 insertions(+), 100 deletions(-)
 create mode 100644 app/assets/stylesheets/milestones/progress_bars.scss
 create mode 100644 app/components/milestones/progress_bars_component.html.erb
 create mode 100644 app/components/milestones/progress_bars_component.rb
 delete mode 100644 app/helpers/milestones_helper.rb
 delete mode 100644 app/views/milestones/_progress_bars.html.erb
 create mode 100644 spec/components/milestones/progress_bars_component_spec.rb

diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
index 202e903de..2d8805553 100644
--- a/app/assets/stylesheets/application.scss
+++ b/app/assets/stylesheets/application.scss
@@ -27,6 +27,7 @@
 @import "layout/**/*";
 @import "machine_learning/**/*";
 @import "management/**/*";
+@import "milestones/**/*";
 @import "moderation/**/*";
 @import "polls/**/*";
 @import "proposals/**/*";
diff --git a/app/assets/stylesheets/milestones.scss b/app/assets/stylesheets/milestones.scss
index 8d16aa395..4907a0d90 100644
--- a/app/assets/stylesheets/milestones.scss
+++ b/app/assets/stylesheets/milestones.scss
@@ -1,40 +1,3 @@
-$progress-bar-background: #fef0e2;
-$progress-bar-color:      #fea230;
-
-.tab-milestones {
-
-  .progress-bars {
-    margin-bottom: $line-height * 2;
-    margin-top: $line-height;
-
-    h5 {
-      font-size: rem-calc(24);
-    }
-
-    .progress {
-      background: $progress-bar-background;
-      border-radius: rem-calc(6);
-      position: relative;
-    }
-
-    .progress-meter {
-      background: $progress-bar-color;
-      border-radius: rem-calc(6);
-    }
-
-    .progress-meter-text {
-      color: color-pick-contrast($progress-bar-background);
-      right: 12px;
-      text-align: right;
-      transform: translate(0%, -50%);
-    }
-
-    .milestone-progress .row {
-      margin-bottom: calc($line-height / 2);
-    }
-  }
-}
-
 .tab-milestones .timeline li {
   margin: 0 auto;
   position: relative;
diff --git a/app/assets/stylesheets/milestones/progress_bars.scss b/app/assets/stylesheets/milestones/progress_bars.scss
new file mode 100644
index 000000000..c6f1d8b65
--- /dev/null
+++ b/app/assets/stylesheets/milestones/progress_bars.scss
@@ -0,0 +1,36 @@
+$progress-bar-background: #fef0e2;
+$progress-bar-color:      #fea230;
+
+.tab-milestones {
+
+  .progress-bars {
+    margin-bottom: $line-height * 2;
+    margin-top: $line-height;
+
+    h5 {
+      font-size: rem-calc(24);
+    }
+
+    .progress {
+      background: $progress-bar-background;
+      border-radius: rem-calc(6);
+      position: relative;
+    }
+
+    .progress-meter {
+      background: $progress-bar-color;
+      border-radius: rem-calc(6);
+    }
+
+    .progress-meter-text {
+      color: color-pick-contrast($progress-bar-background);
+      right: 12px;
+      text-align: right;
+      transform: translate(0%, -50%);
+    }
+
+    .milestone-progress .row {
+      margin-bottom: calc($line-height / 2);
+    }
+  }
+}
diff --git a/app/components/milestones/progress_bars_component.html.erb b/app/components/milestones/progress_bars_component.html.erb
new file mode 100644
index 000000000..e07e71b8c
--- /dev/null
+++ b/app/components/milestones/progress_bars_component.html.erb
@@ -0,0 +1,22 @@
+<section class="progress-bars">
+  <h5><%= t("milestones.index.progress") %></h5>
+
+  <div class="margin">
+    <%= progress_tag_for(milestoneable.primary_progress_bar) %>
+  </div>
+
+  <% if milestoneable.secondary_progress_bars.any? %>
+    <div class="milestone-progress">
+      <% milestoneable.secondary_progress_bars.each do |progress_bar| %>
+        <div class="row margin-bottom">
+          <div class="small-12 medium-6 large-4 column">
+            <%= progress_bar.title %>
+          </div>
+          <div class="small-12 medium-6 large-8 column end">
+            <%= progress_tag_for(progress_bar) %>
+          </div>
+        </div>
+      <% end %>
+    </div>
+  <% end %>
+</section>
diff --git a/app/components/milestones/progress_bars_component.rb b/app/components/milestones/progress_bars_component.rb
new file mode 100644
index 000000000..399005b3f
--- /dev/null
+++ b/app/components/milestones/progress_bars_component.rb
@@ -0,0 +1,28 @@
+class Milestones::ProgressBarsComponent < ApplicationComponent
+  attr_reader :milestoneable
+
+  def initialize(milestoneable)
+    @milestoneable = milestoneable
+  end
+
+  def render?
+    milestoneable.primary_progress_bar
+  end
+
+  private
+
+    def progress_tag_for(progress_bar)
+      text = number_to_percentage(progress_bar.percentage, precision: 0)
+
+      tag.div class: "progress",
+              role: "progressbar",
+              "aria-valuenow": progress_bar.percentage,
+              "aria-valuetext": "#{progress_bar.percentage}%",
+              "aria-valuemax": ProgressBar::RANGE.max,
+              "aria-valuemin": "0",
+              tabindex: "0" do
+        tag.span(class: "progress-meter", style: "width: #{progress_bar.percentage}%;") +
+          tag.p(text, class: "progress-meter-text")
+      end
+    end
+end
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
deleted file mode 100644
index 7a1b4339a..000000000
--- a/app/helpers/milestones_helper.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-module MilestonesHelper
-  def progress_tag_for(progress_bar)
-    text = number_to_percentage(progress_bar.percentage, precision: 0)
-
-    tag.div class: "progress",
-            role: "progressbar",
-            "aria-valuenow": progress_bar.percentage,
-            "aria-valuetext": "#{progress_bar.percentage}%",
-            "aria-valuemax": ProgressBar::RANGE.max,
-            "aria-valuemin": "0",
-            tabindex: "0" do
-      tag.span(class: "progress-meter", style: "width: #{progress_bar.percentage}%;") +
-        tag.p(text, class: "progress-meter-text")
-    end
-  end
-end
diff --git a/app/views/milestones/_milestones_content.html.erb b/app/views/milestones/_milestones_content.html.erb
index fcc76fadf..46576fc15 100644
--- a/app/views/milestones/_milestones_content.html.erb
+++ b/app/views/milestones/_milestones_content.html.erb
@@ -1,6 +1,6 @@
 <div class="row">
   <div class="small-12 column">
-    <%= render "milestones/progress_bars", milestoneable: milestoneable %>
+    <%= render Milestones::ProgressBarsComponent.new(milestoneable) %>
 
     <% if milestoneable.milestones.blank? %>
       <div class="callout primary text-center margin-top">
diff --git a/app/views/milestones/_progress_bars.html.erb b/app/views/milestones/_progress_bars.html.erb
deleted file mode 100644
index d2612487a..000000000
--- a/app/views/milestones/_progress_bars.html.erb
+++ /dev/null
@@ -1,24 +0,0 @@
-<% if milestoneable.primary_progress_bar %>
-  <section class="progress-bars">
-    <h5><%= t("milestones.index.progress") %></h5>
-
-    <div class="margin">
-      <%= progress_tag_for(milestoneable.primary_progress_bar) %>
-    </div>
-
-    <% if milestoneable.secondary_progress_bars.any? %>
-      <div class="milestone-progress">
-        <% milestoneable.secondary_progress_bars.each do |progress_bar| %>
-          <div class="row margin-bottom">
-            <div class="small-12 medium-6 large-4 column">
-              <%= progress_bar.title %>
-            </div>
-            <div class="small-12 medium-6 large-8 column end">
-              <%= progress_tag_for(progress_bar) %>
-            </div>
-          </div>
-        <% end %>
-      </div>
-    <% end %>
-  </section>
-<% end %>
diff --git a/spec/components/milestones/progress_bars_component_spec.rb b/spec/components/milestones/progress_bars_component_spec.rb
new file mode 100644
index 000000000..f30090385
--- /dev/null
+++ b/spec/components/milestones/progress_bars_component_spec.rb
@@ -0,0 +1,33 @@
+require "rails_helper"
+
+describe Milestones::ProgressBarsComponent do
+  let(:milestoneable) { create(:legislation_process) }
+
+  it "is not rendered without a main progress bar" do
+    create(:progress_bar, :secondary, progressable: milestoneable, title: "Defeat Evil Lords")
+
+    render_inline Milestones::ProgressBarsComponent.new(milestoneable)
+
+    expect(page).not_to be_rendered
+  end
+
+  it "renders a main progress bar" do
+    create(:progress_bar, progressable: milestoneable)
+
+    render_inline Milestones::ProgressBarsComponent.new(milestoneable)
+
+    expect(page).to have_content "Progress"
+    expect(page).to have_css "[role=progressbar]", count: 1
+  end
+
+  it "renders both main and secondary progress bars" do
+    create(:progress_bar, progressable: milestoneable)
+    create(:progress_bar, :secondary, progressable: milestoneable, title: "Build laboratory")
+
+    render_inline Milestones::ProgressBarsComponent.new(milestoneable)
+
+    expect(page).to have_content "Progress"
+    expect(page).to have_content "Build laboratory"
+    expect(page).to have_css "[role=progressbar]", count: 2
+  end
+end
diff --git a/spec/system/legislation/processes_spec.rb b/spec/system/legislation/processes_spec.rb
index 9061788c4..750760411 100644
--- a/spec/system/legislation/processes_spec.rb
+++ b/spec/system/legislation/processes_spec.rb
@@ -430,17 +430,7 @@ describe "Legislation" do
         end
       end
 
-      scenario "With main progress bar" do
-        create(:progress_bar, progressable: process)
-
-        visit milestones_legislation_process_path(process)
-
-        within(".tab-milestones") do
-          expect(page).to have_content "Progress"
-        end
-      end
-
-      scenario "With main and secondary progress bar" do
+      scenario "With progress bars" do
         create(:progress_bar, progressable: process)
         create(:progress_bar, :secondary, progressable: process, title: "Build laboratory")
 
@@ -451,17 +441,6 @@ describe "Legislation" do
           expect(page).to have_content "Build laboratory"
         end
       end
-
-      scenario "No main progress bar" do
-        create(:progress_bar, :secondary, progressable: process, title: "Defeat Evil Lords")
-
-        visit milestones_legislation_process_path(process)
-
-        within(".tab-milestones") do
-          expect(page).not_to have_content "Progress"
-          expect(page).not_to have_content "Defeat Evil Lords"
-        end
-      end
     end
   end
 end

From e0fc8bc83f26de8a585ccf73bfa6ea93d578cd46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 25 Feb 2025 18:37:16 +0100
Subject: [PATCH 2/4] Extract methods in milestones progress bars component

This way they're easier to reuse and customize.
---
 .../milestones/progress_bars_component.html.erb        |  6 +++---
 app/components/milestones/progress_bars_component.rb   | 10 +++++++++-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/app/components/milestones/progress_bars_component.html.erb b/app/components/milestones/progress_bars_component.html.erb
index e07e71b8c..e069d5c6e 100644
--- a/app/components/milestones/progress_bars_component.html.erb
+++ b/app/components/milestones/progress_bars_component.html.erb
@@ -2,12 +2,12 @@
   <h5><%= t("milestones.index.progress") %></h5>
 
   <div class="margin">
-    <%= progress_tag_for(milestoneable.primary_progress_bar) %>
+    <%= progress_tag_for(primary_progress_bar) %>
   </div>
 
-  <% if milestoneable.secondary_progress_bars.any? %>
+  <% if secondary_progress_bars.any? %>
     <div class="milestone-progress">
-      <% milestoneable.secondary_progress_bars.each do |progress_bar| %>
+      <% secondary_progress_bars.each do |progress_bar| %>
         <div class="row margin-bottom">
           <div class="small-12 medium-6 large-4 column">
             <%= progress_bar.title %>
diff --git a/app/components/milestones/progress_bars_component.rb b/app/components/milestones/progress_bars_component.rb
index 399005b3f..986385cec 100644
--- a/app/components/milestones/progress_bars_component.rb
+++ b/app/components/milestones/progress_bars_component.rb
@@ -6,11 +6,19 @@ class Milestones::ProgressBarsComponent < ApplicationComponent
   end
 
   def render?
-    milestoneable.primary_progress_bar
+    primary_progress_bar
   end
 
   private
 
+    def primary_progress_bar
+      milestoneable.primary_progress_bar
+    end
+
+    def secondary_progress_bars
+      milestoneable.secondary_progress_bars
+    end
+
     def progress_tag_for(progress_bar)
       text = number_to_percentage(progress_bar.percentage, precision: 0)
 

From 48593a19ea453fafc8e0de01cf344e35572e35b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 2 Apr 2025 14:57:19 +0200
Subject: [PATCH 3/4] Remove redundant test to vote an investment

This is already tested in the "Add an investment" test.
---
 spec/system/budgets/ballots_spec.rb | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb
index 2be0cdd0f..3eed8412a 100644
--- a/spec/system/budgets/ballots_spec.rb
+++ b/spec/system/budgets/ballots_spec.rb
@@ -103,15 +103,17 @@ describe "Ballots" do
     end
 
     context "Adding and Removing Investments" do
-      scenario "Add a investment" do
+      scenario "Add an investment" do
         create(:budget_investment, :selected, heading: new_york, price: 10000, title: "Bring back King Kong")
         create(:budget_investment, :selected, heading: new_york, price: 20000, title: "Paint cabs black")
 
         visit budget_investments_path(budget, heading_id: new_york)
         add_to_ballot("Bring back King Kong")
 
-        expect(page).to have_css("#total_amount", text: "€10,000")
-        expect(page).to have_css("#amount_available", text: "€990,000")
+        within("#progress_bar") do
+          expect(page).to have_css("#total_amount", text: "€10,000")
+          expect(page).to have_css("#amount_available", text: "€990,000")
+        end
 
         within("#sidebar") do
           expect(page).to have_content "Bring back King Kong"
@@ -268,18 +270,6 @@ describe "Ballots" do
         expect(page).to have_content("You have active votes in another heading: District 1")
       end
     end
-
-    scenario "Display progress bar after first vote" do
-      create(:budget_investment, :selected, heading: new_york, price: 10000, title: "Park expansion")
-
-      visit budget_investments_path(budget, heading_id: new_york.id)
-
-      add_to_ballot("Park expansion")
-
-      within("#progress_bar") do
-        expect(page).to have_css("#total_amount", text: "€10,000")
-      end
-    end
   end
 
   context "Groups" do

From 45c74681c4fa05491bd81814525282e78f280f5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 25 Feb 2025 14:11:00 +0100
Subject: [PATCH 4/4] Add ARIA labels to progressbars

People using screen readers might have a hard time knowing what a
progressbar is about unless we provide a label for it. Axe was reporting
failures like:

```
aria-progressbar-name: ARIA progressbar nodes must have an accessible
name (serious)
https://dequeuniversity.com/rules/axe/4.10/aria-progressbar-name?application=axeAPI
The following 1 node violate this rule:

  Selector: .progress
  HTML: <div class="progress" role="progressbar" tabindex="0"
             aria-valuenow="0.0" aria-valuemin="0" aria-valuemax="100">
          <div class="progress-meter" style="width: 0.0%"></div>
        </div>
  Fix any of the following:
  - aria-label attribute does not exist or is empty
  - aria-labelledby attribute does not exist, references
    elements that do not exist or references elements that are empty
  - Element has no title attribute
```

Note that, in the case of the ballot progressbar, it's easier to use
`aria-labelledby`, while in other place it's easier to use `aria-label`,
so we using the easier solution in each scenario.
---
 app/components/milestones/progress_bars_component.html.erb | 4 ++--
 app/components/milestones/progress_bars_component.rb       | 3 ++-
 app/views/budgets/ballot/_progress_bar.html.erb            | 2 +-
 app/views/layouts/dashboard/_proposal_totals.html.erb      | 3 ++-
 spec/components/milestones/progress_bars_component_spec.rb | 3 +++
 spec/system/budgets/ballots_spec.rb                        | 6 ++++++
 6 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/app/components/milestones/progress_bars_component.html.erb b/app/components/milestones/progress_bars_component.html.erb
index e069d5c6e..4349a991e 100644
--- a/app/components/milestones/progress_bars_component.html.erb
+++ b/app/components/milestones/progress_bars_component.html.erb
@@ -2,7 +2,7 @@
   <h5><%= t("milestones.index.progress") %></h5>
 
   <div class="margin">
-    <%= progress_tag_for(primary_progress_bar) %>
+    <%= progress_tag_for(primary_progress_bar, label: t("milestones.index.progress")) %>
   </div>
 
   <% if secondary_progress_bars.any? %>
@@ -13,7 +13,7 @@
             <%= progress_bar.title %>
           </div>
           <div class="small-12 medium-6 large-8 column end">
-            <%= progress_tag_for(progress_bar) %>
+            <%= progress_tag_for(progress_bar, label: progress_bar.title) %>
           </div>
         </div>
       <% end %>
diff --git a/app/components/milestones/progress_bars_component.rb b/app/components/milestones/progress_bars_component.rb
index 986385cec..345619fa6 100644
--- a/app/components/milestones/progress_bars_component.rb
+++ b/app/components/milestones/progress_bars_component.rb
@@ -19,11 +19,12 @@ class Milestones::ProgressBarsComponent < ApplicationComponent
       milestoneable.secondary_progress_bars
     end
 
-    def progress_tag_for(progress_bar)
+    def progress_tag_for(progress_bar, label:)
       text = number_to_percentage(progress_bar.percentage, precision: 0)
 
       tag.div class: "progress",
               role: "progressbar",
+              "aria-label": label,
               "aria-valuenow": progress_bar.percentage,
               "aria-valuetext": "#{progress_bar.percentage}%",
               "aria-valuemax": ProgressBar::RANGE.max,
diff --git a/app/views/budgets/ballot/_progress_bar.html.erb b/app/views/budgets/ballot/_progress_bar.html.erb
index 1351d6e0f..1e54e35ca 100644
--- a/app/views/budgets/ballot/_progress_bar.html.erb
+++ b/app/views/budgets/ballot/_progress_bar.html.erb
@@ -2,7 +2,7 @@
   <%= sanitize(ballot.amount_progress(heading)) %>
 </p>
 
-<div class="progress" role="progressbar" tabindex="0"
+<div class="progress" role="progressbar" tabindex="0" aria-labelledby="total_amount"
      aria-valuenow="<%= ballot.percentage_spent(heading) %>" aria-valuemin="0" aria-valuemax="100">
   <div class="progress-meter"
        style="width: <%= ballot.percentage_spent(heading) %>%">
diff --git a/app/views/layouts/dashboard/_proposal_totals.html.erb b/app/views/layouts/dashboard/_proposal_totals.html.erb
index b1f5b7a40..122626d41 100644
--- a/app/views/layouts/dashboard/_proposal_totals.html.erb
+++ b/app/views/layouts/dashboard/_proposal_totals.html.erb
@@ -37,7 +37,8 @@
           <%= next_goal_progress %>%
         </div>
         <div class="progress" role="progressbar" tabindex="0" aria-valuenow="0" aria-valuemin="0"
-             aria-valuetext="<%= "#{next_goal_progress}%" %>" aria-valuemax="100">
+             aria-valuetext="<%= "#{next_goal_progress}%" %>" aria-valuemax="100"
+             aria-label="<%= t("layouts.dashboard.proposal_totals.current_goal") %>">
           <div class="progress-meter" style="width: <%= next_goal_progress %>%"></div>
         </div>
       </div>
diff --git a/spec/components/milestones/progress_bars_component_spec.rb b/spec/components/milestones/progress_bars_component_spec.rb
index f30090385..2919eca5f 100644
--- a/spec/components/milestones/progress_bars_component_spec.rb
+++ b/spec/components/milestones/progress_bars_component_spec.rb
@@ -18,6 +18,7 @@ describe Milestones::ProgressBarsComponent do
 
     expect(page).to have_content "Progress"
     expect(page).to have_css "[role=progressbar]", count: 1
+    expect(page).to have_css "[role=progressbar][aria-label='Progress']"
   end
 
   it "renders both main and secondary progress bars" do
@@ -29,5 +30,7 @@ describe Milestones::ProgressBarsComponent do
     expect(page).to have_content "Progress"
     expect(page).to have_content "Build laboratory"
     expect(page).to have_css "[role=progressbar]", count: 2
+    expect(page).to have_css "[role=progressbar][aria-label='Progress']"
+    expect(page).to have_css "[role=progressbar][aria-label='Build laboratory']"
   end
 end
diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb
index 3eed8412a..727b996f9 100644
--- a/spec/system/budgets/ballots_spec.rb
+++ b/spec/system/budgets/ballots_spec.rb
@@ -108,6 +108,12 @@ describe "Ballots" do
         create(:budget_investment, :selected, heading: new_york, price: 20000, title: "Paint cabs black")
 
         visit budget_investments_path(budget, heading_id: new_york)
+
+        within("#progress_bar") do
+          expect(page).to have_css "#total_amount", exact_text: "AMOUNT SPENT €0 / TOTAL BUDGET €1,000,000"
+          expect(page).to have_css "[role=progressbar][aria-labelledby='total_amount']"
+        end
+
         add_to_ballot("Bring back King Kong")
 
         within("#progress_bar") do