Skip to content

Commit

Permalink
Decouple the validation from the presentation of invalid fields
Browse files Browse the repository at this point in the history
  • Loading branch information
jcoyne committed Jul 31, 2024
1 parent 5a69c0a commit 344daf5
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 28 deletions.
12 changes: 12 additions & 0 deletions app/assets/stylesheets/registration.css
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
2 changes: 1 addition & 1 deletion app/javascript/controllers/registration_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
}
25 changes: 11 additions & 14 deletions app/javascript/controllers/registration_item_row_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,7 @@ export default class extends Controller {
}

connect () {
// These validations need to be run after values are pasted in, unless the form was loaded in CSV mode
if (!this.csvValue) {
this.validateCatalogRecordId()
this.validateSourceId()
this.validateBarcode()
this.validateLabel()
}
this.setLabelRequired()
}

// Mark any duplicate sourceIds as invalid
Expand All @@ -23,10 +17,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

Expand All @@ -39,28 +32,28 @@ export default class extends Controller {
} else {
this.clearValidation(field)
}
field.classList.toggle('is-invalid', !field.validity.valid)
})
}
}

validateBarcode () {
const field = this.barcodeTarget
if (field.validity.patternMismatch) {
field.classList.toggle('is-invalid', !field.validity.valid)
field.reportValidity()
} else {
this.clearValidation(field)
}
}

// Check that catalog record ID exists.
validateCatalogRecordId () {
this.setLabelRequired()
const field = this.catalogRecordIdTarget
const currentCatalogRecordId = field.value
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)
Expand All @@ -76,8 +69,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)
Expand All @@ -86,12 +80,15 @@ export default class extends Controller {
}

validateLabel () {
this.labelTarget.reportValidity()
}

setLabelRequired() {
const field = this.labelTarget
if (this.catalogRecordIdTarget.value === '') {
field.required = true
} else {
field.required = false
field.classList.toggle('is-invalid', false)
}
}

Expand Down
6 changes: 6 additions & 0 deletions app/javascript/controllers/registration_items_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
14 changes: 4 additions & 10 deletions app/javascript/controllers/tag_validation_controller.js
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
4 changes: 3 additions & 1 deletion app/views/registrations/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@
<div class="col-sm-9">
<span id="tags" style="width: auto">
<%= 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 %>
</span>
</div>
Expand Down
4 changes: 2 additions & 2 deletions spec/features/item_registration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 344daf5

Please sign in to comment.