From 9c057d56950fb62317a5d602c0733fb4b447b7c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 3 Apr 2024 23:24:18 +0200
Subject: [PATCH] Fix labels in color selection

We were using the same label for two elements, but the label was only
assigned to one of them.
---
 .../admin/banners/form_component.html.erb     | 12 +++---
 .../admin/geozones/form_component.html.erb    |  6 +--
 .../processes/form_component.html.erb         | 12 +++---
 .../admin/banners/form_component_spec.rb      | 38 +++++++++++++++++++
 .../admin/geozones/form_component_spec.rb     | 22 +++++++++++
 .../processes/form_component_spec.rb          | 38 +++++++++++++++++++
 6 files changed, 113 insertions(+), 15 deletions(-)
 create mode 100644 spec/components/admin/banners/form_component_spec.rb
 create mode 100644 spec/components/admin/geozones/form_component_spec.rb
 create mode 100644 spec/components/admin/legislation/processes/form_component_spec.rb

diff --git a/app/components/admin/banners/form_component.html.erb b/app/components/admin/banners/form_component.html.erb
index 7468a2ff0..a3d5c3d83 100644
--- a/app/components/admin/banners/form_component.html.erb
+++ b/app/components/admin/banners/form_component.html.erb
@@ -45,12 +45,12 @@
   </div>
 
   <div class="row">
-    <div class="small-12 medium-6 large-3 column">
-      <%= f.label :background_color, nil, for: "background_color_input" %>
+    <div class="small-12 medium-6 large-3 column background-color-inputs">
+      <%= f.label :background_color, nil, for: "background_color_input", id: "background_color_label" %>
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :background_color, label: false, type: :color %>
+          <%= f.text_field :background_color, label: false, type: :color, "aria-labelledby": "background_color_label" %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :background_color, label: false, id: "background_color_input" %>
@@ -58,12 +58,12 @@
       </div>
     </div>
 
-    <div class="small-12 medium-6 large-3 column end">
-      <%= f.label :font_color, nil, for: "font_color_input" %>
+    <div class="small-12 medium-6 large-3 column end font-color-inputs">
+      <%= f.label :font_color, nil, for: "font_color_input", id: "font_color_label" %>
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :font_color, label: false, type: :color %>
+          <%= f.text_field :font_color, label: false, type: :color, "aria-labelledby": "font_color_label" %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :font_color, label: false, id: "font_color_input" %>
diff --git a/app/components/admin/geozones/form_component.html.erb b/app/components/admin/geozones/form_component.html.erb
index bd6a00ab9..ddf2a42d4 100644
--- a/app/components/admin/geozones/form_component.html.erb
+++ b/app/components/admin/geozones/form_component.html.erb
@@ -24,14 +24,14 @@
     <%= f.text_area :geojson, rows: "10", hint: t("admin.geozones.geozone.geojson_help") %>
   </div>
 
-  <div class="small-12 large-3 column">
-    <%= f.label :color, nil, for: "color_input", id: "color_input_label" %>
+  <div class="small-12 large-3 column color-inputs">
+    <%= f.label :color, nil, for: "color_input", id: "color_label" %>
     <p class="help-text">
       <%= t("admin.geozones.geozone.color_help", format_help: t("admin.shared.color_help")) %>
     </p>
     <div class="row collapse">
       <div class="small-12 medium-6 column">
-        <%= f.text_field :color, label: false, type: :color %>
+        <%= f.text_field :color, label: false, type: :color, "aria-labelledby": "color_label" %>
       </div>
       <div class="small-12 medium-6 column">
         <%= f.text_field :color, label: false, id: "color_input" %>
diff --git a/app/components/admin/legislation/processes/form_component.html.erb b/app/components/admin/legislation/processes/form_component.html.erb
index 82516b498..6662148fc 100644
--- a/app/components/admin/legislation/processes/form_component.html.erb
+++ b/app/components/admin/legislation/processes/form_component.html.erb
@@ -121,12 +121,12 @@
       <h3><%= t("admin.legislation.processes.form.banner_title") %></h3>
     </div>
 
-    <div class="small-6 large-3 column">
-      <%= f.label :background_color, nil, for: "background_color_input" %>
+    <div class="small-6 large-3 column background-color-inputs">
+      <%= f.label :background_color, nil, for: "background_color_input", id: "background_color_label" %>
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :background_color, label: false, type: :color %>
+          <%= f.text_field :background_color, label: false, type: :color, "aria-labelledby": "background_color_label" %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :background_color, label: false, id: "background_color_input" %>
@@ -134,12 +134,12 @@
       </div>
     </div>
 
