ngRepeat: insert end comment only after last element, not after each iterated item

[CGenie]

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.

[CGenie]

After going through Travis tests I managed to fix all of them except two:

https://travis-ci.org/angular/angular.js/builds/13518556#L1838
https://travis-ci.org/angular/angular.js/builds/13518556#L1841

These are quite tough, since it looks like there actually is some kind of dependency on comments for the ng-repeat directive. In the second case (with ng-if) it seems that it doesn't take into account the last line of the directive, only the ng-if comment. Unfortunately I haven't yet found the source of the problem. Anyone have an idea how to fix this?

[petebacondarwin]

@CGenie - thanks for highlighting your problem. What would be really good is if you could provide a unit test that failed with the current state of the code for your situation. I am guessing that angular-sortable is trying to move nodes around but is confused because there are these comment nodes all over the place?

If so, I wonder if it would be easier to fix angular-sortable to know about these comment nodes? Alternatively what if we changed the compiler so that if there was only one node to be repeated that comment nodes were not used?

[petebacondarwin]

Also can you please have a good read through https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md and ensure that you have signed the CLA.

[CGenie]

@petebacondarwin Basically my most current code is this CGenie@b038c2b
The errors are visible in the Travis build: https://travis-ci.org/angular/angular.js/builds/13567902
It's true: things get mixed up because of the comments: you drag the li element, leaving the comment node behind. I haven't yet figured out how to solve this in ng-repeat, code there is quite complicated :)
In my opinion this is more of Angular issue. I don't think that marking end of a node with a comment node is a good idea. In my opinion it would be better to, for example, mark the last node with some attribute (like 'ng-repeat-end-loop'). Of course when this is really necessary.
Also, as I suggested before: marking after every iteration is misleading: end comment should be placed only at the end of the loop, just like currently the beginning comment is placed at the start of the loop.
Also, I don't think that including internal logic of ng-repeat (and it turns out that comments do belong into this internal logic) into angular-sortable (or in fact any other third-party code) is a good idea.

[egrajeda]

I think I'm having a similar problem.

In my case, the commit 9efa46a is the one that introduced the code that is causing me trouble. If you look at the commit log and the related issue you'll see the rationale behind adding the <!-- end ... comment after each node. I'm working on isolating the cause of the problem, I'll post it when I find it.

[zachsnow]

I also ran into this problem, in a directive that very lightly wraps jquery.dragsort.js. In my case it was causing an HierarchyRequestError (in code in that plugin, not in AngularJS code), so in some limited sense the introduction of those extra comment nodes seems to be a breaking change. In any event we managed to work around the issue by tweaking the plugin.

[IgorMinar]

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

[IgorMinar]

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

[ktgerg]

Any chance this is going to be released soon? I'm seeing this behavior too , it adds a lot of text in the page, especially if it's a long list <!--end ngRepeat: x in x | x | x, after each iteration.

[IgorMinar]

the comment is needed after each iteration so that we can tell where an iteration starts and where it ends in case of multi-element repeaters, or repeaters in which elements get replaced by directives.

I suggest that you let ng-repeat to sort the list rather than have an 3rd party javascript change the dom produced by ng-repeat