Skip to content

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

xforman2
Copy link

@xforman2 xforman2 commented Mar 2, 2025

Description

Replaced checkValidity with reportValidity as described here #2372 (comment)

Corresponding issue:
#2372

Testing

Tested manually in the scratch demo

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@Telroshan Telroshan added the bug Something isn't working label Mar 3, 2025
@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Mar 3, 2025
@@ -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()) {
Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Collaborator

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.

@dwasyl
Copy link

dwasyl commented Jun 9, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants