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

Commit 68d5bef

Browse files
committed
fix(ngMessages): ensure that the ngMessagesInclude element is persisted in
the DOM NgRepeat and any other directives that alter the DOM structure using transclusion may cause ngMessagesInclude to behave in an unpredictable manner. This fix ensures that the element containing the ngMessagesInclude directive is persisted in the DOM and its downloaded template code will be placed inside of that element. Despite there now being an extra child container within the ngMessages element, the selection process of finding the next message to display still works in the same way. BREAKING CHANGE: From here on the ngMessageInclude directive will be persisted in the dom alongside any adjacent ngMessage elements. The interior ngMessage elements present within the ngMessageInclude template will be stored as child elements. The reasoning for this is to avoid any collision with ngRepeat or any other directive elements that may cause problems with inherited controllers, scopes and transclusion. This patch shouldn't have any effect on your JavaScript code, however when it comes to animations and CSS styling please note that the structure of DOM child elements is different. ```html <!-- before (1.4.0-beta.5 and lower) --> <div ng-message="val"> <div ng-message="error1">...</div> <!-- ng-messages-include="aAndB.html" put this in here --> <div ng-message="errorA">...</div> <div ng-message="errorB">...</div> <div ng-message="error10">...</div> <div> <!-- now --> <div ng-message="val"> <div ng-message="error1">...</div> <!-- ng-messages-include="aAndB.html" put this in here --> <div ng-messages-include="aAndB.html"> <div ng-message="errorA">...</div> <div ng-message="errorB">...</div> </div> <div ng-message="error10">...</div> <div> ``` Closes #11196
1 parent bfd7b22 commit 68d5bef

File tree

2 files changed

+124
-42
lines changed

2 files changed

+124
-42
lines changed

src/ngMessages/messages.js

