From 6029870e5f13377cca1c38fe397dba0107f31ca3 Mon Sep 17 00:00:00 2001 From: Chris Preisinger Date: Mon, 25 Nov 2024 00:32:38 -0500 Subject: [PATCH 1/4] 275 Fix for max input values for scale types --- .../evaluation_criteria_controller.js | 12 +++ .../controllers/evaluation_form_controller.js | 79 +++++++++++++------ .../_evaluation_criterion_fields.html.erb | 5 +- app/views/evaluation_forms/_form.html.erb | 4 +- spec/system/evaluation_form_spec.rb | 58 ++++++++++++++ 5 files changed, 131 insertions(+), 27 deletions(-) diff --git a/app/javascript/controllers/evaluation_criteria_controller.js b/app/javascript/controllers/evaluation_criteria_controller.js index 964dd898..e0b8e26b 100644 --- a/app/javascript/controllers/evaluation_criteria_controller.js +++ b/app/javascript/controllers/evaluation_criteria_controller.js @@ -100,6 +100,18 @@ export default class extends Controller { }); } + checkPointsOrWeightMax(event) { + const input = event.target; + const min = input.min; + const max = input.max; + const value = input.value; + + // If field has a value on blur then check if it's valid + if ((value && value < min) || value > max) { + event.target.reportValidity(); + } + } + updateScoringOptions(row, scoringType) { const options = { scaleOptions: row.querySelector(".criteria-scale-options"), diff --git a/app/javascript/controllers/evaluation_form_controller.js b/app/javascript/controllers/evaluation_form_controller.js index 90605314..b2b79ce6 100644 --- a/app/javascript/controllers/evaluation_form_controller.js +++ b/app/javascript/controllers/evaluation_form_controller.js @@ -1,54 +1,85 @@ -import { Controller } from "@hotwired/stimulus" +import { Controller } from "@hotwired/stimulus"; // Connects to data-controller="evaluation-form" export default class extends Controller { static targets = ["challengeID", "phaseID", "startDate", "datePicker"]; handleChallengeSelect(e) { - let id, phase_id, end_date - [id, phase_id, end_date] = e.target.value.split(".") + let id, phase_id, end_date; + [id, phase_id, end_date] = e.target.value.split("."); if (id) { - // set values of hidden form fields - this.challengeIDTarget.value = id - this.phaseIDTarget.value = phase_id + // set values of hidden form fields + this.challengeIDTarget.value = id; + this.phaseIDTarget.value = phase_id; - // set the start date of the evaluation form + // set the start date of the evaluation form // to be the challenge's end date - this.startDateTarget.innerHTML = end_date || "mm/dd/yyyy" - let day, month, year - [month, day, year] = end_date.split("/") - this.datePickerTarget.setAttribute("data-min-date", `${year}-${month}-${day}`) + this.startDateTarget.innerHTML = end_date || "mm/dd/yyyy"; + let day, month, year; + [month, day, year] = end_date.split("/"); + this.datePickerTarget.setAttribute( + "data-min-date", + `${year}-${month}-${day}` + ); - this.updateErrorMessage("evaluation_form_challenge_id", "") - this.updateErrorMessage("evaluation_form_phase_id", "") + this.updateErrorMessage("evaluation_form_challenge_id", ""); + this.updateErrorMessage("evaluation_form_phase_id", ""); } else { - this.updateErrorMessage("evaluation_form_challenge_id", "can't be blank") - this.startDateTarget.innerHTML = "mm/dd/yyyy" + this.updateErrorMessage("evaluation_form_challenge_id", "can't be blank"); + this.startDateTarget.innerHTML = "mm/dd/yyyy"; } } + // Opens all accordions, remove existing points/weights, update max points/weights values updateMaxPoints(e) { const form = e.target.closest('form[data-controller="evaluation-form"]'); const pointsWeights = form.querySelectorAll(".points-or-weight"); - if (e.target.id == 'weighted_scale') { - pointsWeights.forEach((input) => input.max = "100") + const weightedScale = e.target.value == "true"; + + // Check if any input has a value over 100 + const hasValuesOver100 = Array.from(pointsWeights).some( + (input) => parseInt(input.value.trim()) > 100 + ); + + // Display confirmation dialog if switching to weighted and any point inputs have a value + if (weightedScale && hasValuesOver100) { + const confirmed = window.confirm( + "You have values over 100. Changing the scale type to weighted will reset your weight values" + ); + if (!confirmed) { + e.preventDefault(); + return; + } + + pointsWeights.forEach((input) => (input.value = "")); + } + + if (e.target.id == "weighted_scale") { + pointsWeights.forEach((input) => (input.max = "100")); } else { - pointsWeights.forEach((input) => input.max = "9999") + pointsWeights.forEach((input) => (input.max = "9999")); } + + const accordionButtons = form.querySelectorAll(".usa-accordion__button"); + accordionButtons.forEach((accordionButton) => + accordionButton.setAttribute("aria-expanded", true) + ); + + const accordions = form.querySelectorAll(".usa-accordion__content"); + accordions.forEach((accordion) => accordion.removeAttribute("hidden")); } validatePresence(e) { if (!e.target.value) { - e.target.classList.add("border-secondary") - this.updateErrorMessage(e.target.id, "can't be blank") - + e.target.classList.add("border-secondary"); + this.updateErrorMessage(e.target.id, "can't be blank"); } else { - e.target.classList.remove("border-secondary") - this.updateErrorMessage(e.target.id, "") + e.target.classList.remove("border-secondary"); + this.updateErrorMessage(e.target.id, ""); } } updateErrorMessage(field, message) { - document.getElementById(field + "_error").innerHTML = message + document.getElementById(field + "_error").innerHTML = message; } } diff --git a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb index f324f4ba..256a81f2 100644 --- a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb +++ b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb @@ -65,7 +65,10 @@ placeholder: "Add criteria points/weight here", required: true, disabled: is_template || form_disabled, - data: {"evaluation-criteria-target": "pointsOrWeightField"} + data: { + "evaluation-criteria-target": "pointsOrWeightField", + action: "blur->evaluation-criteria#checkPointsOrWeightMax" + } %>
diff --git a/app/views/evaluation_forms/_form.html.erb b/app/views/evaluation_forms/_form.html.erb index e717647b..ce1708a3 100644 --- a/app/views/evaluation_forms/_form.html.erb +++ b/app/views/evaluation_forms/_form.html.erb @@ -98,7 +98,7 @@ type="radio" name="evaluation_form[weighted_scoring]" value="false" - data-action="input->evaluation-form#updateMaxPoints" + data-action="click->evaluation-form#updateMaxPoints" <%= 'checked' if !evaluation_form.weighted_scoring? && evaluation_form.persisted? %> <%= 'disabled' if disabled %> required @@ -112,7 +112,7 @@ type="radio" name="evaluation_form[weighted_scoring]" value="true" - data-action="input->evaluation-form#updateMaxPoints" + data-action="click->evaluation-form#updateMaxPoints" <%= 'checked' if evaluation_form.weighted_scoring? %> <%= 'disabled' if disabled %> > diff --git a/spec/system/evaluation_form_spec.rb b/spec/system/evaluation_form_spec.rb index e386e5e5..6b6b8a44 100644 --- a/spec/system/evaluation_form_spec.rb +++ b/spec/system/evaluation_form_spec.rb @@ -184,6 +184,64 @@ save_form expect(page).to have_content("Evaluation Form Saved") end + + it "expands all criteria and resets values if switching to weighted scale with value over 100" do + visit new_evaluation_form_path + + fill_in_base_form_info + select_scale_type("point") + + # Fill in criteria with one being over 100 + fill_in_numeric_criteria_type(initial: true) + fill_in_criterion_points_weight(0, 10) + fill_in_numeric_criteria_type + fill_in_criterion_points_weight(1, 101) + + toggle_all_criteria_accordions(open: false) + + check_criteria_accordion_expanded(0, false) + check_criteria_accordion_expanded(1, false) + + select_scale_type("weighted") + accept_confirm + + check_criteria_accordion_expanded(0, true) + check_criteria_accordion_expanded(1, true) + + # Scale type should be weighted + expect_form_scale_type_to_equal(true) + expect_criterion_points_or_weight_to_equal(0, 0) + expect_criterion_points_or_weight_to_equal(1, 0) + end + + it "does nothing and prevents switching scale if switching to weighted scale with value over 100 and not accepting" do + visit new_evaluation_form_path + + fill_in_base_form_info + select_scale_type("point") + + # Fill in criteria with one being over 100 + fill_in_numeric_criteria_type(initial: true) + fill_in_criterion_points_weight(0, 10) + fill_in_numeric_criteria_type + fill_in_criterion_points_weight(1, 101) + + toggle_all_criteria_accordions(open: false) + + check_criteria_accordion_expanded(0, false) + check_criteria_accordion_expanded(1, false) + + select_scale_type("weighted") + dismiss_confirm + + check_criteria_accordion_expanded(0, false) + check_criteria_accordion_expanded(1, false) + + # Scale type should be pointed still + expect_form_scale_type_to_equal(false) + expect_criterion_points_or_weight_to_equal(0, 10) + expect_criterion_points_or_weight_to_equal(1, 101) + end end describe "update evaluation form page" do From 4e2a368eccec7565149084cbf5a8eb5256d657e2 Mon Sep 17 00:00:00 2001 From: Chris Preisinger Date: Mon, 25 Nov 2024 11:47:07 -0500 Subject: [PATCH 2/4] 275 Refactor updateMaxPoints function --- .../controllers/evaluation_form_controller.js | 60 ++++++++++++------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/app/javascript/controllers/evaluation_form_controller.js b/app/javascript/controllers/evaluation_form_controller.js index b2b79ce6..fbe94c4f 100644 --- a/app/javascript/controllers/evaluation_form_controller.js +++ b/app/javascript/controllers/evaluation_form_controller.js @@ -34,39 +34,53 @@ export default class extends Controller { updateMaxPoints(e) { const form = e.target.closest('form[data-controller="evaluation-form"]'); const pointsWeights = form.querySelectorAll(".points-or-weight"); - const weightedScale = e.target.value == "true"; + const weightedScale = e.target.value === "true"; - // Check if any input has a value over 100 - const hasValuesOver100 = Array.from(pointsWeights).some( - (input) => parseInt(input.value.trim()) > 100 - ); - - // Display confirmation dialog if switching to weighted and any point inputs have a value - if (weightedScale && hasValuesOver100) { - const confirmed = window.confirm( - "You have values over 100. Changing the scale type to weighted will reset your weight values" - ); - if (!confirmed) { + if (weightedScale && this.hasValuesOverLimit(pointsWeights, 100)) { + if (!this.confirmReset()) { e.preventDefault(); return; } - - pointsWeights.forEach((input) => (input.value = "")); + this.clearInputs(pointsWeights); + this.expandAllAccordions(form); } - if (e.target.id == "weighted_scale") { - pointsWeights.forEach((input) => (input.max = "100")); - } else { - pointsWeights.forEach((input) => (input.max = "9999")); - } + this.updateMaxValues(pointsWeights, weightedScale ? 100 : 9999); + } - const accordionButtons = form.querySelectorAll(".usa-accordion__button"); - accordionButtons.forEach((accordionButton) => - accordionButton.setAttribute("aria-expanded", true) + // Helper: Check if any input values exceed a given limit + hasValuesOverLimit(inputs, limit) { + return Array.from(inputs).some( + (input) => parseInt(input.value.trim()) > limit ); + } + // Helper: Show confirmation dialog for resetting values + confirmReset() { + return window.confirm( + "You have values over 100. Changing the scale type to weighted will reset your weight values." + ); + } + + // Helper: Clear all input values + clearInputs(inputs) { + inputs.forEach((input) => (input.value = "")); + } + + // Helper: Update max values for inputs + updateMaxValues(inputs, maxValue) { + inputs.forEach((input) => (input.max = maxValue)); + } + + // Helper: Expand all accordions + expandAllAccordions(form) { + const accordionButtons = form.querySelectorAll(".usa-accordion__button"); const accordions = form.querySelectorAll(".usa-accordion__content"); - accordions.forEach((accordion) => accordion.removeAttribute("hidden")); + + accordionButtons.forEach((button) => + button.setAttribute("aria-expanded", true) + ); + accordions.forEach((content) => content.removeAttribute("hidden")); } validatePresence(e) { From f4620044e2a20e652a088322a223ee106b34e7b9 Mon Sep 17 00:00:00 2001 From: Chris Preisinger Date: Tue, 26 Nov 2024 22:16:18 -0500 Subject: [PATCH 3/4] 275 Set input value to min or max if outside range --- .../controllers/evaluation_criteria_controller.js | 12 +++++++----- .../_evaluation_criterion_fields.html.erb | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/javascript/controllers/evaluation_criteria_controller.js b/app/javascript/controllers/evaluation_criteria_controller.js index e0b8e26b..7e0a1481 100644 --- a/app/javascript/controllers/evaluation_criteria_controller.js +++ b/app/javascript/controllers/evaluation_criteria_controller.js @@ -102,13 +102,15 @@ export default class extends Controller { checkPointsOrWeightMax(event) { const input = event.target; - const min = input.min; - const max = input.max; - const value = input.value; + const min = parseInt(input.min); + const max = parseInt(input.max); + const value = parseInt(input.value); // If field has a value on blur then check if it's valid - if ((value && value < min) || value > max) { - event.target.reportValidity(); + if (value && value < min) { + input.value = min; + } else if (value && value > max) { + input.value = max; } } diff --git a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb index 256a81f2..fd9a0f4a 100644 --- a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb +++ b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb @@ -67,7 +67,7 @@ disabled: is_template || form_disabled, data: { "evaluation-criteria-target": "pointsOrWeightField", - action: "blur->evaluation-criteria#checkPointsOrWeightMax" + action: "input->evaluation-criteria#checkPointsOrWeightMax" } %> From 1d923f7bca3d56885a2001d859d8cc5667f473ac Mon Sep 17 00:00:00 2001 From: Chris Preisinger Date: Wed, 27 Nov 2024 14:58:24 -0500 Subject: [PATCH 4/4] 275 No longer confirm or blank on scale switch --- .../evaluation_criteria_controller.js | 8 +++----- .../controllers/evaluation_form_controller.js | 20 +++---------------- .../_evaluation_criterion_fields.html.erb | 2 +- spec/system/evaluation_form_spec.rb | 20 +++++++++---------- 4 files changed, 17 insertions(+), 33 deletions(-) diff --git a/app/javascript/controllers/evaluation_criteria_controller.js b/app/javascript/controllers/evaluation_criteria_controller.js index 7e0a1481..97873799 100644 --- a/app/javascript/controllers/evaluation_criteria_controller.js +++ b/app/javascript/controllers/evaluation_criteria_controller.js @@ -106,11 +106,9 @@ export default class extends Controller { const max = parseInt(input.max); const value = parseInt(input.value); - // If field has a value on blur then check if it's valid - if (value && value < min) { - input.value = min; - } else if (value && value > max) { - input.value = max; + // If invalid value is entered then pop up error message + if (value && (value < min || value > max)) { + event.target.reportValidity(); } } diff --git a/app/javascript/controllers/evaluation_form_controller.js b/app/javascript/controllers/evaluation_form_controller.js index fbe94c4f..1d9fc0e0 100644 --- a/app/javascript/controllers/evaluation_form_controller.js +++ b/app/javascript/controllers/evaluation_form_controller.js @@ -37,11 +37,6 @@ export default class extends Controller { const weightedScale = e.target.value === "true"; if (weightedScale && this.hasValuesOverLimit(pointsWeights, 100)) { - if (!this.confirmReset()) { - e.preventDefault(); - return; - } - this.clearInputs(pointsWeights); this.expandAllAccordions(form); } @@ -55,21 +50,12 @@ export default class extends Controller { ); } - // Helper: Show confirmation dialog for resetting values - confirmReset() { - return window.confirm( - "You have values over 100. Changing the scale type to weighted will reset your weight values." - ); - } - - // Helper: Clear all input values - clearInputs(inputs) { - inputs.forEach((input) => (input.value = "")); - } - // Helper: Update max values for inputs updateMaxValues(inputs, maxValue) { inputs.forEach((input) => (input.max = maxValue)); + Array.from(inputs).every((input) => { + input.reportValidity(); + }); } // Helper: Expand all accordions diff --git a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb index fd9a0f4a..9250bafc 100644 --- a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb +++ b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb @@ -67,7 +67,7 @@ disabled: is_template || form_disabled, data: { "evaluation-criteria-target": "pointsOrWeightField", - action: "input->evaluation-criteria#checkPointsOrWeightMax" + action: "input->evaluation-criteria#checkPointsOrWeightMax blur->evaluation-criteria#checkPointsOrWeightMax" } %> diff --git a/spec/system/evaluation_form_spec.rb b/spec/system/evaluation_form_spec.rb index 6b6b8a44..54bb002c 100644 --- a/spec/system/evaluation_form_spec.rb +++ b/spec/system/evaluation_form_spec.rb @@ -185,7 +185,7 @@ expect(page).to have_content("Evaluation Form Saved") end - it "expands all criteria and resets values if switching to weighted scale with value over 100" do + it "expands all criteria if switching to weighted scale with value over 100" do visit new_evaluation_form_path fill_in_base_form_info @@ -196,25 +196,26 @@ fill_in_criterion_points_weight(0, 10) fill_in_numeric_criteria_type fill_in_criterion_points_weight(1, 101) + fill_in_numeric_criteria_type + fill_in_criterion_points_weight(2, 102) toggle_all_criteria_accordions(open: false) check_criteria_accordion_expanded(0, false) check_criteria_accordion_expanded(1, false) + check_criteria_accordion_expanded(2, false) select_scale_type("weighted") - accept_confirm check_criteria_accordion_expanded(0, true) check_criteria_accordion_expanded(1, true) + check_criteria_accordion_expanded(2, true) # Scale type should be weighted expect_form_scale_type_to_equal(true) - expect_criterion_points_or_weight_to_equal(0, 0) - expect_criterion_points_or_weight_to_equal(1, 0) end - it "does nothing and prevents switching scale if switching to weighted scale with value over 100 and not accepting" do + it "does nothing if switching to weighted scale with no value over 100" do visit new_evaluation_form_path fill_in_base_form_info @@ -224,7 +225,7 @@ fill_in_numeric_criteria_type(initial: true) fill_in_criterion_points_weight(0, 10) fill_in_numeric_criteria_type - fill_in_criterion_points_weight(1, 101) + fill_in_criterion_points_weight(1, 100) toggle_all_criteria_accordions(open: false) @@ -232,15 +233,14 @@ check_criteria_accordion_expanded(1, false) select_scale_type("weighted") - dismiss_confirm check_criteria_accordion_expanded(0, false) check_criteria_accordion_expanded(1, false) - # Scale type should be pointed still - expect_form_scale_type_to_equal(false) + # Scale type should be weighted + expect_form_scale_type_to_equal(true) expect_criterion_points_or_weight_to_equal(0, 10) - expect_criterion_points_or_weight_to_equal(1, 101) + expect_criterion_points_or_weight_to_equal(1, 100) end end