Validate hidden inputs#1016
Conversation
06bafc3 to
b84eb61
Compare
|
Thanks for this PR.
Neither can I: https://www.githubstatus.com/ |
There was a problem hiding this comment.
Pull request overview
Adds an opt-in mechanism to validate form controls that are not visible in the DOM (e.g., native <select> elements hidden by custom widgets or forms hidden via collapse/display rules), addressing the post-jQuery removal gap where validate_inputs was previously used.
Changes:
- Introduce
data-csv-validate-hiddento include hidden elements in validation/binding via a newisValidatablehelper. - Add QUnit coverage for validating hidden inputs both before and after enabling validations.
- Update docs/changelog and remove some unused/legacy internal JS exports/APIs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/javascript/public/test/form_builders/validateForm.js | Adds QUnit tests covering hidden-input opt-in behavior. |
| src/utils.js | Adds isValidatable and adjusts visibility logic; removes some exports. |
| src/index.js | Uses isValidatable when determining which validated inputs to check on submit. |
| src/events.js | Removes unused default export. |
| src/core.js | Uses isValidatable when enabling/binding inputs; removes unused selector entries. |
| README.md | Documents data-csv-validate-hidden usage and semantics. |
| CHANGELOG.md | Notes the new opt-in feature and related internal export cleanups. |
Comments suppressed due to low confidence (1)
src/events.js:37
- Removing the
defaultexport fromsrc/events.jsis a breaking change for npm consumers deep-importing fromsrc/(the package publishessrc/**/*.js). If this is intended, it should be treated/documented as a breaking change; otherwise consider keeping a backwards-compatible default export (even if deprecated) that re-exports the named functions.
export const dispatchCustomEvent = (element, eventName, detail) => {
element.dispatchEvent(new CustomEvent(eventName, {
bubbles: true,
detail
}))
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b84eb61 to
54b0c3b
Compare
|
I updated my commits and removed the parts you changed in https://github.com/DavyJonesLocker/client_side_validations/pull/1017/changes I also managed to build the new JS and run the test localy |
…Validations.selectors'
# Conflicts: # lib/client_side_validations/version.rb
54b0c3b to
4441b5f
Compare
|
@rocket-turtle As per my comment at the issue below, not validating hidden fields is currently a deliberate design choice: #616 (comment) I also have some concerns about the proposed implementation and naming.
To clarify, my goal was never to completely prevent validation of hidden fields at all costs. I am open to supporting this behavior if there is a strong and clear use case for it. Could you provide a concrete real-world scenario where validating hidden fields on the client side is necessary? A minimal reproducible example using the playground would help a lot: I guess it is something like adding a javascript select dropbox |
|
You're right on the naming. What do you think about: The scenario is a native Worth noting this used to be easily doable by overriding I'm out for two weeks. I'll put together the Tom-Select repro on csv-playground when I'm back, rename to whatever you prefer, or refactor to something other to support this usecase. |
That approach had a subtle bug that made it appear to work correctly only by coincidence. The problem with overriding Consider a form with a conditionally revealed field: When the checkbox is ticked, a sub-field appears: Broadening In other words: the old workaround didn't correctly distinguish between fields that are always hidden (e.g. a Reproduction A minimal repro is available at: The hidden sub-checkbox fails validation and blocks submission, demonstrating the bug. |
|
I'm trying to test this behavior on a local application. It appears to me that tomselect does not trigger a "focusout" event on the original element when the input is unchanged, and this prevents clientside validations to properly work. A reproducible test case based on https://github.com/tagliala/csv-playground with the minimal changes to make this work would definitely help AI suggested to implement an adapter API, which I've tried, but it looks overcomplicated and then every single javascript plugin may require a specific adapter, which I would to avoid. At the moment, I acknowledge the issue, but we should find a good solution |
We use some custom selects which hide the original select. In v23 it was possible to add them back in
validate_inputs. After the removal of jQuery in v24validate_inputsis dead.This would also be a clean way for this Issue: #616
BTW: I couldnt run the tests but I hope they work :D