296 Initial pass at inline errors for eval form

GSA · Dec 17, 2024 · bfd87c5 · bfd87c5
1 parent 348d3af
commit bfd87c5
Show file tree

Hide file tree

Showing 12 changed files with 260 additions and 233 deletions.
diff --git a/app/assets/stylesheets/application.sass.scss b/app/assets/stylesheets/application.sass.scss
@@ -21,3 +21,7 @@ dialog::backdrop {
 #add-criteria-button.usa-button {
   background-color: #4d8055;
 }
+
+.usa-combo-box .border-secondary + input {
+  border: 1px solid red;
+}
diff --git a/app/controllers/evaluation_forms_controller.rb b/app/controllers/evaluation_forms_controller.rb
@@ -2,6 +2,8 @@
 
 # Controller for evaluation forms CRUD actions.
 class EvaluationFormsController < ApplicationController
+  helper FormHelper
+
   before_action -> { authorize_user('challenge_manager') }
   before_action :set_evaluation_form, only: %i[show edit update destroy]
   before_action :set_evaluation_forms, only: %i[index]

diff --git a/app/helpers/evaluation_forms_helper.rb b/app/helpers/evaluation_forms_helper.rb
@@ -29,11 +29,6 @@ def evaluation_period(evaluation_form)
     "#{start_date} - #{end_date}"
   end
 
-  def inline_error(evaluation_form, field)
-    error = evaluation_form.errors[field].present? ? evaluation_form.errors[field].first : ""
-    tag.span(error, class: "text-secondary font-body-2xs", id: "evaluation_form_#{field}_error")
-  end
-
   def criteria_field_id(form, attribute, is_template)
     prefix = "evaluation_form_evaluation_criteria_attributes"
 

diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+# Helpers for rendering various form elements and errors
+module FormHelper
+  def label_error_class(form, field)
+    object = form.object
+    object.errors[field].present? ? "text-secondary" : ""
+  end
+
+  def input_error_class(form, field)
+    object = form.object
+    object.errors[field].present? ? "border-secondary" : ""
+  end
+
+  def inline_error(form, field)
+    object = form.object
+    field_id = (form.object_name + "_#{field}_error").gsub(/[\[\]]/, "_").squeeze('_')
+    error = object.errors[field].present? ? object.errors[field].first : ""
+
+    tag.span(error, class: "text-secondary font-body-2xs", id: field_id)
+  end
+end
diff --git a/app/javascript/controllers/evaluation_criteria_controller.js b/app/javascript/controllers/evaluation_criteria_controller.js
@@ -171,7 +171,7 @@ export default class extends Controller {
       .querySelectorAll(".criteria-option-label-row")
       .forEach((labelRow, index) => {
         labelRow.style.display =
-          index >= start && index <= end ? "flex" : "none";
+          index >= start && index <= end ? "block" : "none";
         const input = labelRow.querySelector("input");
         input.disabled = index < start || index > end;
       });

diff --git a/app/javascript/controllers/evaluation_form_controller.js b/app/javascript/controllers/evaluation_form_controller.js
@@ -21,11 +21,10 @@ export default class extends Controller {
         "data-min-date",
         `${year}-${month}-${day}`
       );
-
-      this.updateErrorMessage("evaluation_form_challenge_id", "");
-      this.updateErrorMessage("evaluation_form_phase_id", "");
     } else {
-      this.updateErrorMessage("evaluation_form_challenge_id", "can't be blank");
+      this.challengeIDTarget.value = null;
+      this.phaseIDTarget.value = null;
+
       this.startDateTarget.innerHTML = "mm/dd/yyyy";
     }
   }
