Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(ngModel): provide ng-empty and ng-not-empty CSS classes #12848

Merged
merged 2 commits into from
Sep 22, 2015

Conversation

petebacondarwin
Copy link
Contributor

I believe that this feature should be enough to fix #10050

@gkalpak
Copy link
Member

gkalpak commented Sep 14, 2015

I wonder if people will ask for this to be propagated to a parent FormController...

@petebacondarwin
Copy link
Contributor Author

I was trying not to mention that :-P

@Narretz
Copy link
Contributor

Narretz commented Sep 14, 2015

For the original use case, you would then useng-pristine and ng-not-empty together?

@@ -316,6 +318,17 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
return isUndefined(value) || value === '' || value === null || value !== value;
};

this.$$updateEmptyClasses = function(value) {
if (this.$isEmpty(value)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be ctrl?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really matter because we have control over when it is called but I agree it is probably better for consistency.

@petebacondarwin
Copy link
Contributor Author

Thanks for the reviews @Narretz and @gkalpak. I added a commit.

@petebacondarwin
Copy link
Contributor Author

@Narretz and @gkalpak can you take another look?

@@ -316,6 +318,17 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
return isUndefined(value) || value === '' || value === null || value !== value;
};

this.$$updateEmptyClasses = function(value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be simplified by always using ctrl.$viewValue instead of passing it as an arg. It should only be the viewValue anyway. You'll have to change the order on line https://github.com/angular/angular.js/pull/12848/files#diff-ae1963e24d6328c3aa019c0c1b9f4432R851 though, because otherwise it'll use the old viewValue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it would have to use $$lastCommittedViewValue and we would have to change the order as you suggest. I am not that keen on doing this though as I don't feel that not passing in an argument makes it that much simpler.

@Narretz
Copy link
Contributor

Narretz commented Sep 21, 2015

Regarding my question

For the original use case, you would then use ng-pristine and ng-not-empty together?

because only ng-empty doesn't work if I understood the issue correctly.

@petebacondarwin
Copy link
Contributor Author

@Narretz: Yes, you would use a combination of the CSS classes for the original use case, but this seemed reasonable and intuitive to me.

@petebacondarwin
Copy link
Contributor Author

Thanks for the review @Narretz.
So assuming that we don't simplify the method and that the original use case is covered by a combination of CSS classes. Is this PR ready to merge?

@Narretz
Copy link
Contributor

Narretz commented Sep 21, 2015

Yep, feel free to merge!
Am 21.09.2015 22:45 schrieb "Pete Bacon Darwin" notifications@github.com:

Thanks for the review @Narretz https://github.com/Narretz.
So assuming that we don't simplify the method and that the original use
case is covered by a combination of CSS classes. Is this PR ready to merge?


Reply to this email directly or view it on GitHub
#12848 (comment).

It is no longer appropriate to test this here as $animate takes care of
queueing up CSS class changes into a single update.
If the `$viewValue` is empty then the `ng-empty` CSS class is applied
to the input. Conversely, if it is not empty the `ng-not-empty` CSS class
is applied. Emptiness is ascertained by calling `NgModelController.$isEmpty()`

Closes angular#10050
Closes angular#12848
@petebacondarwin petebacondarwin merged commit 630280c into angular:master Sep 22, 2015
@petebacondarwin petebacondarwin deleted the issue-10050 branch November 24, 2016 09:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: .ng-prefilled class helper
3 participants