Skip to content
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

BUG: pre-submit custom form validation not applied to radiobuttons and checkboxes #2676

Open
ehenighan opened this issue Jun 25, 2024 · 5 comments
Labels
1.0 bug Something isn't working

Comments

@ehenighan
Copy link

Found while testing a slightly niche use-case in a questionnaire form I'm building - when you apply custom validation across a form using the HTML5 hooks to enforce that the user has to fill out at least one of a set of answers on a page (i.e. no particular radiogroup is required, but at least one of the visible radiogroups must have a value), the onsubmit validation isn't respected.

This is because the 'shouldInclude' function excludes unticked checkboxes/radiobuttons but the 'validateElement' check is inside the 'shouldInclude' block.

It's easy to fix and I've tested locally - just separated out 'couldInclude' and 'shouldValidate' from 'shouldInclude' in order to lift up the validation outside the 'shouldInclude' block, but wasn't sure where you'd want a PR targeting now there's version 2 and 1.9.12 on the go at the same time.

Will check back shortly to add code examples!

@ehenighan
Copy link
Author

HTMX Bug #2676 - Replication.zip

Zip file contains a basic HTML file with two forms demonstrating the issue:

  • First form has a number input and a text input and won't submit unless you set at least one of them
  • Second form has a checkbox input and a radiogroup and will show the same error but (try to) submit in the background anyway

N.B. that the validation's broken after the first pass due to the fake POST that goes nowhere.

Will update with proposed fix shortly

ehenighan added a commit to ehenighan/htmx that referenced this issue Jun 25, 2024
…bmission

bigskysoftware#2676

Validate unchecked checkbox or radio inputs even though they won't be included in the POST body, in case there is e.g. HTML5 custom validation applied to the element which should prevent form submission.
@ehenighan
Copy link
Author

Forked and added the fix I've tested locally here, for comparison:

dev...ehenighan:htmx:bug-2676-pre-submit-custom-validation

Haven't raised a PR yet as I'm neck-deep in a project at work and I'm mindful that I've not worked with this test suite before etc. Would you want the test going into test/core/validation.js next to the existing one about custom-validation?

it('Custom validation error prevents request', function() {

@Telroshan Telroshan added the bug Something isn't working label Jun 26, 2024
@Telroshan
Copy link
Collaborator

Hey!

wasn't sure where you'd want a PR targeting now there's version 2 and 1.9.12 on the go at the same time

I see you're using htmx 1.9.12 in your bug replication sample, would you mind checking if it also happens on htmx 2?

As it's a bug, and if it's occuring on both versions, I'd say htmx would benefit from the bugfix on both htmx 1 and 2.
The easiest way to go about it would probably to make a first PR targeting dev (htmx 2) where the most active development happens at, then, if and once accepted, make another PR to port that fix to v1

Would you want the test going into test/core/validation.js next to the existing one about custom-validation?

Sounds good to me!

@ehenighan
Copy link
Author

ehenighan commented Aug 19, 2024

Hi, sorry for the delay in coming back to this. Since I last looked into it, the refactoring to use HTMLFormElement in processInputValue has resolved the issue in v2 but I've raised an initial PR for v2 anyway just to add the relevant tests to prevent regression: #2829

I'll raise another PR to actually apply the fix to the v1 branch and then update this issue after that.

ehenighan pushed a commit to ehenighan/htmx that referenced this issue Aug 19, 2024
ehenighan pushed a commit to ehenighan/htmx that referenced this issue Aug 19, 2024
@ehenighan
Copy link
Author

...and here's the v1 fix PR: #2830

Slight formatting changes from my original branch, which I will delete - I think there were probably some interim changes to linting standards or something.

Telroshan pushed a commit that referenced this issue Aug 19, 2024
)

* Tests for v2 to prevent regression of issue from v1

* Linting

---------

Co-authored-by: Ed Henighan <ed.henighan@adi-uk.com>
@Telroshan Telroshan added the 1.0 label Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants