-
Notifications
You must be signed in to change notification settings - Fork 54
[Delivers #98157690] add client-side form validation #658
Conversation
...so that they aren't submitted with forms
Will review after #656 is finished/merged since it seems like there is a good amount of overlap |
Ready for some 👀 ! |
@@ -53,6 +53,7 @@ gem 'workflow' | |||
group :test, :development do | |||
gem 'bullet', require: false # use BULLET_ENABLED=true | |||
gem 'database_cleaner' | |||
gem 'konacha' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
niiiice
Updated! |
I will QA |
In general this looks good and doesn't seem to introduce any regressions. There are some bugs though. Notably, an empty "Approver" field does not seem to trigger the "required" check. At first glance this seems to be because the HAML markup for that particular field is different than the others. Likewise, empty "Building Code" and "Vendor" dropdown fields are not caught as required either. Server-side validation still works as expected, though, so this might be acceptable in this first pass. |
c4c7a15
to
3b8b95a
Compare
Yeah, unfortunately client-side validation doesn't work with Selectize fields (selectize/selectize.js#733). Client-side validation is a convenience, so I personally don't think it's a blocker if it doesn't work across every field. |
[Delivers #98157690] add client-side form validation
Depends/builds on #656. The diff:
96851664-simplify-forms...98157690-client-side-validation
Changes
disable
s the form fields that are hidden by the filter, so that they:recurring_interval
value whenrecurring
isn't checkedYay for limited-internet time to work on this! 🚂
Testing notes
required
hidden fields (e.g. the BA80 fields whenBA61
is selected) don't block submissions of the form