From 331228cb2a1ef44b67a9254a28ad3f72f4417cad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 10 Oct 2024 22:30:09 +0200
Subject: [PATCH] Add proper labels for site customization texts

We were rendering one label and many textarea fields for that label.
This meant that, when switching to a different language, the label
wasn't correctly associated with the textarea.

So we're now rendering one label for each textarea. We could use
`aria-label` or `aria-labelledby` instead, but using a label offers some
advantages like the fact that clicking on the label makes the textarea
take the focus.
---
 .../information_texts/form_component.html.erb          |  1 -
 .../information_texts/form_field_component.html.erb    | 10 ++++------
 .../information_texts/form_field_component.rb          | 10 +++++++++-
 spec/system/admin/translatable_spec.rb                 |  4 ++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/app/components/admin/site_customization/information_texts/form_component.html.erb b/app/components/admin/site_customization/information_texts/form_component.html.erb
index d9b1bb245..044bd1019 100644
--- a/app/components/admin/site_customization/information_texts/form_component.html.erb
+++ b/app/components/admin/site_customization/information_texts/form_component.html.erb
@@ -6,7 +6,6 @@
   <% end %>
   <% contents.each do |group| %>
     <% group.each do |content| %>
-      <b><%= content.key %></b>
       <%= hidden_field_tag "contents[content_#{content.key}][id]", content.key %>
       <% (content.globalize_locales & enabled_locales.map(&:to_sym)).each do |locale| %>
         <%= render Admin::SiteCustomization::InformationTexts::FormFieldComponent.new(content, locale: locale) %>
diff --git a/app/components/admin/site_customization/information_texts/form_field_component.html.erb b/app/components/admin/site_customization/information_texts/form_field_component.html.erb
index ddc552f6d..1e546cf19 100644
--- a/app/components/admin/site_customization/information_texts/form_field_component.html.erb
+++ b/app/components/admin/site_customization/information_texts/form_field_component.html.erb
@@ -1,8 +1,6 @@
 <% globalize(locale) do %>
-  <%= text_area_tag "contents[content_#{i18n_key}]values[value_#{locale}]",
-                    text,
-                    rows: 5,
-                    class: "js-globalize-attribute",
-                    style: site_customization_display_translation_style,
-                    data: { locale: locale } %>
+  <div class="js-globalize-attribute" data-locale="<%= locale %>" style="<%= display_style %>">
+    <%= label_tag html_id, i18n_key %>
+    <%= text_area_tag html_name, text, id: html_id, rows: 5 %>
+  </div>
 <% end %>
diff --git a/app/components/admin/site_customization/information_texts/form_field_component.rb b/app/components/admin/site_customization/information_texts/form_field_component.rb
index d000471e2..046630722 100644
--- a/app/components/admin/site_customization/information_texts/form_field_component.rb
+++ b/app/components/admin/site_customization/information_texts/form_field_component.rb
@@ -27,7 +27,15 @@ class Admin::SiteCustomization::InformationTexts::FormFieldComponent < Applicati
       i18n_content.key
     end
 
-    def site_customization_display_translation_style
+    def html_id
+      html_name.tr("[", "_").tr("]", "")
+    end
+
+    def html_name
+      "contents[content_#{i18n_key}]values[value_#{locale}]"
+    end
+
+    def display_style
       site_customization_enable_translation?(locale) ? "" : "display: none;"
     end
 end
diff --git a/spec/system/admin/translatable_spec.rb b/spec/system/admin/translatable_spec.rb
index c9deea0e1..f9bc3dcc9 100644
--- a/spec/system/admin/translatable_spec.rb
+++ b/spec/system/admin/translatable_spec.rb
@@ -496,11 +496,11 @@ describe "Admin edit translatable records", :admin do
     scenario "Show selected locale form" do
       visit path
 
-      expect(page).to have_field "contents_content_#{content.key}values_value_en", with: "Value in English"
+      expect(page).to have_field content.key, with: "Value in English"
 
       select "Español", from: "Current language"
 
-      expect(page).to have_field "contents_content_#{content.key}values_value_es", with: "Value en español"
+      expect(page).to have_field content.key, with: "Value en español"
     end
 
     scenario "Select a locale and add it to the form" do