From 53e63b9cf6e7f93b9e40975cd73dec63364423aa Mon Sep 17 00:00:00 2001 From: alex Date: Mon, 13 Jan 2025 22:16:47 -0500 Subject: [PATCH 1/9] first draft to fix issue# 5888 makes the generate doc select element required, validates on form submit, clears form errors when changing the select, clears the errors on modal close, adds styles to regular select as well as the select2 select element --- app/assets/stylesheets/pages/casa_cases.scss | 31 +++++++++++++- app/javascript/src/casa_case.js | 41 ++++++++++++++++++- .../_generate_docx.html.erb | 19 +++++---- 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/pages/casa_cases.scss b/app/assets/stylesheets/pages/casa_cases.scss index 34ad221b4f..1ae08a8141 100644 --- a/app/assets/stylesheets/pages/casa_cases.scss +++ b/app/assets/stylesheets/pages/casa_cases.scss @@ -169,4 +169,33 @@ body.casa_cases-show { flex-direction: column; gap: .2rem; } -} \ No newline at end of file +} + +// are these styles scoped specifically to "/case_court_reports" +// or are there other pages that use them? +#case-selection:required, .select2.select2-container { + background: transparent; + border: 1px solid #e5e5e5; + border-radius: 10px; + padding: 8px; + color: #5d657b; + appearance: none; + -webkit-appearance: none; + -moz-appearance: none; + -webkit-transition: all 0.3s ease-out 0s; + -moz-transition: all 0.3s ease-out 0s; + -ms-transition: all 0.3s ease-out 0s; + -o-transition: all 0.3s ease-out 0s; + transition: all 0.3s ease-out 0s; +} + +.select-wrapper { + display: flex; + flex-direction: column; + gap: 8px; +} + +span[role=combobox] { + padding-top: 2px; +} + diff --git a/app/javascript/src/casa_case.js b/app/javascript/src/casa_case.js index d0c39a679b..370a5118bf 100644 --- a/app/javascript/src/casa_case.js +++ b/app/javascript/src/casa_case.js @@ -85,11 +85,28 @@ function showAlert (html) { flashContainer && flashContainer.replaceWith(alertEl) } +function validateForm(formEl) { + const errorEl = document.querySelector(".select-required-error") + + if (!formEl || !errorEl) { + return + } + + // check html validations, checkValidity returns false if doesn't pass validation + if (!formEl.checkValidity()) { + errorEl.classList.remove("d-none") + return + } +} + function handleGenerateReport (e) { e.preventDefault() - const formData = Object.fromEntries(new FormData(e.currentTarget.form)) + const form = e.currentTarget.form + const formData = Object.fromEntries(new FormData(form)) + + validateForm(form) if (formData.case_number.length === 0) return const generateBtn = e.currentTarget @@ -127,6 +144,23 @@ function handleGenerateReport (e) { }) } +function clearSelectErrors () { + const errorEl = document.querySelector(".select-required-error") + + if (!errorEl) return + + errorEl.classList.add("d-none") +} + +function handleModalClose() { + const selectEl = document.querySelector("#case-selection") + + if (!selectEl) return + + clearSelectErrors() + // TODO: clear selected case as well? +} + $(() => { // JQuery's callback for the DOM loading $('button.copy-court-button').on('click', copyOrdersFromCaseWithConfirmation) @@ -134,6 +168,8 @@ $(() => { // JQuery's callback for the DOM loading disableBtn($('button.copy-court-button')[0]) } + $("#case-selection").on("change", clearSelectErrors) + $('select.siblings-casa-cases').on('change', () => { if ($('select.siblings-casa-cases').find(':selected').text()) { enableBtn($('button.copy-court-button')[0]) @@ -141,6 +177,9 @@ $(() => { // JQuery's callback for the DOM loading disableBtn($('button.copy-court-button')[0]) } }) + // modal id is defined in _generate_docx.html.erb so would like to be able to implement modal close logic in that file + // but not sure how to + $('#generate-docx-report-modal').on('hidden.bs.modal', () => handleModalClose()) $('#btnGenerateReport').on('click', handleGenerateReport) diff --git a/app/views/case_court_reports/_generate_docx.html.erb b/app/views/case_court_reports/_generate_docx.html.erb index 87d3ffc1f7..66a1214e7c 100644 --- a/app/views/case_court_reports/_generate_docx.html.erb +++ b/app/views/case_court_reports/_generate_docx.html.erb @@ -27,14 +27,17 @@ <% select_case_prompt = show_search ? "Search by volunteer name or case number" : "Select case number" %> <% select2_class = show_search ? " select2" : "" %> - <%= select_tag :case_number, - options_for_select(select_options), - prompt: select_case_prompt, - include_blank: false, - id: "case-selection", - class: "custom-select#{select2_class}", - data: { dropdown_parent: "##{id}" } %> - +
+ <%= select_tag :case_number, + options_for_select(select_options), + prompt: select_case_prompt, + include_blank: false, + id: "case-selection", + class: "custom-select#{select2_class}", + required: true, + data: { dropdown_parent: "##{id}", width: "100%" } %> +

