From ca94c730774f3cd2ba7c4985c727262b8161accd Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 31 Jul 2024 12:17:26 -0500 Subject: [PATCH] Decouple the validation from the presentation of invalid fields --- app/assets/stylesheets/application.scss | 1 + app/assets/stylesheets/registration.scss | 12 ++++++++++++ .../controllers/registration_controller.js | 2 +- .../registration_item_row_controller.js | 13 ++++++------- .../controllers/registration_items_controller.js | 6 ++++++ .../controllers/tag_validation_controller.js | 14 ++++---------- app/views/registrations/show.html.erb | 4 +++- spec/features/item_registration_spec.rb | 4 ++-- 8 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 app/assets/stylesheets/registration.scss diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 434f83899..aa6999496 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -19,4 +19,5 @@ @import "impersonate_groups"; @import "workflow_grid"; @import "tags-autocomplete"; +@import "registration"; @import "text-extraction"; diff --git a/app/assets/stylesheets/registration.scss b/app/assets/stylesheets/registration.scss new file mode 100644 index 000000000..c33d8696c --- /dev/null +++ b/app/assets/stylesheets/registration.scss @@ -0,0 +1,12 @@ +/* Here we turn off the style for the valid fields, so it only shows invalid */ +.was-validated { + .form-control, + .form-select { + --bs-form-valid-border-color: var(--bs-border-color); + --bs-success-rgb: var(--stanford-digital-blue-rgb); + + &:valid { + background-image: none; + } + } +} diff --git a/app/javascript/controllers/registration_controller.js b/app/javascript/controllers/registration_controller.js index 516d7b144..287374726 100644 --- a/app/javascript/controllers/registration_controller.js +++ b/app/javascript/controllers/registration_controller.js @@ -4,6 +4,6 @@ export default class extends Controller { // If the field is marked as invalid, then show the invalid (bootstrap) style. displayValidation (event) { const field = event.target - field.classList.toggle('is-invalid', !field.validity.valid) + field.closest('form').classList.add('was-validated') } } diff --git a/app/javascript/controllers/registration_item_row_controller.js b/app/javascript/controllers/registration_item_row_controller.js index d9c50f140..c7e9450ee 100644 --- a/app/javascript/controllers/registration_item_row_controller.js +++ b/app/javascript/controllers/registration_item_row_controller.js @@ -23,10 +23,9 @@ export default class extends Controller { if (currentSourceId === '') { return } if (field.validity.patternMismatch) { - field.classList.toggle('is-invalid', !field.validity.valid) + field.reportValidity() } else if (this.isDuplicateSourceId(currentSourceId)) { this.setValidation(field, 'Duplicate source ID on this form') - field.classList.add('is-invalid') } else { this.clearValidation(field) // all other checks passed @@ -39,7 +38,6 @@ export default class extends Controller { } else { this.clearValidation(field) } - field.classList.toggle('is-invalid', !field.validity.valid) }) } } @@ -47,7 +45,7 @@ export default class extends Controller { validateBarcode () { const field = this.barcodeTarget if (field.validity.patternMismatch) { - field.classList.toggle('is-invalid', !field.validity.valid) + field.reportValidity() } else { this.clearValidation(field) } @@ -60,7 +58,7 @@ export default class extends Controller { if (currentCatalogRecordId === '') { return } if (field.validity.patternMismatch) { - field.classList.toggle('is-invalid', !field.validity.valid) + this.setValidation(field, 'Incorrect format for HRID') } else { // Only check if the format is valid this.clearValidation(field) @@ -76,8 +74,9 @@ export default class extends Controller { .then((data) => { if (!data) { this.setValidation(field, 'Not found in catalog') + } else { + this.clearValidation(field) } - field.classList.toggle('is-invalid', !field.validity.valid) }) .catch((error) => { this.setValidation(field, error.message) @@ -91,8 +90,8 @@ export default class extends Controller { field.required = true } else { field.required = false - field.classList.toggle('is-invalid', false) } + field.reportValidity() } setValidation (field, msg) { diff --git a/app/javascript/controllers/registration_items_controller.js b/app/javascript/controllers/registration_items_controller.js index 767069183..62f91780f 100644 --- a/app/javascript/controllers/registration_items_controller.js +++ b/app/javascript/controllers/registration_items_controller.js @@ -12,9 +12,15 @@ export default class extends Controller { event.preventDefault() const content = this.templateTarget.innerHTML.replace(/TEMPLATE_RECORD/g, crypto.randomUUID()) this.add_itemTarget.insertAdjacentHTML('beforebegin', content) + this.resetValidations() this.count++ } + // Prevent the new fields from showing up marked as invalid + resetValidations () { + this.add_itemTarget.closest('form').classList.remove('was-validated') + } + removeAssociation (event) { event.preventDefault() event.target.closest(this.selectorValue).remove() diff --git a/app/javascript/controllers/tag_validation_controller.js b/app/javascript/controllers/tag_validation_controller.js index 31f45db75..0529e944d 100644 --- a/app/javascript/controllers/tag_validation_controller.js +++ b/app/javascript/controllers/tag_validation_controller.js @@ -1,20 +1,14 @@ import { Controller } from '@hotwired/stimulus' -// Validates free-text tags fields and sets the .invalid class on elements -// that aren't well formed tags +// Alerts the form to display errors if the tags aren't well formed export default class extends Controller { connect () { this.validate() } validate () { - const parts = this.element.value.trim().split(/\s*:\s*/) - this.element.value = parts.join(' : ') - this.element.classList.toggle('is-invalid', this.is_invalid(parts)) - } - - is_invalid (parts) { - return (parts.length === 1 && parts[0] !== '') || - (parts.length > 1 && parts.includes('')) + if (this.element.validity.patternMismatch) { + this.element.reportValidity() + } } } diff --git a/app/views/registrations/show.html.erb b/app/views/registrations/show.html.erb index d4c1981d0..e66121cc5 100644 --- a/app/views/registrations/show.html.erb +++ b/app/views/registrations/show.html.erb @@ -46,7 +46,9 @@
<%= f.fields_for :tags do |tags_form| %> - <%= tags_form.text_field :name, class: 'form-control mb-2', data: { controller: 'tag-validation', action: 'change->tag-validation#validate', tag_autocomplete: true } %> + <%= tags_form.text_field :name, class: 'form-control mb-2', + pattern: '^\w+\s*:\s*\w+(\s*:\s*\w+)*$', + data: { controller: 'tag-validation', action: 'change->tag-validation#validate invalid->registration#displayValidation', tag_autocomplete: true } %> <% end %>
diff --git a/spec/features/item_registration_spec.rb b/spec/features/item_registration_spec.rb index ff5cddfaf..251f1d64a 100644 --- a/spec/features/item_registration_spec.rb +++ b/spec/features/item_registration_spec.rb @@ -177,7 +177,7 @@ click_button 'Register' - expect(page).to have_css('.is-invalid') + expect(page).to have_css('.was-validated :invalid') expect(page).to have_no_content 'Items successfully registered.' end end @@ -201,7 +201,7 @@ click_button 'Register' - expect(page).to have_css('.is-invalid') + expect(page).to have_css('.was-validated :invalid') expect(page).to have_no_content 'Items successfully registered.' end end