fix: use reportValidity instead of checkValidity in validateElement #3215
fix: use reportValidity instead of checkValidity in validateElement #3215xforman2 wants to merge 1 commit intobigskysoftware:devfrom
Conversation
| if (element.willValidate) { | ||
| triggerEvent(element, 'htmx:validation:validate') | ||
| if (!element.checkValidity()) { | ||
| if (!element.reportValidity()) { |
There was a problem hiding this comment.
Maybe we should add a doc comment saying that validateElement will now report validation errors instead of just checking them?
There was a problem hiding this comment.
Agree, this is a small but significant change. Maybe we should gate it with a config?
There was a problem hiding this comment.
Maybe...although reportValidity() is also the browser default behaviour, so on that assumption we might expect htmx to also do it by default.
There was a problem hiding this comment.
triggerEvent(element, 'htmx:validation:validate')
if (!element.checkValidity()) {
if(!errors.length) {
element.reportValidity()
}
errors.push({ elt: element, message: element.validationMessage, validity: element.validity })
triggerEvent(element, 'htmx:validation:failed', { message: element.validationMessage, validity: element.validity })
}Another issue is that when i tested this change it does not behave like normal form validation as it always triggers on the last input that is missing or invalid instead of the first one like a normal form would. This is because we reportValidity() of all the inputs in order and the last invalid input gets the warning message for the user. The above code snipit solved this for me by only doing reportValidtity on the first error found which is a much nicer UX.
But this does seem like a kind of breaking change to add user alerts to existing projects that were not using them. The current setup reports these error alerts as events via htmx.validation:failed and so i would assume some users would have used this feature to update the UI on validation errors in their apps and adding another layer of reporting would be kind of a breaking change even though it does put back the default submit behavior.
Also @Telroshan mentioned it could be canceled with the invalid event so there is a way for a user to disable the alert if needed. I did get this to work with preventDefault() but this invalid event does not bubble which means you have to add hx-on or event listeners to EVERY input one by one.
triggerEvent(element, 'htmx:validation:validate')
if (!element.checkValidity()) {
if(triggerEvent(element, 'htmx:validation:failed', { message: element.validationMessage, validity: element.validity }) && !errors.length) {
element.reportValidity()
}
errors.push({ elt: element, message: element.validationMessage, validity: element.validity })
}Something like this could allow a hx-on hx-on::validation:failed="event.preventDefault()" or event listener on body could allow reverting to the old behavior by using the failed event to cancel the alert when needed which bubbles fine.
But It could be better to just add documentation to our validation docs to get users who want this validation to add
document.body.addEventListener('htmx:validation:failed', function(event) {
event.target.reportValidity()
})or hx-on::validation:failed="event.target.reportValidity()
so the can opt into this feature.
|
Any chance of this fix making it into a release anytime soon? I just spent a few hours trying to track down what was happening since the validate seemed to be working but there was no user displayed message and this deals with that nicely. |
|
obviated by #3372 |
Description
Replaced
checkValiditywithreportValidityas described here #2372 (comment)Corresponding issue:
#2372
Testing
Tested manually in the scratch demo
Checklist
masterfor website changes,devforsource changes)
approved via an issue
npm run test) and verified that it succeeded