Skip to content

Commit 3200e3b

Browse files
matskomatsko
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 angular#11196
1 parent bfd7b22 commit 3200e3b

File tree

2 files changed

+71
-42
lines changed

2 files changed

+71
-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

+64-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,55 @@ 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 retain the messages include container within the children of ngMessages and make sure the order is the same',
455+
{'<div ng-messages-include="...">': '<div ng-messages="data" ng-messages-multiple="true">' +
456+
'<div ng-message="0">0</div>' +
457+
'<div ng-messages-include="abc.html"></div>' +
458+
'<div ng-message="X">X</div>' +
459+
'</div>',
460+
'<ng-messages-include src="...">': '<ng-messages for="data" multiple="true">' +
461+
'<div ng-message="0">0</div>' +
462+
'<ng-messages-include src="abc.html"></ng-messages-include>' +
463+
'<div ng-message="X">X</div>' +
464+
'</ng-messages>'},
465+
function(html) {
466+
inject(function($compile, $rootScope, $templateCache) {
467+
$templateCache.put('abc.html', '<div ng-message="a">A</div>' +
468+
'<div ng-message="b">B</div>' +
469+
'<div ng-message="c">C</div>');
470+
471+
element = $compile(html)($rootScope);
472+
473+
var includeContainer = element[0].querySelector('ng-messages-include, [ng-messages-include]');
474+
expect(includeContainer).toBeTruthy();
475+
expect(includeContainer.parentNode).toBe(element[0]);
476+
477+
$rootScope.$apply(function() {
478+
$rootScope.data = {
479+
'0': true,
480+
'a': 1,
481+
'b': 2,
482+
'c': 3,
483+
'X': true
484+
};
485+
});
486+
487+
expect(messageChildren(includeContainer).length).toBe(3);
488+
expect(messageChildren(element).length).toBe(5);
489+
490+
var children = element.children();
491+
var firstElement = children[0];
492+
var lastElement = children[children.length - 1];
493+
expect(firstElement).toBe(includeContainer.previousElementSibling);
494+
expect(lastElement).toBe(includeContainer.nextElementSibling);
495+
});
496+
});
497+
454498
it('should cache the template after download',
455499
inject(function($rootScope, $compile, $templateCache, $httpBackend) {
456500

@@ -484,13 +528,13 @@ describe('ngMessages', function() {
484528

485529
$rootScope.$digest();
486530

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

490534
$httpBackend.flush();
491535
$rootScope.$digest();
492536

493-
expect(element.children().length).toBe(1);
537+
expect(messageChildren(element).length).toBe(1);
494538
expect(trim(element.text())).toEqual("You did not enter a value");
495539
}));
496540

@@ -515,7 +559,7 @@ describe('ngMessages', function() {
515559
};
516560
});
517561

518-
expect(element.children().length).toBe(1);
562+
expect(messageChildren(element).length).toBe(1);
519563
expect(trim(element.text())).toEqual("AAA");
520564

521565
$rootScope.$apply(function() {
@@ -525,7 +569,7 @@ describe('ngMessages', function() {
525569
};
526570
});
527571

528-
expect(element.children().length).toBe(1);
572+
expect(messageChildren(element).length).toBe(1);
529573
expect(trim(element.text())).toEqual("B");
530574

531575
$rootScope.$apply(function() {
@@ -534,7 +578,7 @@ describe('ngMessages', function() {
534578
};
535579
});
536580

537-
expect(element.children().length).toBe(1);
581+
expect(messageChildren(element).length).toBe(1);
538582
expect(trim(element.text())).toEqual("C");
539583
}));
540584

@@ -584,14 +628,14 @@ describe('ngMessages', function() {
584628
};
585629
});
586630

587-
expect(element.children().length).toBe(2);
631+
expect(messageChildren(element).length).toBe(2);
588632
expect(s(element.text())).toEqual("XZ");
589633

590634
$rootScope.$apply(function() {
591635
$rootScope.data.y = {};
592636
});
593637

594-
expect(element.children().length).toBe(3);
638+
expect(messageChildren(element).length).toBe(3);
595639
expect(s(element.text())).toEqual("XYZ");
596640
}));
597641

@@ -616,14 +660,14 @@ describe('ngMessages', function() {
616660
};
617661
});
618662

619-
expect(element.children().length).toBe(2);
663+
expect(messageChildren(element).length).toBe(2);
620664
expect(s(element.text())).toEqual("ZZZX");
621665

622666
$rootScope.$apply(function() {
623667
$rootScope.data.y = {};
624668
});
625669

626-
expect(element.children().length).toBe(3);
670+
expect(messageChildren(element).length).toBe(3);
627671
expect(s(element.text())).toEqual("YYYZZZX");
628672
}));
629673
});

0 commit comments

Comments
 (0)