client_side_validations
client_side_validations copied to clipboard
Add support for date/time/date_time selects
Added support new fields, which weren't overridden by CSV form_builder and therefore not included in CSV Hash.
Showing of error_message doesn't work well with these, because of special naming and logic (multiple select should share one message) of this fields and would require customizing JS FormBuilder add/remove functions - this would be easier once you transfer JS FormBuilder's wrappers:
from my CSV_simple_form PR https://github.com/DavyJonesLocker/client_side_validations-simple_form/pull/81.
Anyway, at least validations are in CSV hash now, and input's are being marked as invalid and this PR is also needed to make date/time inputs work in CSV_simple_form
There is again one rubocop offense Metrics/ModuleLength: Module has too many lines
for form_builder.rb, I leave this to your decision.
Coverage remained the same at 100.0% when pulling 06ce081da5a78cc8cf4f5fc2d15ccb28aa8dbaf1 on MichalRemis:SupportDateTimeSelects into a49dd74a14b259b9a08551fedd163cf2406ba25c on DavyJonesLocker:master.
Thanks for feedback. All requested changes are fixed.
@MichalRemis I want to thank you for your work and apologize because I do not have too much time during the week to review this change.
During the weekend I will share a demo rails application with CSV to test stuff and add you permissions.
Hi @MichalRemis , as promised I've created a new repo for testing PR/issues: https://github.com/tagliala/csv-playground
Branch https://github.com/tagliala/csv-playground/tree/client-side-validations/784
I've investigated this issue and I think that a proper implementation should focus on how validation errors are being detected, instead of how to display them in multi select fields.
I think that the following could be a proper behavior:
- Error should not appear while I'm filling multiple fields of the same select (eg: fill in day of
date_of_birth
, press TAB, should not get an error because month and year indate_of_birth
are still empty). - Error should appear when I change focus from a multi select to another field (eg: fill in day of
date_of_birth
, change focus toname
); - Only one error should be displayed for a multi-select field
Point 1 suggests that the validation should trigger only on certain circumstances: probably CSV should manage 1i
, 2i
, ni
as they are the same field.
Point 2 gives us a hint on when to run the validation add the error, but it also suggests that we should check the element with the focus after a blur occurs... and I do not see anything good here
Hopefully, we can get the 3rd point for free, if we manage to properly detect errors and ask CSV to add the error to the nth-element of the multi select
Ok I checked the playground app and I think it will be quite a challenge to to implement this in main CSV, because, unlike simple_form
, it doesn't use wrappers nor error class, so we don't even know, that we are validating these special (datetime/checkboxes) field nor we don't know if sibling inputs are invalid.
I can imagine, (after implementing https://github.com/DavyJonesLocker/client_side_validations-simple_form/pull/81) that custom default wrapper could be set to date_selects etc., with special add/remove functions to achieve desired behaviour. But it won't be straightforward because of field_error_proc
approach of mainCSV.
Due to these limitations I am not sure now, if it makes sense to put these special fields into mainCSV. I can put it all into into CSV-simple_form only, without need for changes in mainCSV what do you think? You can test all new fields from my simple_form's PRs here http://kamnavylet.sk:3000/