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

feat(ngMessages): add support for default message #12213

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 115 additions & 40 deletions src/ngMessages/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var jqLite = angular.element;
* sequencing based on the order of how the messages are defined in the template.
*
* Currently, the ngMessages module only contains the code for the `ngMessages`, `ngMessagesInclude`
* `ngMessage` and `ngMessageExp` directives.
* `ngMessage`, `ngMessageExp` and `ngMessageDefault` directives.
*
* # Usage
* The `ngMessages` directive listens on a key/value collection which is set on the ngMessages attribute.
Expand Down Expand Up @@ -239,6 +239,19 @@ var jqLite = angular.element;
* .some-message.ng-leave.ng-leave-active {}
* ```
*
* ## Displaying a default message
* If the ngMessages renders no inner ngMessage directive (that is to say when the key values does not
* match the attribute value present on each ngMessage directive), then it will render a default message
* using the ngMessageDefault directive.
*
* ```html
* <div ng-messages="myForm.myField.$error" role="alert">
* <div ng-message="required">This field is required</div>
* <div ng-message="minlength">This field is too short</div>
* <div ng-message-default>This is a default message</div>
Copy link
Member

Choose a reason for hiding this comment

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

I would use a more meaningful message here to give a better impression of how to use the directive.

Copy link

Choose a reason for hiding this comment

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

Anything in specific that should be mentioned here?

Copy link
Member

Choose a reason for hiding this comment

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

Anything that you would use in an actual app. In this case, since these seem to be validation messages, the default message would be something like 'Valid' or a checkmark or 'This field is valid` etc.

* </div>
* ```
*
* {@link ngAnimate Click here} to learn how to use JavaScript animations or to learn more about ngAnimate.
*/
angular.module('ngMessages', [])
Expand All @@ -260,6 +273,9 @@ angular.module('ngMessages', [])
* at a time and this depends on the prioritization of the messages within the template. (This can
* be changed by using the `ng-messages-multiple` or `multiple` attribute on the directive container.)
*
* A default message can also be displayed when no `ngMessage` directive is inserted, using the
* `ngMessageDefault` directive.
*
* A remote template can also be used to promote message reusability and messages can also be
* overridden.
*
Expand All @@ -272,13 +288,15 @@ angular.module('ngMessages', [])
* <ANY ng-message="stringValue">...</ANY>
* <ANY ng-message="stringValue1, stringValue2, ...">...</ANY>
* <ANY ng-message-exp="expressionValue">...</ANY>
* <ANY ng-message-default>...</ANY>
* </ANY>
*
* <!-- or by using element directives -->
* <ng-messages for="expression" role="alert">
* <ng-message when="stringValue">...</ng-message>
* <ng-message when="stringValue1, stringValue2, ...">...</ng-message>
* <ng-message when-exp="expressionValue">...</ng-message>
* <ng-message-default>...</ng-message-default>
* </ng-messages>
* ```
*
Expand Down Expand Up @@ -307,6 +325,7 @@ angular.module('ngMessages', [])
* <div ng-message="required">You did not enter a field</div>
* <div ng-message="minlength">Your field is too short</div>
* <div ng-message="maxlength">Your field is too long</div>
* <div ng-message-default>This is a default message</div>
* </div>
* </form>
* </file>
Expand Down Expand Up @@ -379,9 +398,17 @@ angular.module('ngMessages', [])
messageCtrl.detach();
});

unmatchedMessages.length !== totalMessages
? $animate.setClass($element, ACTIVE_CLASS, INACTIVE_CLASS)
: $animate.setClass($element, INACTIVE_CLASS, ACTIVE_CLASS);
if (unmatchedMessages.length !== totalMessages) {
// Unset default message if setted
Copy link
Member

Choose a reason for hiding this comment

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

setted --> set

Copy link

Choose a reason for hiding this comment

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

Not sure I follow...?

Copy link
Member

Choose a reason for hiding this comment

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

X --> Y is my way of telling that there is a typo in X, it should be changed to Y 😛
(Basically, change setted to set.)

if (ctrl.default) { ctrl.default.detach(); }

$animate.setClass($element, ACTIVE_CLASS, INACTIVE_CLASS);
} else {
// Set default message when no other one matched
if (ctrl.default) { ctrl.default.attach(); }

$animate.setClass($element, INACTIVE_CLASS, ACTIVE_CLASS);
}
};

$scope.$watchCollection($attrs.ngMessages || $attrs['for'], ctrl.render);
Expand All @@ -397,23 +424,32 @@ angular.module('ngMessages', [])
}
};

