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

Strange behavior with ng-repeat-start/end and ng-if in 1.1.6-a22596c #3104

Closed
gsson opened this issue Jul 1, 2013 · 6 comments
Closed

Strange behavior with ng-repeat-start/end and ng-if in 1.1.6-a22596c #3104

gsson opened this issue Jul 1, 2013 · 6 comments

Comments

@gsson
Copy link

gsson commented Jul 1, 2013

When using ng-repeat-start/end to cover two consecutive <tr>s in a table, the latter having an additional ng-if directive the data binding acts really strange. When adding items while ng-if is true, the order of the rows change and if ng-if is subsequently set to false, all new rows are kept in the DOM.

Example: http://da024c74860939fa.paste.se

@gsson
Copy link
Author

gsson commented Jul 16, 2013

Issue still present in angular-1.1.6-031da1f

Here's a plunker: http://plnkr.co/ddTiumaNzxOyhs8iwBKN

@tunguski
Copy link

Found this issue before submitting a bug. In AngularJS v1.1.6-52123ae problem still exists.

For me it looks like all tr's beside first (generated for every element on list used in ng-repeat) stay as orphans in table, when element list is cleared. Repeating list & clear operation, add new set of orphan rows.

But in my case there is ng-show, not ng-if.

@rodyhaddad
Copy link
Contributor

@tunguski it works fine for me with a ng-show (plunkr).
Maybe you're structure was different?


I seem to have understood what's causing the problem with ng-repeat-* combined with ng-if :

  • When $compile finds the ng-repeat-start, it starts a groupScan till it finds ng-repeat-end, and it puts all intermediate nodes into one jqLite collection. That collection will eventually be passed to ng-repeat
  • Then, when $compile finds the ng-if, it sees that the directive has transclude: 'element', and so it replaces the element with a comment node: <!-- ngIf: showA -->. That comment node will eventually be passed to ng-if as its element

Since the element where ng-if is got replaced with a comment, this is the collection that ng-repeat gets:

screen shot 2013-07-25 at 4 01 59 pm
Note the comment instead of ng-repeat-end
(in case you want to know what clone and linker are: here)

So ng-repeat is no longer manipulating a collection that has the ng-repeat-end, it's manipulating a collection that has a comment in it. A comment that ng-if uses to append its element after.

When comes a time for ng-repeat to remove a record, it removes the comment <!-- ngIf: showA --> (but not the element that ng-if appended afterwards!)

You can see this more easily if you just have 1 record at a time: plunkr
As you can see, the last element, the one that ng-if appends, stays, but the comment <!-- ngIf: showA --> gets removed (ng-repeat doing its job)

So the problem really is: when the comment of a directive with transclude: 'element' gets removed, the element that it transcluded doesn't get removed with it (if you were to uncheck the checkbox in the plunkrs before updating, everything works fine)

The best solution to the problem I could come up with is:
Adding a controller to ng-repeat, that way ng-if can know whenever an element it's taking care of is getting moved or removed.
An alternative would be to bake it in jqLite, allowing two sibling elements to be 'connected' with each other, though that would demand a lot of refactoring. Directive controllers seems cleaner imho :)

Edit:
Though with the way that require works for directive, ng-if couldn't ask for the controller of ng-repeat

Edit 2:
Solution for this that passed through my mind till now:

  • Bake it in jqLite (would have to patch it when jQuery is there, we already do something similar for .remove)
  • Bake it in $animate using .data (a bit weird, but does work)
  • Have the transclude function give in a clone variable that is special, so that append/remove works differently if we're transcluding a multi-node collection
  • Something similar to what feat(ngRepeat): use block separator comments #4157 does, but that is generic (baked in $compile), could be combined with the point above

@tunguski
Copy link

Ok, you're right, my case is different. It is probably related to inner ng-repeat. One plnkr will be worth a tousand words:
http://plnkr.co/edit/87CdNzUxtazR08WNgahN?p=preview

  • uncheck "Show A"
  • add element (update)
  • check "Show A"
  • added subelements rows stayed in table

@thenikso
Copy link
Contributor

Probably related with the current issue with ng-if and ng-include demonstrated here and discussed in #3307

@VilemP
Copy link

VilemP commented Sep 30, 2013

same problem in 1-2.0-rc2
here's the plunk: http://plnkr.co/edit/UwKQ1WedB9HjGcoJrdNJ?p=preview

ng-íf behaves unexpectedly, leaving the TR in the table, while if ng-show is used, works as expected

vojtajina pushed a commit to vojtajina/angular.js that referenced this issue Oct 9, 2013
Issue: multi-elements ng-repeat (ng-repeat-start, ng-repeat-end) can contain elements with a trancluding directive. This directive changes content of the row (template) and ng-repeat does not work correctly (when removing/moving rows), because ng-repeat works with the original template (elements).

This changes ng-repeat behavior to traverse the DOM to find current elements everytime we are moving/removing rows (if the template has multiple elements).

Closes angular#3104
@jankuca jankuca closed this as completed in 9efa46a Oct 9, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Issue: multi-elements ng-repeat (ng-repeat-start, ng-repeat-end) can contain elements with a trancluding directive. This directive changes content of the row (template) and ng-repeat does not work correctly (when removing/moving rows), because ng-repeat works with the original template (elements).

This changes ng-repeat behavior to traverse the DOM to find current elements everytime we are moving/removing rows (if the template has multiple elements).

Closes angular#3104
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Issue: multi-elements ng-repeat (ng-repeat-start, ng-repeat-end) can contain elements with a trancluding directive. This directive changes content of the row (template) and ng-repeat does not work correctly (when removing/moving rows), because ng-repeat works with the original template (elements).

This changes ng-repeat behavior to traverse the DOM to find current elements everytime we are moving/removing rows (if the template has multiple elements).

Closes angular#3104
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants