From 1dcea62c2d7025f21acc3ec3cca8d0ea7dd287e9 Mon Sep 17 00:00:00 2001 From: Przemek Kaminski Date: Tue, 5 Nov 2013 11:22:29 +0100 Subject: [PATCH 1/3] ngRepeat: insert end comment only after last element, not after each iterated item This caused a subtle bug in angular-sortable directive: when moving elements between lists, previousNode was a comment, which doesn't have the .parentNode attribute so the $animate.enter function tried to call insertBefore of a null. Moreover, it looks bad that 'end ngRepeat' comment appears after every iterated node, it should be just at the end of the repeat block. --- src/ng/directive/ngRepeat.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 9e5ad2f5a6ad..d04603037e17 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -379,7 +379,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { if (!block.startNode) { linker(childScope, function(clone) { - clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' '); + if(childScope.$last) { + clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' '); + } $animate.enter(clone, null, jqLite(previousNode)); previousNode = clone; block.scope = childScope; From 7cbb092fdadb5c4b2f5602e7b3bfba9826a7ccd5 Mon Sep 17 00:00:00 2001 From: Przemek Kaminski Date: Tue, 5 Nov 2013 11:51:16 +0100 Subject: [PATCH 2/3] ngRepeat: fix test spec after removing end comment --- test/ng/directive/ngRepeatSpec.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 9dde36e7cc24..c999099810bc 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -621,7 +621,6 @@ describe('ngRepeat', function() { '
' + '' + '
1|
' + - '' + '
2|
' + '' + '
' @@ -653,7 +652,6 @@ describe('ngRepeat', function() { '
' + '' + '
1|
' + - '' + '
2|
' + '' + '
' @@ -759,9 +757,7 @@ describe('ngRepeat', function() { // Note: COMMENT_NODE === 8 expect(children[0].nextSibling.nodeType).toBe(8); - expect(children[0].nextSibling.nodeValue).toBe(' end ngRepeat: val in values '); expect(children[1].nextSibling.nodeType).toBe(8); - expect(children[1].nextSibling.nodeValue).toBe(' end ngRepeat: val in values '); expect(children[2].nextSibling.nodeType).toBe(8); expect(children[2].nextSibling.nodeValue).toBe(' end ngRepeat: val in values '); } From b038c2b3ea219147934e65853a99a16b59549492 Mon Sep 17 00:00:00 2001 From: Przemek Kaminski Date: Wed, 6 Nov 2013 08:57:00 +0100 Subject: [PATCH 3/3] ngRepeat: fix (almost all) tests after end comment removal --- test/BinderSpec.js | 11 +---------- test/ng/directive/ngClassSpec.js | 8 ++++---- test/ng/directive/ngRepeatSpec.js | 32 ++++++++++++++++++++++++++++--- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/test/BinderSpec.js b/test/BinderSpec.js index b553c68dcfd0..b8b95f31bd3a 100644 --- a/test/BinderSpec.js +++ b/test/BinderSpec.js @@ -96,7 +96,6 @@ describe('Binder', function() { '
    ' + '' + '
  • A
    • ' + - '' + '
  • B
    • ' + '' + '
'); @@ -107,9 +106,7 @@ describe('Binder', function() { '
    ' + '' + '
  • C
    • ' + - '' + '
  • A
    • ' + - '' + '
  • B
    • ' + '' + '
'); @@ -120,7 +117,6 @@ describe('Binder', function() { '
    ' + '' + '
  • A
    • ' + - '' + '
  • B
    • ' + '' + '
