feat(ngMessages): add support for default message #16587
Conversation
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
1 similar comment
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
Narretz
force-pushed
the
feat-ngMessage-default
branch
from
74298b6
to
b96700c
Compare
| delete messages[key]; | ||
| this.deregister = function(comment, isDefault) { | ||
| if (isDefault) { | ||
| delete ctrl.default; |
There was a problem hiding this comment.
In the og PR you wondered if we need to detach the default controller first here @gkalpak - I've tested this and in defautl circumstances it definitely gets detached.
Added support for showing default message when no values are mapped with ng-message. Closes angular#12008
Narretz
force-pushed
the
feat-ngMessage-default
branch
from
b96700c
to
62b285d
Compare
src/ngMessages/messages.js
Outdated
| * more about ngAnimate. | ||
| * | ||
| * ## Displaying a default message | ||
| * If the ngMessages renders no inner ngMessage directive (that is to say when the key values does not |
There was a problem hiding this comment.
The wording here does not sound right. Should it be "(that is to say when no value matches any of the ngMessage directives)"?
There was a problem hiding this comment.
Yeah, it's not correct. It should be something like "when a key in the ngMessages collection is truthy, but no matching message is defined"
|
I just noticed that the behavior of the default message is not correct, at least not how it was requested in the original issue: #12008 However, that raises the question how ngMessageDefault should deal with multiple errors (ng-message-multiple) when some errors are defined and others are not. |
src/ngMessages/messages.js
Outdated
|
|
||
| // Set default message if keys in collection do not match any message | ||
| if (ctrl.default && truthyKeys > 0) ctrl.default.attach(); | ||
|
|
||
| $animate.setClass($element, INACTIVE_CLASS, ACTIVE_CLASS); |
There was a problem hiding this comment.
This line should only be called if there is no default... Or perhaps this whole block should look like:
var messageMatched = unmatchedMessages.length !== totalMessages;
if (ctrl.default) {
if (messageMatched) {
ctrl.default.detach();
} else if (truthyKeys > 0) {
ctrl.default.attach();
}
}
if (messageMatched || ctrl.default) {
$animate.setClass($element, ACTIVE_CLASS, INACTIVE_CLASS);
} else {
$animate.setClass($element, INACTIVE_CLASS, ACTIVE_CLASS);
}
test/ngMessages/messagesSpec.js
Outdated
| @@ -1,6 +1,6 @@ | |||
| 'use strict'; | |||
|
|
|||
| describe('ngMessages', function() { | |||
| fdescribe('ngMessages', function() { | |||
Added support for showing default message when no values are mapped with ng-message.
Closes #12008
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change?
Please check if the PR fulfills these requirements
Other information: