-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Change: make browser validations configurable via DSL #2339
Change: make browser validations configurable via DSL #2339
Conversation
I'm not able to reproduce the failure with mongoid... |
Actually, browser validations are generating a lot of javascript errors for has_many/has_one widget because the input fields are hidden. For instance:
|
@mshibuya Let me know what do you think about this PR 😉 |
+1 |
1 similar comment
+1 |
@@ -254,6 +257,7 @@ def models | |||
# @see RailsAdmin::Config.registry | |||
def reset | |||
@compact_show_view = true | |||
@browser_validations = false |
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.
I'm a bit concerned about defaulting this to false
.
Don't you think that client-side validation is a nice-to-have feature?
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.
@mshibuya If you prefer I can set the default value to true. It's not a problem for me.
My personal opinion is that browser validation is a nice to have feature only for simple application's models.
In my experience there are two use cases that it could be a trouble:
- When a model has has_many/has_one nested attributes to be validated the browser-validation is generating a lot of js errors as commented above.
- When a model has conditional validations.
Let me know what do you prefer as default value so I can do a commit and rebase the PR 😉
0362e9f
to
b7d8c64
Compare
@mshibuya I changed the default value to |
…ions are disabled
163105d
to
962ee4a
Compare
Squashed and cherry-picked with slight modification( |
❤️ Thanks @mshibuya!! |
in my case , config.browser_validations = false has no effect, and browser validation still works for me. cannot see novalidate attribute in field |
now, it works, thx 👍 |
@dalpo as you said, I am getting a bunch of the following js errors on rails_admin 0.7:
This did not happen on 0.6.8. Disabling all browser validation feels like overkill. Did we figure out what was wrong in this case? |
@vincentwoo I didn't investigate too much about it. Btw this was the old PR that introduced the required attribute: As you can see that PR didn't introduce the required attribute for all input fields, but only for those that are using the However with the latest changes, now the required attribute is present for every input field. P.S. |
Add the ability to enable/disable html5 browser validations through: