From 988abf7a8ce4698a7372fea2ff877dc1e7a8a759 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Thu, 17 Sep 2020 08:27:45 -0700
Subject: [PATCH] update preference field interface to be more clear

---
 app/helpers/spree/admin/base_helper.rb             | 14 +++++++++++---
 .../enterprise_fees/_calculator_settings.html.haml |  4 ++--
 .../payment_methods/_provider_settings.html.haml   |  4 ++--
 .../admin/shared/_calculator_fields.html.haml      |  4 ++--
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/app/helpers/spree/admin/base_helper.rb b/app/helpers/spree/admin/base_helper.rb
index 6fb6dc4ba6..7615f2b6c9 100644
--- a/app/helpers/spree/admin/base_helper.rb
+++ b/app/helpers/spree/admin/base_helper.rb
@@ -62,6 +62,10 @@ module Spree
         end
       end
 
+      # Here we show a text field for all string fields except when the field name ends in
+      # "_from_list", in that case we render a dropdown.
+      # In this specific case, to render the dropdown, the object provided must have a method named
+      # like "#{field}_values" that returns an array with the string options to be listed.
       def preference_field_for_text_field(form, field, options, object)
         if field.end_with?('_from_list') && object.respond_to?("#{field}_values")
           list_values = object.__send__("#{field}_values")
@@ -102,13 +106,17 @@ module Spree
         )
       end
 
+      # maps each preference to a hash containing the label and field html.
+      # E.g. { :label => "<label>...", :field => "<select>..." }
       def preference_fields(object, form)
         return unless object.respond_to?(:preferences)
 
         object.preferences.keys.map { |key|
-          [form.label("preferred_#{key}", Spree.t(key.to_s.gsub("_from_list", "")) + ": "),
-            preference_field_for(form, "preferred_#{key}",
-                                 { type: object.preference_type(key) }, object)]
+          preference_label = form.label("preferred_#{key}",
+            Spree.t(key.to_s.gsub("_from_list", "")) + ": ").html_safe
+          preference_field = preference_field_for(form, "preferred_#{key}",
+            { type: object.preference_type(key) }, object).html_safe
+          { :label => preference_label, :field => preference_field }
         }
       end
 
diff --git a/app/views/admin/enterprise_fees/_calculator_settings.html.haml b/app/views/admin/enterprise_fees/_calculator_settings.html.haml
index 415992a008..6c13962ed0 100644
--- a/app/views/admin/enterprise_fees/_calculator_settings.html.haml
+++ b/app/views/admin/enterprise_fees/_calculator_settings.html.haml
@@ -9,7 +9,7 @@
           = f.fields_for :calculator do |calculator_form|
             - fields = preference_fields(enterprise_fee.calculator, calculator_form)
             - fields.each do |f|
-              = f[0].html_safe
-              = f[1].html_safe
+              = f[:label]
+              = f[:field]
 
 = calculator_form_string
diff --git a/app/views/spree/admin/payment_methods/_provider_settings.html.haml b/app/views/spree/admin/payment_methods/_provider_settings.html.haml
index bf582636a7..7c7c5177b4 100644
--- a/app/views/spree/admin/payment_methods/_provider_settings.html.haml
+++ b/app/views/spree/admin/payment_methods/_provider_settings.html.haml
@@ -13,6 +13,6 @@
           - fields = preference_fields(@payment_method, payment_method_form)
           - fields.each do |f|
             .field.alpha.three.columns
-              = f[0].html_safe
+              = f[:label]
             .field.omega.eight.columns
-              = f[1].html_safe
+              = f[:field]
diff --git a/app/views/spree/admin/shared/_calculator_fields.html.haml b/app/views/spree/admin/shared/_calculator_fields.html.haml
index cafdf01e3c..0165750f25 100644
--- a/app/views/spree/admin/shared/_calculator_fields.html.haml
+++ b/app/views/spree/admin/shared/_calculator_fields.html.haml
@@ -13,8 +13,8 @@
             - fields = preference_fields(@object.calculator, calculator_form)
             - fields.each do |f|
               .field.alpha.three.columns
-                = f[0].html_safe
+                = f[:label]
               .field.omega.eight.columns
-                = f[1].html_safe
+                = f[:field]
         - if @object.calculator.respond_to?(:preferences)
           %span.calculator-settings-warning.info.warning= t(:calculator_settings_warning)