+7-22
Original file line numberDiff line numberDiff line change
@@ -496,31 +496,16 @@ angular.module('ngMessages', [])
496496
* @param {string} ngMessagesInclude|src a string value corresponding to the remote template.
497497
*/
498498
.directive('ngMessagesInclude',
499-
['$templateRequest', '$document', '$compile', function($templateRequest, $document, $compile) {
499+
['$templateRequest', '$compile', function($templateRequest, $compile) {
500500

501501
return {
502502
restrict: 'AE',
503-
require: '^^ngMessages',
504-
compile: function(element, attrs) {
505-
var comment = jqLite($document[0].createComment(' ngMessagesInclude: '));
506-
element.after(comment);
507-
508-
return function($scope, $element, attrs, ngMessagesCtrl) {
509-
// we're removing this since we only need access to the newly
510-
// created comment node as an anchor.
511-
element.remove();
512-
513-
$templateRequest(attrs.ngMessagesInclude || attrs.src).then(function(html) {
514-
var elements = jqLite('<div></div>').html(html).contents();
515-
var cursor = comment;
516-
forEach(elements, function(elm) {
517-
elm = jqLite(elm);
518-
cursor.after(elm);
519-
cursor = elm;
520-
});
521-
$compile(elements)($scope);
522-
});
523-
};
503+
require: '^^ngMessages', // we only require this for validation sake
504+
link: function($scope, element, attrs) {
505+
$templateRequest(attrs.ngMessagesInclude || attrs.src).then(function(html) {
506+
element.html(html);
507+
$compile(element.contents())($scope);
508+
});
524509
}
525510
};
526511
}])

test/ngMessages/messagesSpec.js

+117-20
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ describe('ngMessages', function() {
44
beforeEach(inject.strictDi());
55
beforeEach(module('ngMessages'));
66

7+
function messageChildren(element) {
8+
return (element.length ? element[0] : element).querySelectorAll('[ng-message], [ng-message-exp]');
9+
}
10+
711
function they(msg, vals, spec, focus) {
812
forEach(vals, function(val, key) {
913
var m = msg.replace('$prop', key);
@@ -346,29 +350,29 @@ describe('ngMessages', function() {
346350
];
347351
});
348352

349-
expect(messageChildren().length).toBe(0);
353+
expect(messageChildren(element).length).toBe(0);
350354
expect(trim(element.text())).toEqual("");
351355

352356
$rootScope.$apply(function() {
353357
$rootScope.col = { hair: true };
354358
});
355359

356-
expect(messageChildren().length).toBe(1);
360+
expect(messageChildren(element).length).toBe(1);
357361
expect(trim(element.text())).toEqual("Your hair is too long");
358362

359363
$rootScope.$apply(function() {
360364
$rootScope.col = { age: true, hair: true};
361365
});
362366

363-
expect(messageChildren().length).toBe(1);
367+
expect(messageChildren(element).length).toBe(1);
364368
expect(trim(element.text())).toEqual("Your age is incorrect");
365369

366370
$rootScope.$apply(function() {
367371
// remove the age!
368372
$rootScope.items.shift();
369373
});
370374

371-
expect(messageChildren().length).toBe(1);
375+
expect(messageChildren(element).length).toBe(1);
372376
expect(trim(element.text())).toEqual("Your hair is too long");
373377

374378
$rootScope.$apply(function() {
@@ -377,12 +381,8 @@ describe('ngMessages', function() {
377381
$rootScope.col.primary = true;
378382
});
379383

380-
expect(messageChildren().length).toBe(1);
384+
expect(messageChildren(element).length).toBe(1);
381385
expect(trim(element.text())).toEqual("Enter something");
382-
383-
function messageChildren() {
384-
return element[0].querySelectorAll('[ng-message], [ng-message-exp]');
385-
}
386386
}));
387387

388388

@@ -437,7 +437,7 @@ describe('ngMessages', function() {
437437
};
438438
});
439439

440-
expect(element.children().length).toBe(1);
440+
expect(messageChildren(element).length).toBe(1);
441441
expect(trim(element.text())).toEqual("A");
442442

443443
$rootScope.$apply(function() {
@@ -446,11 +446,108 @@ describe('ngMessages', function() {
446446
};
447447
});
448448

449-
expect(element.children().length).toBe(1);
449+
expect(messageChildren(element).length).toBe(1);
450450
expect(trim(element.text())).toEqual("C");
451451
});
452452
});
453453

454+
they('should work with a dynamic collection model which is managed by ngRepeat',
455+
{'<div ng-messages-include="...">': '<div ng-messages="item">' +
456+
'<div ng-messages-include="abc.html"></div>' +
457+
'</div>',
458+
'<ng-messages-include src="...">': '<ng-messages for="item">' +
459+
'<ng-messages-include src="abc.html"></ng-messages-include>' +
460+
'</ng-messages>'},
461+
function(html) {
462+
inject(function($compile, $rootScope, $templateCache) {
463+
$templateCache.put('abc.html', '<div ng-message="a">A</div>' +
464+
'<div ng-message="b">B</div>' +
465+
'<div ng-message="c">C</div>');
466+
467+
html = '<div><div ng-repeat="item in items">' + html + '</div></div>';
468+
$rootScope.items = [ {}, {}, {} ];
469+
470+
element = $compile(html)($rootScope);
471+
$rootScope.$apply(function() {
472+
$rootScope.items[0].a = true;
473+
$rootScope.items[1].b = true;
474+
$rootScope.items[2].c = true;
475+
});
476+
477+
var elements = element[0].querySelectorAll('[ng-repeat]');
478+
479+
expect(messageChildren(element).length).toBe(3);
480+
expect(messageChildren(elements[0]).length).toBe(1);
481+
expect(messageChildren(elements[1]).length).toBe(1);
482+
expect(messageChildren(elements[2]).length).toBe(1);
483+
484+
expect(element.text().trim()).toBe('ABC');
485+
486+
$rootScope.$apply(function() {
487+
$rootScope.items[0].a = false;
488+
$rootScope.items[0].c = true;
489+
490+
$rootScope.items[1].b = false;
491+
492+
$rootScope.items[2].c = false;
493+
$rootScope.items[2].a = true;
494+
});
495+
496+
expect(element.text().trim()).toBe('CA');
497+
498+
$rootScope.$apply(function() {
499+
$rootScope.items[1].b = true;
500+
$rootScope.items.reverse();
501+
});
502+
503+
expect(element.text().trim()).toBe('ABC');
504+
});
505+
});
506+
507+
they('should retain the messages include container within the children of ngMessages and make sure the order is the same',
508+
{'<div ng-messages-include="...">': '<div ng-messages="data" ng-messages-multiple="true">' +
509+
'<div ng-message="0">0</div>' +
510+
'<div ng-messages-include="abc.html"></div>' +
511+
'<div ng-message="X">X</div>' +
512+
'</div>',
513+
'<ng-messages-include src="...">': '<ng-messages for="data" multiple="true">' +
514+
'<div ng-message="0">0</div>' +
515+
'<ng-messages-include src="abc.html"></ng-messages-include>' +
516+
'<div ng-message="X">X</div>' +
517+
'</ng-messages>'},
518+
function(html) {
519+
inject(function($compile, $rootScope, $templateCache) {
520+
$templateCache.put('abc.html', '<div ng-message="a">A</div>' +
521+
'<div ng-message="b">B</div>' +
522+
'<div ng-message="c">C</div>');
523+
524+
element = $compile(html)($rootScope);
525+
526+
var includeContainer = element[0].querySelector('ng-messages-include, [ng-messages-include]');
527+
expect(includeContainer).toBeTruthy();
528+
expect(includeContainer.parentNode).toBe(element[0]);
529+
530+
$rootScope.$apply(function() {
531+
$rootScope.data = {
532+
'0': true,
533+
'a': 1,
534+
'b': 2,
535+
'c': 3,
536+
'X': true
537+
};
538+
});
539+
540+
expect(messageChildren(includeContainer).length).toBe(3);
541+
expect(messageChildren(element).length).toBe(5);
542+
543+
var children = element.children();
544+
var firstElement = children[0];
545+
var lastElement = children[children.length - 1];
546+
expect(firstElement).toBe(includeContainer.previousElementSibling);
547+
expect(lastElement).toBe(includeContainer.nextElementSibling);
548+
});
549+
});
550+
454551
it('should cache the template after download',
455552
inject(function($rootScope, $compile, $templateCache, $httpBackend) {
456553

@@ -484,13 +581,13 @@ describe('ngMessages', function() {
484581

485582
$rootScope.$digest();
486583

487-
expect(element.children().length).toBe(1);
584+
expect(messageChildren(element).length).toBe(1);
488585
expect(trim(element.text())).toEqual("Your value is that of failure");
489586

490587
$httpBackend.flush();
491588
$rootScope.$digest();
492589

493-
expect(element.children().length).toBe(1);
590+
expect(messageChildren(element).length).toBe(1);
494591
expect(trim(element.text())).toEqual("You did not enter a value");
495592
}));
496593

@@ -515,7 +612,7 @@ describe('ngMessages', function() {
515612
};
516613
});
517614

518-
expect(element.children().length).toBe(1);
615+
expect(messageChildren(element).length).toBe(1);
519616
expect(trim(element.text())).toEqual("AAA");
520617

521618
$rootScope.$apply(function() {
@@ -525,7 +622,7 @@ describe('ngMessages', function() {
525622
};
526623
});
527624

528-
expect(element.children().length).toBe(1);
625+
expect(messageChildren(element).length).toBe(1);
529626
expect(trim(element.text())).toEqual("B");
530627

531628
$rootScope.$apply(function() {
@@ -534,7 +631,7 @@ describe('ngMessages', function() {
534631
};
535632
});
536633

537-
expect(element.children().length).toBe(1);
634+
expect(messageChildren(element).length).toBe(1);
538635
expect(trim(element.text())).toEqual("C");
539636
}));
540637

@@ -584,14 +681,14 @@ describe('ngMessages', function() {
584681
};
585682
});
586683

587-
expect(element.children().length).toBe(2);
684+
expect(messageChildren(element).length).toBe(2);
588685
expect(s(element.text())).toEqual("XZ");
589686

590687
$rootScope.$apply(function() {
591688
$rootScope.data.y = {};
592689
});
593690

594-
expect(element.children().length).toBe(3);
691+
expect(messageChildren(element).length).toBe(3);
595692
expect(s(element.text())).toEqual("XYZ");
596693
}));
597694

@@ -616,14 +713,14 @@ describe('ngMessages', function() {
616713
};
617714
});
618715

619-
expect(element.children().length).toBe(2);
716+
expect(messageChildren(element).length).toBe(2);
620717
expect(s(element.text())).toEqual("ZZZX");
621718

622719
$rootScope.$apply(function() {
623720
$rootScope.data.y = {};
624721
});
625722

626-
expect(element.children().length).toBe(3);
723+
expect(messageChildren(element).length).toBe(3);
627724
expect(s(element.text())).toEqual("YYYZZZX");
628725
}));
629726
});

0 commit comments

Comments
 (0)