-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: use reportValidity instead of checkValidity in validateElement #3215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
@@ -3492,7 +3492,7 @@ var htmx = (function() { | |||
const element = /** @type {HTMLElement & ElementInternals} */ (elt) | |||
if (element.willValidate) { | |||
triggerEvent(element, 'htmx:validation:validate') | |||
if (!element.checkValidity()) { | |||
if (!element.reportValidity()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, this is a small but significant change. Maybe we should gate it with a config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
Description
Replaced
checkValidity
withreportValidity
as described here #2372 (comment)Corresponding issue:
#2372
Testing
Tested manually in the scratch demo
Checklist
master
for website changes,dev
forsource changes)
approved via an issue
npm run test
) and verified that it succeeded