@@ -72,12 +71,29 @@ export default class extends Controller {
   }
 
   validatePresence(e) {
-    if (!e.target.value) {
-      e.target.classList.add("border-secondary");
-      this.updateErrorMessage(e.target.id, "can't be blank");
+    const target = e.target;
+    const formGroup = target.closest(".usa-form-group");
+    const fieldName = target.dataset.fieldName;
+
+    const isSelect =
+      target.tagName === "SELECT" ||
+      target.classList.contains("usa-combo-box__input");
+
+    const isRadio = target.type === "radio";
+
+    const labelId = isSelect ? target.name : target.id;
+    const labelQuery = isRadio ? "legend" : `label[for="${labelId}"]`;
+
+    const label = formGroup.querySelector(labelQuery);
+
+    if (!target.value) {
+      target.classList.add("border-secondary");
+      if (label) label.classList.add("text-secondary");
+      this.updateErrorMessage(fieldName || target.id, "can't be blank");
     } else {
-      e.target.classList.remove("border-secondary");
-      this.updateErrorMessage(e.target.id, "");
+      target.classList.remove("border-secondary");
+      if (label) label.classList.remove("text-secondary");
+      this.updateErrorMessage(fieldName || target.id, "");
     }
   }
 

diff --git a/app/models/evaluation_criterion.rb b/app/models/evaluation_criterion.rb
@@ -37,4 +37,19 @@ class EvaluationCriterion < ApplicationRecord
   validates :title, length: { maximum: 150 }
   validates :description, length: { maximum: 1000 }
   validates :points_or_weight, numericality: { only_integer: true }
+  validates :scoring_type, presence: true
+
+  validate :validate_option_labels_not_blank, if: -> { rating? || binary? }
+
+  private
+
+  def validate_option_labels_not_blank
+    return unless option_labels.is_a?(Hash)
+
+    option_labels.each do |key, value|
+      if value.blank?
+        errors.add("option_labels_#{key}", "can't be blank")
+      end
+    end
+  end
 end
diff --git a/app/models/evaluation_form.rb b/app/models/evaluation_form.rb
@@ -48,7 +48,9 @@ def validate_unique_criteria_titles
   end
 
   def criteria_weights_must_sum_to_one_hundred
-    total_weight = evaluation_criteria.reject(&:marked_for_destruction?).sum(&:points_or_weight)
+    total_weight = evaluation_criteria.reject(&:marked_for_destruction?).sum do |criteria|
+      criteria.points_or_weight.to_i
+    end
 
     return unless weighted_scoring? && total_weight != 100
 

diff --git a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb
@@ -18,100 +18,113 @@
   <% end %>
 
   <div class="usa-form-group margin-top-0">
-    <%= f.label :title, "Criteria title", for: criteria_field_id(f, "title", is_template), class: "text-bold" %>
-    <span style="color: darkred;">*</span>
-    <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
+    <div class="display-flex flex-align-center text-bold">
+      <%= f.label :title, "Criteria title", for: criteria_field_id(f, "title", is_template), class: "margin-right-05 #{label_error_class(f, :title)}" %>
+      <span class="margin-right-05" style="color: darkred;">*</span>
+      <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
+    </div>
 
     <%= f.text_field :title, 
       id: criteria_field_id(f, "title", is_template),
       name: criteria_field_name(f, "title", is_template),
-      class: "usa-input", 
-      placeholder: "Add criteria title here", 
+      class: "usa-input #{input_error_class(f, :title)}",
       maxlength: 150,
-      required: true, 
       disabled: is_template || form_disabled,
-      data: {"evaluation-criteria-target": "titleField"}
+      data: {
+        "evaluation-criteria-target": "titleField",
+        "action": "evaluation-form#validatePresence focusout->evaluation-form#validatePresence"
+        }
     %>
+
+    <%= inline_error(f, :title) %>
   </div>
 
-  <div class="usa-character-count">
-    <div class="usa-form-group">
-      <%= f.label :description, "Criteria description", for: criteria_field_id(f, "description", is_template), class: "text-bold" %>
-      <span style="color: darkred;">*</span>
-      <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
+  <div class="usa-form-group">
+    <div class="usa-character-count">
+      <div class="display-flex flex-align-center text-bold">
+        <%= f.label :description, "Criteria description", for: criteria_field_id(f, "description", is_template), class: "margin-right-05 #{label_error_class(f, :description)}" %>
+        <span class="margin-right-05" style="color: darkred;">*</span>
+        <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
+      </div>
 
       <%= f.text_area :description, 
         id: criteria_field_id(f, "description", is_template),
         name: criteria_field_name(f, "description", is_template),
-        class: "usa-textarea usa-character-count__field", 
-        placeholder: "Add criteria description here", 
+        class: "usa-textarea usa-character-count__field #{input_error_class(f, :description)}",
         maxlength: 1000,
-        required: true, 
         disabled: is_template || form_disabled,
-        data: {"evaluation-criteria-target": "descriptionField"}
+        data: {
+          "evaluation-criteria-target": "descriptionField",
+          "action": "evaluation-form#validatePresence focusout->evaluation-form#validatePresence"
+        }
       %>
+      <span class="usa-character-count__message">You can enter up to 1000 characters</span>
     </div>
-    <span class="usa-character-count__message">You can enter up to 1000 characters</span>
+
+    <%= inline_error(f, :description) %>
   </div>
 
-  <div class="usa-form-group display-flex flex-align-center">
+  <div class="usa-form-group">
+    <div class="display-flex flex-align-center text-bold">
+      <%= f.label :points_or_weight, "Criteria Points/Weight", for: criteria_field_id(f, "points_or_weight", is_template), class: "margin-right-05 #{label_error_class(f, :points_or_weight)}" %>
+      <span class="margin-right-05" style="color: darkred;">*</span>
+      <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
+    </div>
+
     <%= f.number_field :points_or_weight, 
       id: criteria_field_id(f, "points_or_weight", is_template),
       name: criteria_field_name(f, "points_or_weight", is_template),
-      class: "points-or-weight usa-input flex-1 margin-top-0 margin-right-2 font-sans-lg",
+      class: "points-or-weight usa-input #{input_error_class(f, :points_or_weight)}",
       min: 1, 
       max: f.object.evaluation_form&.weighted_scoring? ? 100 : 9999,
       step: 1, 
-      placeholder: "Add criteria points/weight here",
-      required: true, 
       disabled: is_template || form_disabled,
       data: { 
         "evaluation-criteria-target": "pointsOrWeightField",
-        action: "input->evaluation-criteria#checkPointsOrWeightMax blur->evaluation-criteria#checkPointsOrWeightMax"
+        action: "
+          input->evaluation-criteria#checkPointsOrWeightMax blur->evaluation-criteria#checkPointsOrWeightMax
+          evaluation-form#validatePresence focusout->evaluation-form#validatePresence
+        "
       }
     %>
 
-    <div class="flex-7 font-sans-lg">
-      <%= f.label :points_or_weight, "Criteria Points/Weight", 
-        for: criteria_field_id(f, "points_or_weight", is_template),
-        class: "text-bold"
-      %>
-
-      <span style="color: darkred;">*</span>
-
-      <%= image_tag "images/usa-icons/help_outline.svg", 
-        width: 16, height: 16, alt: "Help for criteria title", 
-        class: "flex-0" 
-      %>
-    </div>
+    <%= inline_error(f, :points_or_weight) %>
   </div>
 
   <div class="usa-form-group">
-    <%= f.label :scoring_type, "Scoring Type", for: criteria_field_id(f, "scoring_type", is_template), class: "font-sans-lg text-bold" %>
-    <span style="color: darkred;">*</span>
-    <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
-
-    <% scoring_types = { "numeric" => "Numeric score / points", "rating" => "Rating scale", "binary" => "Binary scale" } %>
-    <div>
-      <% scoring_types.each_with_index do |(value, label), index| %>
-        <div class="usa-radio">
-          <%= f.radio_button :scoring_type, value, 
-              id: criteria_field_id(f, "scoring_type_#{value}", is_template), 
-              name: criteria_field_name(f, :scoring_type, is_template),
-              class: "usa-radio__input scoring-type-radio",
-              checked: f.object.scoring_type == value,
-              required: index == 0,
-              disabled: is_template || form_disabled,
-              data: {
-                "evaluation-criteria-target": "scoringTypeRadio",
-                action: "click->evaluation-criteria#toggleScoringType"
-              }
-          %>
+    <fieldset class="usa-fieldset margin-top-205">
+      <legend class="usa-legend display-flex flex-align-center text-bold <%= label_error_class(f, :scoring_type) %>">
+        <span class="margin-right-05">Scoring type</span>
+        <span class="margin-right-05" style="color: darkred;">*</span>
+        <%= image_tag "images/usa-icons/help_outline.svg", width: 16, height: 16, alt: "Help for criteria title" %>
+      </legend>
+
+      <% scoring_types = { "numeric" => "Numeric score / points", "rating" => "Rating scale", "binary" => "Binary scale" } %>
+      <div>
+        <% scoring_types.each_with_index do |(value, label), index| %>
+          <div class="usa-radio">
+            <%= f.radio_button :scoring_type, value,
+                id: criteria_field_id(f, "scoring_type_#{value}", is_template),
+                name: criteria_field_name(f, :scoring_type, is_template),
+                class: "usa-radio__input scoring-type-radio",
+                checked: f.object.scoring_type == value,
+                disabled: is_template || form_disabled,
+                data: {
+                  "evaluation-criteria-target": "scoringTypeRadio",
+                  action: "click->evaluation-criteria#toggleScoringType evaluation-form#validatePresence",
+                  field_name: criteria_field_id(f, "scoring_type", is_template)
+                }
+            %>
+
+            <%= label_tag criteria_field_id(f, "scoring_type_#{value}", is_template), label, class: "usa-radio__label" %>
+          </div>
+        <% end %>
+      </div>
 
-          <%= label_tag criteria_field_id(f, "scoring_type_#{value}", is_template), label, class: "usa-radio__label" %>
-        </div>
-      <% end %>
-    </div>
+      <div class="margin-top-1">
+        <%= inline_error(f, :scoring_type) %>
+      </div>
+    </fieldset>
   </div>
 
   <div class="criteria-scale-options usa-form-group" style="<%= f.object.numeric? || !f.object.scoring_type.present? || is_template ? 'display: none;' : '' %>">
@@ -163,20 +176,26 @@
       <% (0..10).each do |index| %>
         <% disabledOptionLabel = is_template || !f.object.scoring_type.present? || f.object.numeric? || ((f.object.option_range_start && index < f.object.option_range_start) || (f.object.option_range_end && index > f.object.option_range_end)) %>
 
-        <div class="<%= disabledOptionLabel ? 'display-none' : 'display-flex' %> flex-align-center flex-justify margin-top-2 criteria-option-label-row"
-          data-evaluation-criteria-target="optionLabelRow">
-          <%= label_tag criteria_field_id(f, "option_labels", is_template) ++ "_#{index}", index, class: "font-sans-lg text-bold text-base margin-right-2" %>
-
-          <%= f.text_field "option_labels", 
-            id: criteria_field_id(f, "option_labels", is_template) ++ "_#{index}",
-            name: criteria_field_name(f, "option_labels", is_template) ++ "[#{index}]",
-            class: "usa-input margin-0",
-            value: f.object.option_labels[index.to_s],
-            required: true, 
-            disabled: disabledOptionLabel || form_disabled,
-            data: {"evaluation-criteria-target": "optionLabelInput"}
-          %>
+        <div class="<%= disabledOptionLabel ? 'display-none' : 'display' %> margin-top-2 criteria-option-label-row" data-evaluation-criteria-target="optionLabelRow">
+          <div class="display-flex flex-align-center flex-justify">
+            <%= label_tag criteria_field_id(f, "option_labels", is_template) ++ "_#{index}", index, class: "font-sans-lg text-bold text-base margin-right-2 #{label_error_class(f, "option_labels_#{index}")}" %>
+
+            <%= f.text_field "option_labels",
+              id: criteria_field_id(f, "option_labels", is_template) ++ "_#{index}",
+              name: criteria_field_name(f, "option_labels", is_template) ++ "[#{index}]",
+              class: "usa-input margin-0 #{input_error_class(f, "option_labels_#{index}")}",
+              value: f.object.option_labels[index.to_s],
+              disabled: disabledOptionLabel || form_disabled,
+              data: {
+                "action": "evaluation-form#validatePresence focusout->evaluation-form#validatePresence",
+                "evaluation-criteria-target": "optionLabelInput"
+              }
+            %>
+          </div>
+
+          <%= inline_error(f, "option_labels_#{index}") %>
         </div>
+
       <% end %>
     </div>
   </div>