this.register = function(comment, messageCtrl) {
var nextKey = latestKey.toString();
messages[nextKey] = {
message: messageCtrl
};
insertMessageNode($element[0], comment, nextKey);
comment.$$ngMessageNode = nextKey;
latestKey++;
this.register = function(comment, messageCtrl, isDefault) {
if (isDefault) {
ctrl.default = messageCtrl;
Copy link
Member

Choose a reason for hiding this comment

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

Wrong indentation (here and below).

} else {
var nextKey = latestKey.toString();
messages[nextKey] = {
message: messageCtrl
};
insertMessageNode($element[0], comment, nextKey);
comment.$$ngMessageNode = nextKey;
latestKey++;
}

ctrl.reRender();
};

this.deregister = function(comment) {
var key = comment.$$ngMessageNode;
delete comment.$$ngMessageNode;
removeMessageNode($element[0], comment, key);
delete messages[key];
this.deregister = function(comment, isDefault) {
if (isDefault) {
delete ctrl.default;
Copy link
Member

Choose a reason for hiding this comment

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

I think you also need to detach ctrl.default (if defined), because if it is visible, it won't get removed by ctrl.reRender().

Copy link

Choose a reason for hiding this comment

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

Do you mean to call removeMessageNode here for the default?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think removeMessageNode() works for ctrl.default. I was thinking ctrl.default.detach().
And there should also be a test about dynamically adding/removing an <ng-message-default> element (e.g. using ngIf).

} else {
var key = comment.$$ngMessageNode;
delete comment.$$ngMessageNode;
removeMessageNode($element[0], comment, key);
delete messages[key];
}

ctrl.reRender();
};

Expand Down Expand Up @@ -594,35 +630,74 @@ angular.module('ngMessages', [])
*
* @param {expression} ngMessageExp|whenExp an expression value corresponding to the message key.
*/
.directive('ngMessageExp', ngMessageDirectiveFactory('A'));
.directive('ngMessageExp', ngMessageDirectiveFactory('A'))

/**
* @ngdoc directive
* @name ngMessageDefault
* @restrict AE
* @scope
*
* @description
* `ngMessageDefault` is a directive with the purpose to show and hide a default message.
* For `ngMessageDefault` to operate, no `ngMessage` inner directive should be displayed
* in the parent `ngMessages` directive.
*
* More information about using `ngMessageDefault` can be found in the
* {@link module:ngMessages `ngMessages` module documentation}.
*
* @usage
* ```html
* <!-- using attribute directives -->
* <ANY ng-messages="expression" role="alert">
* <ANY ng-message="stringValue">...</ANY>
* <ANY ng-message="stringValue1, stringValue2, ...">...</ANY>
* <ANY ng-message-default>...</ANY>
* </ANY>
*
* <!-- or by using element directives -->
* <ng-messages for="expression" role="alert">
* <ng-message when="stringValue">...</ng-message>
* <ng-message when="stringValue1, stringValue2, ...">...</ng-message>
* <ng-message-default>...</ng-message-default>
* </ng-messages>
* ```
*
* @param {expression} ngMessageDefault|when no ngMessage matches.
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't take any params.

*/
.directive('ngMessageDefault', ngMessageDirectiveFactory('AE', true));

function ngMessageDirectiveFactory(restrict) {

function ngMessageDirectiveFactory(restrict, isDefault) {
return ['$animate', function($animate) {
return {
restrict: 'AE',
transclude: 'element',
terminal: true,
require: '^^ngMessages',
link: function(scope, element, attrs, ngMessagesCtrl, $transclude) {
var commentNode = element[0];

var records;
var staticExp = attrs.ngMessage || attrs.when;
var dynamicExp = attrs.ngMessageExp || attrs.whenExp;
var assignRecords = function(items) {
records = items
? (isArray(items)
? items
: items.split(/[\s,]+/))
: null;
ngMessagesCtrl.reRender();
};

if (dynamicExp) {
assignRecords(scope.$eval(dynamicExp));
scope.$watchCollection(dynamicExp, assignRecords);
} else {
assignRecords(staticExp);
var commentNode, records, staticExp, dynamicExp;

if (!isDefault) {
commentNode = element[0];
staticExp = attrs.ngMessage || attrs.when;
dynamicExp = attrs.ngMessageExp || attrs.whenExp;

var assignRecords = function(items) {
records = items
? (isArray(items)
? items
: items.split(/[\s,]+/))
: null;
ngMessagesCtrl.reRender();
};

if (dynamicExp) {
assignRecords(scope.$eval(dynamicExp));
scope.$watchCollection(dynamicExp, assignRecords);
} else {
assignRecords(staticExp);
}
}

var currentElement, messageCtrl;
Expand All @@ -641,7 +716,7 @@ function ngMessageDirectiveFactory(restrict) {
// to deregister the message from the controller
currentElement.on('$destroy', function() {
if (currentElement) {
ngMessagesCtrl.deregister(commentNode);
ngMessagesCtrl.deregister(commentNode, isDefault);
messageCtrl.detach();
}
});
Expand All @@ -655,7 +730,7 @@ function ngMessageDirectiveFactory(restrict) {
$animate.leave(elm);
}
}
});
}, isDefault);
}
};
}];
Expand Down
18 changes: 18 additions & 0 deletions test/ngMessages/messagesSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,24 @@ describe('ngMessages', function() {
});
});

it('should render a default message when no one matched', inject(function($rootScope, $compile) {
element = $compile('<div ng-messages="col">' +
' <div ng-message="val">Message is set</div>' +
' <div ng-message-default>Default message is set</div>' +
'</div>')($rootScope);
$rootScope.$digest();

//expect(element.text()).not.toContain('Message is set');
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment.

expect(element.text()).toContain('Default message is set');
Copy link
Member

Choose a reason for hiding this comment

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

Why not use .toBe()?


$rootScope.$apply(function() {
$rootScope.col = { val: true };
});
Copy link
Member

Choose a reason for hiding this comment

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

$rootScope.$apply('col = {val: true}') is more concise 😃


//expect(element.text()).toContain('Message is set');
expect(element.text()).not.toContain('Default message is set');
Copy link
Member

Choose a reason for hiding this comment

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

Again, why not use .toBe() and verify the exact contents?

}));

describe('when including templates', function() {
they('should work with a dynamic collection model which is managed by ngRepeat',
{'<div ng-messages-include="...">': '<div ng-messages="item">' +
Expand Down