'); @@ -231,15 +227,12 @@ describe('Binder', function() { '
'+ '' + '
    '+ - '' + '
      '+ '' + '
      '+ - '' + '
      '+ '' + '
        '+ - '' + '
          '+ '' + '
          ' + @@ -322,14 +315,13 @@ describe('Binder', function() { $rootScope.$apply(); var d1 = jqLite(element[0].childNodes[1]); - var d2 = jqLite(element[0].childNodes[3]); + var d2 = jqLite(element[0].childNodes[2]); expect(d1.hasClass('o')).toBeTruthy(); expect(d2.hasClass('e')).toBeTruthy(); expect(sortedHtml(element)).toBe( '
          ' + '' + '
          ' + - '' + '
          ' + '' + '
          '); @@ -437,7 +429,6 @@ describe('Binder', function() { '
            ' + '' + '
          • a0
            • ' + - '' + '
          • b1
            • ' + '' + '
          '); diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js index a788e45255b5..0db321cb7386 100644 --- a/test/ng/directive/ngClassSpec.js +++ b/test/ng/directive/ngClassSpec.js @@ -166,7 +166,7 @@ describe('ngClass', function() { element = $compile('
            • ')($rootScope); $rootScope.$digest(); var e1 = jqLite(element[0].childNodes[1]); - var e2 = jqLite(element[0].childNodes[3]); + var e2 = jqLite(element[0].childNodes[2]); expect(e1.hasClass('existing')).toBeTruthy(); expect(e1.hasClass('odd')).toBeTruthy(); expect(e2.hasClass('existing')).toBeTruthy(); @@ -181,7 +181,7 @@ describe('ngClass', function() { '
                ')($rootScope); $rootScope.$apply(); var e1 = jqLite(element[0].childNodes[1]); - var e2 = jqLite(element[0].childNodes[3]); + var e2 = jqLite(element[0].childNodes[2]); expect(e1.hasClass('plainClass')).toBeTruthy(); expect(e1.hasClass('odd')).toBeTruthy(); @@ -199,7 +199,7 @@ describe('ngClass', function() { '
                  ')($rootScope); $rootScope.$apply(); var e1 = jqLite(element[0].childNodes[1]); - var e2 = jqLite(element[0].childNodes[3]); + var e2 = jqLite(element[0].childNodes[2]); expect(e1.hasClass('A')).toBeTruthy(); expect(e1.hasClass('B')).toBeTruthy(); @@ -273,7 +273,7 @@ describe('ngClass', function() { $rootScope.$digest(); var e1 = jqLite(element[0].childNodes[1]); - var e2 = jqLite(element[0].childNodes[3]); + var e2 = jqLite(element[0].childNodes[2]); expect(e1.hasClass('odd')).toBeTruthy(); expect(e1.hasClass('even')).toBeFalsy(); diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index c999099810bc..f55239d81701 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -750,14 +750,12 @@ describe('ngRepeat', function() { }); - it('should add separator comments after each item', inject(function ($compile, $rootScope) { + it('should add separator comment after end of block', inject(function ($compile, $rootScope) { var check = function () { var children = element.find('div'); expect(children.length).toBe(3); // Note: COMMENT_NODE === 8 - expect(children[0].nextSibling.nodeType).toBe(8); - expect(children[1].nextSibling.nodeType).toBe(8); expect(children[2].nextSibling.nodeType).toBe(8); expect(children[2].nextSibling.nodeValue).toBe(' end ngRepeat: val in values '); } @@ -1030,6 +1028,34 @@ describe('ngRepeat', function() { })); + it('should properly react to list change', inject(function ($compile, $rootScope) { + $rootScope.values = [1, 2, 3]; + $rootScope.showMe = true; + + element = $compile( + '
                  ' + + '
                  val:{{val}};
                  ' + + '
                  val-end:{{val}};
                  ' + + '
                  ' + )($rootScope); + + $rootScope.$digest(); + expect(element.find('div').length).toBe(6); + + $rootScope.values.shift(); + $rootScope.values.push(4); + + $rootScope.$digest(); + console.log(element.html()); + console.log($rootScope.values); + expect(element.find('div').length).toBe(6); + expect(element.text()).not.toContain('val:1;'); + expect(element.text()).not.toContain('val-end:1;'); + expect(element.text()).toContain('val:4;'); + expect(element.text()).toContain('val-end:4;'); + })); + + it('should not clobber ng-if when updating collection', inject(function ($compile, $rootScope) { $rootScope.values = [1, 2, 3]; $rootScope.showMe = true;