Add :valid_class on input wrapper when value is present and valid #1333
Conversation
|
@plataformatec Feedback, please? |
There was a problem hiding this comment.
Hi @aeberlin
Looks good to me. I tested it and it works as expected. The cool thing is that if you don't want to use this feature you can remove the valid_class from the initializer.
Thanks for the pull-request :)
@rafaelfranca Any thougths?
| @@ -243,4 +243,16 @@ def with_full_error_for(object, *args) | |||
| assert_no_select 'span.error' | |||
| end | |||
| end | |||
|
|
|||
| # NO ERRORS | |||
There was a problem hiding this comment.
I think you can remove this comentary
| @@ -13,6 +13,16 @@ def has_errors? | |||
| object && object.respond_to?(:errors) && errors.present? | |||
| end | |||
|
|
|||
| def has_value? | |||
| return unless object && object.respond_to?(attribute_name) | |||
There was a problem hiding this comment.
As we expect a boolean here we can return false. What do you think?
There was a problem hiding this comment.
The whole method could be a a single operation:
object && object.respond_to?(attribute_name) && object.send(attribute_name).present?
|
@feliperenan would you may take over this PR? It seems @aeberlin is not very active, except probably no response. Did he allow PR edits from maintainers? Otherwise, I would volunteer, with your guidance. |
|
👍 I'm good with the idea |
|
@m5o We can't commit on this branch as well. If you are willing in working on it go ahead, just open another pull request then we can close this one. |
* use single operation for has_value? * via: comment from @josevalim at: heartcombo#1333 (comment)
|
closing in favor #1553 |
* use single operation for has_value? * via: comment from @josevalim at: heartcombo#1333 (comment)
Purpose
The purpose of this PR is to improve style support for inputs with validations. Simply put, the lack of an applied
:error_classis not indicative of the attribute being valid. While this isn't a guaranteed method of determining if it is valid, it is as close as programmatically possible (AFAIK). See caveats below.Logic
When a
:valid_classis specified, it will be applied to an input wrapper given the following conditions::errors.Changes
:valid_classdeclarations (similar to those for:error_class) on default wrappers.:has_value?and:valid?toSimpleForm::Components::Errors.SimpleForm::Wrappers::Root#html_classesto apply:valid_classwhen applicable.Known caveats
If an attribute is valid when
:blank?, the:valid_classwill not be applied. I am open to suggestions here, as this is the only hole in my logic, albeit an acceptable and predictable one (IMO).Further notes
I adjusted two failing tests that were missing stubs, and added another test for this feature.
Please let me know if there's something else I should include. Thanks, cheers!