Case selection is required.

+
<%= form.hidden_field :time_zone, id: "user-time-zone" %>
From c11158df68446620a56fca6070948fc363ac9efe Mon Sep 17 00:00:00 2001 From: alex Date: Thu, 16 Jan 2025 16:31:40 -0500 Subject: [PATCH 2/9] change casa_case.js form validation logic to work on /case_court_reports and /casa_cases/$case_number pages --- app/javascript/src/casa_case.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/javascript/src/casa_case.js b/app/javascript/src/casa_case.js index 370a5118bf..36d244dbb0 100644 --- a/app/javascript/src/casa_case.js +++ b/app/javascript/src/casa_case.js @@ -85,15 +85,13 @@ function showAlert (html) { flashContainer && flashContainer.replaceWith(alertEl) } -function validateForm(formEl) { - const errorEl = document.querySelector(".select-required-error") - - if (!formEl || !errorEl) { +function validateForm(formEl, errorEl) { + if (!formEl) { return } // check html validations, checkValidity returns false if doesn't pass validation - if (!formEl.checkValidity()) { + if (errorEl && !formEl.checkValidity()) { errorEl.classList.remove("d-none") return } @@ -105,8 +103,8 @@ function handleGenerateReport (e) { const form = e.currentTarget.form const formData = Object.fromEntries(new FormData(form)) - - validateForm(form) + const errorEl = document.querySelector(".select-required-error") + validateForm(form, errorEl ?? null) if (formData.case_number.length === 0) return const generateBtn = e.currentTarget From 6dea4450177c1a8515fa207c443ac752dc712c05 Mon Sep 17 00:00:00 2001 From: alex Date: Thu, 16 Jan 2025 18:36:53 -0500 Subject: [PATCH 3/9] clears case selection on modal close --- app/javascript/src/casa_case.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/javascript/src/casa_case.js b/app/javascript/src/casa_case.js index 36d244dbb0..7a5c1700e9 100644 --- a/app/javascript/src/casa_case.js +++ b/app/javascript/src/casa_case.js @@ -156,7 +156,8 @@ function handleModalClose() { if (!selectEl) return clearSelectErrors() - // TODO: clear selected case as well? + // this line taken from docs https://select2.org/programmatic-control/add-select-clear-items + $('#case-selection').val(null).trigger('change') } $(() => { // JQuery's callback for the DOM loading From 8f7ff45a494b9a301270d7d11d1dfb35be59352b Mon Sep 17 00:00:00 2001 From: alex Date: Thu, 23 Jan 2025 17:41:40 -0500 Subject: [PATCH 4/9] cleans up select styles --- app/assets/stylesheets/pages/casa_cases.scss | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/assets/stylesheets/pages/casa_cases.scss b/app/assets/stylesheets/pages/casa_cases.scss index 1ae08a8141..2aa2825d4a 100644 --- a/app/assets/stylesheets/pages/casa_cases.scss +++ b/app/assets/stylesheets/pages/casa_cases.scss @@ -178,15 +178,6 @@ body.casa_cases-show { border: 1px solid #e5e5e5; border-radius: 10px; padding: 8px; - color: #5d657b; - appearance: none; - -webkit-appearance: none; - -moz-appearance: none; - -webkit-transition: all 0.3s ease-out 0s; - -moz-transition: all 0.3s ease-out 0s; - -ms-transition: all 0.3s ease-out 0s; - -o-transition: all 0.3s ease-out 0s; - transition: all 0.3s ease-out 0s; } .select-wrapper { From bcf34325ca667d1a9740113ca43c6f23db8b7460 Mon Sep 17 00:00:00 2001 From: alex Date: Fri, 24 Jan 2025 00:26:43 -0500 Subject: [PATCH 5/9] appease the linter --- app/javascript/src/casa_case.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/app/javascript/src/casa_case.js b/app/javascript/src/casa_case.js index 7a5c1700e9..a11a65ef94 100644 --- a/app/javascript/src/casa_case.js +++ b/app/javascript/src/casa_case.js @@ -85,15 +85,14 @@ function showAlert (html) { flashContainer && flashContainer.replaceWith(alertEl) } -function validateForm(formEl, errorEl) { +function validateForm (formEl, errorEl) { if (!formEl) { return } // check html validations, checkValidity returns false if doesn't pass validation if (errorEl && !formEl.checkValidity()) { - errorEl.classList.remove("d-none") - return + errorEl.classList.remove('d-none') } } @@ -103,7 +102,7 @@ function handleGenerateReport (e) { const form = e.currentTarget.form const formData = Object.fromEntries(new FormData(form)) - const errorEl = document.querySelector(".select-required-error") + const errorEl = document.querySelector('.select-required-error') validateForm(form, errorEl ?? null) if (formData.case_number.length === 0) return @@ -143,15 +142,15 @@ function handleGenerateReport (e) { } function clearSelectErrors () { - const errorEl = document.querySelector(".select-required-error") + const errorEl = document.querySelector('.select-required-error') if (!errorEl) return - errorEl.classList.add("d-none") + errorEl.classList.add('d-none') } -function handleModalClose() { - const selectEl = document.querySelector("#case-selection") +function handleModalClose () { + const selectEl = document.querySelector('#case-selection') if (!selectEl) return @@ -167,7 +166,7 @@ $(() => { // JQuery's callback for the DOM loading disableBtn($('button.copy-court-button')[0]) } - $("#case-selection").on("change", clearSelectErrors) + $('#case-selection').on('change', clearSelectErrors) $('select.siblings-casa-cases').on('change', () => { if ($('select.siblings-casa-cases').find(':selected').text()) { From b97ab9dc9c655408b4c270603bfa287afca9f9a1 Mon Sep 17 00:00:00 2001 From: alex Date: Fri, 24 Jan 2025 01:06:42 -0500 Subject: [PATCH 6/9] add back in styles to remove select element chevon icon when signed in as volunteer --- app/assets/stylesheets/pages/casa_cases.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/pages/casa_cases.scss b/app/assets/stylesheets/pages/casa_cases.scss index 2aa2825d4a..4c2447b478 100644 --- a/app/assets/stylesheets/pages/casa_cases.scss +++ b/app/assets/stylesheets/pages/casa_cases.scss @@ -178,6 +178,9 @@ body.casa_cases-show { border: 1px solid #e5e5e5; border-radius: 10px; padding: 8px; + appearance: none; + -webkit-appearance: none; + -moz-appearance: none; } .select-wrapper { From 63ef7f70e60b6691034c6fac9d5d82172421a53f Mon Sep 17 00:00:00 2001 From: alex Date: Wed, 29 Jan 2025 21:48:37 -0500 Subject: [PATCH 7/9] add tests to check that error message appears when expected and is hidden when expected --- spec/system/case_court_reports/index_spec.rb | 22 +++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/spec/system/case_court_reports/index_spec.rb b/spec/system/case_court_reports/index_spec.rb index 05fe8a1726..c27773be7d 100644 --- a/spec/system/case_court_reports/index_spec.rb +++ b/spec/system/case_court_reports/index_spec.rb @@ -71,8 +71,28 @@ click_button "Generate Report" expect(page).to have_selector("#btnGenerateReport .lni-download", visible: true) - expect(page).not_to have_selector("#btnGenerateReport[disabled]") + expect(page).not_to have_selector("#btnGenerateReport[disabled]") # is the button expected to be disabled? expect(page).to have_selector("#spinner", visible: :hidden) + + # when 'Generate Report' button is clicked without a selection, should display an error saying to make a selection + expect(page).to have_selector(".select-required-error", visible: :visible) + + test_case_number = casa_cases.find(&:in_transition_age?).case_number.to_s + + # when we make a selection, the error is no longer visible + page.select test_case_number, from: "case-selection" + expect(page).not_to have_selector(".select-required-error", visible: :visible) + + # test the flow for clearing case selection error message by closing modal + click_button "Close" + page.find(modal_selector).click + click_button "Generate Report" + # expect the error message to be visible because we tried to generate the doc without making a selection + expect(page).to have_selector(".select-required-error", visible: :visible) + # expect error message to be gone after we close and re-open the modal + click_button "Close" + page.find(modal_selector).click + expect(page).not_to have_selector(".select-required-error", visible: :visible) end end From ba6a9f076a9899965d2a129320c797d9a746b21c Mon Sep 17 00:00:00 2001 From: alex Date: Wed, 29 Jan 2025 21:50:04 -0500 Subject: [PATCH 8/9] remove unnecessary comments --- app/assets/stylesheets/pages/casa_cases.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/assets/stylesheets/pages/casa_cases.scss b/app/assets/stylesheets/pages/casa_cases.scss index 4c2447b478..2fbf9464e7 100644 --- a/app/assets/stylesheets/pages/casa_cases.scss +++ b/app/assets/stylesheets/pages/casa_cases.scss @@ -171,8 +171,6 @@ body.casa_cases-show { } } -// are these styles scoped specifically to "/case_court_reports" -// or are there other pages that use them? #case-selection:required, .select2.select2-container { background: transparent; border: 1px solid #e5e5e5; From d08af7f6a632ff8079d26e9335bfdb5f82cd0c6d Mon Sep 17 00:00:00 2001 From: alex Date: Wed, 29 Jan 2025 21:51:40 -0500 Subject: [PATCH 9/9] remove more comments --- spec/system/case_court_reports/index_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/case_court_reports/index_spec.rb b/spec/system/case_court_reports/index_spec.rb index c27773be7d..985fcc7baa 100644 --- a/spec/system/case_court_reports/index_spec.rb +++ b/spec/system/case_court_reports/index_spec.rb @@ -71,7 +71,7 @@ click_button "Generate Report" expect(page).to have_selector("#btnGenerateReport .lni-download", visible: true) - expect(page).not_to have_selector("#btnGenerateReport[disabled]") # is the button expected to be disabled? + expect(page).not_to have_selector("#btnGenerateReport[disabled]") expect(page).to have_selector("#spinner", visible: :hidden) # when 'Generate Report' button is clicked without a selection, should display an error saying to make a selection