-    <div class="small-6 large-3 column end">
-      <%= f.label :font_color, nil, for: "font_color_input" %>
+    <div class="small-6 large-3 column end font-color-inputs">
+      <%= f.label :font_color, nil, for: "font_color_input", id: "font_color_label" %>
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :font_color, label: false, type: :color %>
+          <%= f.text_field :font_color, label: false, type: :color, "aria-labelledby": "font_color_label" %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :font_color, label: false, id: "font_color_input" %>
diff --git a/spec/components/admin/banners/form_component_spec.rb b/spec/components/admin/banners/form_component_spec.rb
new file mode 100644
index 000000000..ccfae6b05
--- /dev/null
+++ b/spec/components/admin/banners/form_component_spec.rb
@@ -0,0 +1,38 @@
+require "rails_helper"
+
+describe Admin::Banners::FormComponent do
+  let(:banner) { build(:banner) }
+  let(:component) { Admin::Banners::FormComponent.new(banner) }
+
+  describe "background color fields" do
+    it "renders two inputs sharing the same label" do
+      render_inline component
+
+      page.find(".background-color-inputs") do |color_inputs|
+        expect(color_inputs).to have_css "input", count: 2
+        expect(color_inputs).to have_css "label", count: 1
+
+        expect(color_inputs).to have_css "label#background_color_label[for=background_color_input]"
+        expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "background_color_label"
+        expect(color_inputs.all("input")[0][:id]).not_to eq "background_color_input"
+        expect(color_inputs.all("input")[1][:id]).to eq "background_color_input"
+      end
+    end
+  end
+
+  describe "font color fields" do
+    it "renders two inputs sharing the same label" do
+      render_inline component
+
+      page.find(".font-color-inputs") do |color_inputs|
+        expect(color_inputs).to have_css "input", count: 2
+        expect(color_inputs).to have_css "label", count: 1
+
+        expect(color_inputs).to have_css "label#font_color_label[for=font_color_input]"
+        expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "font_color_label"
+        expect(color_inputs.all("input")[0][:id]).not_to eq "font_color_input"
+        expect(color_inputs.all("input")[1][:id]).to eq "font_color_input"
+      end
+    end
+  end
+end
diff --git a/spec/components/admin/geozones/form_component_spec.rb b/spec/components/admin/geozones/form_component_spec.rb
new file mode 100644
index 000000000..cd236efcb
--- /dev/null
+++ b/spec/components/admin/geozones/form_component_spec.rb
@@ -0,0 +1,22 @@
+require "rails_helper"
+
+describe Admin::Geozones::FormComponent do
+  let(:geozone) { build(:geozone) }
+  let(:component) { Admin::Geozones::FormComponent.new(geozone) }
+
+  describe "color fields" do
+    it "renders two inputs sharing the same label" do
+      render_inline component
+
+      page.find(".color-inputs") do |color_inputs|
+        expect(color_inputs).to have_css "input", count: 2
+        expect(color_inputs).to have_css "label", count: 1
+
+        expect(color_inputs).to have_css "label#color_label[for=color_input]"
+        expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "color_label"
+        expect(color_inputs.all("input")[0][:id]).not_to eq "color_input"
+        expect(color_inputs.all("input")[1][:id]).to eq "color_input"
+      end
+    end
+  end
+end
diff --git a/spec/components/admin/legislation/processes/form_component_spec.rb b/spec/components/admin/legislation/processes/form_component_spec.rb
new file mode 100644
index 000000000..dbc6204ef
--- /dev/null
+++ b/spec/components/admin/legislation/processes/form_component_spec.rb
@@ -0,0 +1,38 @@
+require "rails_helper"
+
+describe Admin::Legislation::Processes::FormComponent, :admin do
+  let(:process) { build(:legislation_process) }
+  let(:component) { Admin::Legislation::Processes::FormComponent.new(process) }
+
+  describe "background color fields" do
+    it "renders two inputs sharing the same label" do
+      render_inline component
+
+      page.find(".background-color-inputs") do |color_inputs|
+        expect(color_inputs).to have_css "input", count: 2
+        expect(color_inputs).to have_css "label", count: 1
+
+        expect(color_inputs).to have_css "label#background_color_label[for=background_color_input]"
+        expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "background_color_label"
+        expect(color_inputs.all("input")[0][:id]).not_to eq "background_color_input"
+        expect(color_inputs.all("input")[1][:id]).to eq "background_color_input"
+      end
+    end
+  end
+
+  describe "font color fields" do
+    it "renders two inputs sharing the same label" do
+      render_inline component
+
+      page.find(".font-color-inputs") do |color_inputs|
+        expect(color_inputs).to have_css "input", count: 2
+        expect(color_inputs).to have_css "label", count: 1
+
+        expect(color_inputs).to have_css "label#font_color_label[for=font_color_input]"
+        expect(color_inputs.all("input")[0][:"aria-labelledby"]).to eq "font_color_label"
+        expect(color_inputs.all("input")[0][:id]).not_to eq "font_color_input"
+        expect(color_inputs.all("input")[1][:id]).to eq "font_color_input"
+      end
+    end
+  end
+end