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.

Sign up for GitHub

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
Open

fix: use reportValidity instead of checkValidity in validateElement #3215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/htmx.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Contributor

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.

errors.push({ elt: element, message: element.validationMessage, validity: element.validity })
triggerEvent(element, 'htmx:validation:failed', { message: element.validationMessage, validity: element.validity })
}
Expand Down