-
Notifications
You must be signed in to change notification settings - Fork 27.4k
$isEmpty
fixes
#10164
$isEmpty
fixes
#10164
Conversation
CLAs look good, thanks! |
5f61240
to
187654c
Compare
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
@@ -1812,7 +1816,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ | |||
* default. The `checkboxInputType` directive does this because in its case a value of `false` | |||
* implies empty. | |||
* | |||
* @param {*} value Model value to check. | |||
* @param {*} value Reference to check. |
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.
Reference is a little too vague for my taste, especially since it should always be the $viewValue that is checked.
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.
OK, that was just the straight revert of Tobias's commit
So if I fix up the |
Yes, LGTM. |
This commit tried to create consistency by ensuring that `$isEmpty` is not called on both model and view values but it chose to only use `$modelValue`, which is not actually correct. `$isEmpty` is designed to compute whether the `$viewValue` is empty. In practice this is the only part of the parse/format system that the directive has control over. We can't rely on the `$modelValue` being in any particular format since other directives can add in their own formatters and parsers to completely change this. (reverted from commit 3e51b84)
187654c
to
42d09f1
Compare
We should only use $isEmpty on $viewValue not for validating models.