Change form validation to support Bootstrap 4.0.0.beta (replaces #345) #370
Conversation
|
Now this PR addresses the action |
lib/bootstrap_form/form_builder.rb
Outdated
| if options[:custom] | ||
| validation = nil | ||
| validation = "is-invalid" if has_error?(name) | ||
| validation = "is-valid" if is_valid?(name) |
There was a problem hiding this comment.
So bootstrap4 added valid states, however I wonder if this is the behaviour we want to support.
I can see this making sense for client side validated forms, but in server side rendered cases it feels a little odd as the field will render with a is-valid class only in the case when the model fails validation error and the form is re-rendered, not at the point when a user inputs content in the fields.
What's everyones opinion on this?
There was a problem hiding this comment.
It seems like the right thing to, just as it was before, just with the new classes.
There was a problem hiding this comment.
Sorry, didn't read the context of this comment. Now I am not so sure. I think marking the valid fields is OK, but only marking the invalid fields is better. Actually, for red/green color blind users, only marking the invalid fields helps them stand out. I think it is good either way :)
There was a problem hiding this comment.
A quick test in a real app showed me that marking valid fields with green looks OK, but does not improve the UX, really.
There was a problem hiding this comment.
I think we should leave out is-valid for this PR. I don't see myself using it for server-side validation.
There was a problem hiding this comment.
I just removed the 'is-valid' class within the PR.
There was a problem hiding this comment.
Yup, definitely feels like the right behaviour. Thanks for the great work @CyberDeck!
CHANGELOG.md
Outdated
|
|
||
| Features: | ||
| - Your contribution here! | ||
| - is-valid will be applied to validated form field (i.e. not on unchanged or new records) |
There was a problem hiding this comment.
This is not true anymore, right?
There was a problem hiding this comment.
Yes. I will remove it.
mattbrictson
changed the title
|
Looks good from my end |
|
Thanks for the PR and congratulations on your first contribution to the project! 🎉 |
…strap-ruby#345) (bootstrap-ruby#370) * New PR to replace PR bootstrap-ruby#345 * Removed has_danger and form-control-label * Removed 'is-valid' * Removed wrong line from Changelog * Update html help test with latest v4 help markup
4 participants
This should contain all relevant changes to reassemble the old PR #345.
Bugfixes:
form-control-dangeris replaced withis-invalidfor bootstrap 4.0.0.beta3form-control-feedbackis replaced withinvalid-feedbackfor bootstrap 4.0.0.beta3<small>tag instead of<span>tag, i.e. like in bootstrap 4.0.0.beta3Features:
is-validwill be applied to validated form field (i.e. not on unchanged or new records)custom: trueoption for radio buttons and check boxes according to bootstrap 4.0.0.beta3