Skip to content
This repository was archived by the owner on Sep 8, 2020. It is now read-only.

(ng1.2) nested ng-repeat not rendering correctly #47

Closed
hlandao opened this issue Nov 1, 2013 · 17 comments
Closed

(ng1.2) nested ng-repeat not rendering correctly #47

hlandao opened this issue Nov 1, 2013 · 17 comments
Labels

Comments

@hlandao
Copy link

hlandao commented Nov 1, 2013

Hi,

I have a nested ng-repeat, the inner is ui-sortable, markup looks like this :

<div class="apps-container screen"  ng-model="screen"  ng-repeat="screen in rawScreens">
<div class="app" ng-repeat="app in screen">{{$index}}</div>
</div>

Sometimes, after dragging and dropping, the ng-repeat messes up the order while the rendered div with the $index has the correct values.
I added an alert before the $scope.$apply at the end of the onStop function in sortable.js, before the $scope.apply is invoked, the order of the divs is the same as the expected result. After invoking scope.$apply, the order changed, while the indexes are the same as the first state.

Does anyone know what is the issue ?

Thanks!

@thgreasi
Copy link
Contributor

thgreasi commented Nov 1, 2013

Please also provide the uiSortable element or even better a fiddle demonstrating your problem.

First of all note that the model you provide to the uiSortable element, has to match with the model that is used to produce the DOM elements inside it.
(The index of the elements is used to update the model after a reorder occurs).

The obvious way to fulfill the above requirement is to use one ngRepeater inside the uiSortable element and provide the same ngModel to both of them.

The other way (the harder one) is to split the outer model to smaller ones inside the controller, and then provide them to the repeaters (if you know what you are doing).

You should probably try the first approach and use a separate uiSortable or element for each repeater that you are using.

Finally, some words about apply().
Inside the uiSortable's implementation of the stop() callback, the ngModel's items get reordered according to the user actions. After this, apply() is called to update the DOM elements based on the new model. Since the uiSortable's model was updated and marked as dirty, each dirty repeater recreates the DOM elements based on its own model.

@hlandao
Copy link
Author

hlandao commented Nov 1, 2013

This is the 2 level array using to render the 2-level ng-repeat.

$scope.rawScreens = [
        [
            {icon : './img/icons/facebook.jpg', title: 'Facebook', link : 'http://www.facebook.com'},
            {icon : './img/icons/youtube.jpg', title: 'Youtube', link : 'http://www.youtube.com'},
            {icon : './img/icons/gmail.jpg', title: 'Gmail', link : 'http://www.gmail.com'},
            {icon : './img/icons/google+.jpg', title: 'Google+', link : 'http://plus.google.com'},
            {icon : './img/icons/twitter.jpg', title: 'Twitter', link : 'http://www.twitter.com'},
            {icon : './img/icons/yahoomail.jpg', title: 'Yahoo Mail', link : 'http://mail.yahoo.com'},
            {icon : './img/icons/pinterest.jpg', title: 'Pinterest', link : 'http://www.pinterest.com'}
        ],
        [
            {icon : './img/icons/facebook.jpg', title: 'Facebook', link : 'http://www.facebook.com'},
            {icon : './img/icons/youtube.jpg', title: 'Youtube', link : 'http://www.youtube.com'},
            {icon : './img/icons/gmail.jpg', title: 'Gmail', link : 'http://www.gmail.com'},
            {icon : './img/icons/google+.jpg', title: 'Google+', link : 'http://plus.google.com'},
            {icon : './img/icons/twitter.jpg', title: 'Twitter', link : 'http://www.twitter.com'},
            {icon : './img/icons/yahoomail.jpg', title: 'Yahoo Mail', link : 'http://mail.yahoo.com'},
            {icon : './img/icons/pinterest.jpg', title: 'Pinterest', link : 'http://www.pinterest.com'}
        ]
    ];

The 2nd level array is used as sortable (its children are getting the drag&drop usability).

This is the ui-sortable-options :

   $scope.sortableOptions = {
        disabled: false,
        start: function (e, u) {
            var uiItemJ = $(u.item);
            uiItemJ.appendTo(uiItemJ.parent());
            $(u.helper).parent().addClass('edit');

            $(u.helper).addClass("dragging");
        },
        stop: function (e, u) {
            $(u.item).removeClass("dragging");
            $(u.item).parent().removeClass('edit');
        },
        placeholder: "app",
        revert: 500,
        opacity: .75,
        helper: "clone",
        over: function (e, u) {
        },
        sort: function (e, u) {
        },
        receive: function (e, u) {
        },
        connectWith: ".apps-container"
    };

If I cancel the $scope.apply inside the on-stop - the items are properly organized.

Thanks,
Hadar.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 1, 2013

In the case that you are canceling apply(), do the models of your app get properly updated?

Do you need to move items between different sub arrays?

Something like this is not in the general use case. In any case I would suggest you use no more than ONE repeater inside each uiSortable element.

@hlandao
Copy link
Author

hlandao commented Nov 1, 2013

Regardless if the scope.$apply is called at the end of the onStop , models are sorted properly (they onStop sorting of the models works correctly). Though my final target is to move items between sub arrays, the problems
occures when items are moved within their parents array. Actually, moving
items between sub arrays work properly.
On Nov 1, 2013 9:19 PM, "Thodoris Greasidis" [email protected]
wrote:

In the case that you are canceling apply(), do the models of your app get
properly updated?

Do you need to move items between different sub arrays?

Something like this is not in the general use case. In any case I would
suggest you use no more than ONE repeater inside each uiSortable element.


Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-27593304
.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 2, 2013

Does your use case looks like this pen?
If not, please fork it to properly present the scenario that produces your problem.

@hlandao
Copy link
Author

hlandao commented Nov 3, 2013

Hi, it is very similar, the only differences are the styles applied.
I will fork the pen so we can see if the issue is with the styles because your pen work properly.

Thanks,
Hadar.

@hlandao
Copy link
Author

hlandao commented Nov 3, 2013

I must say that your pen work perfectly.
This is a more similar pen that is more similar to my real code : http://codepen.io/anon/pen/yhnpr.
I use -webkit-transform to sort the items so that I will gain a smooth effect when items move.

I don't see such a big difference between your pen and my code - I'll give it a deeper review - maybe I was using a bit older version of sortable.js

Thanks so much,
Hadar.

@hlandao
Copy link
Author

hlandao commented Nov 3, 2013

BTW - why in my pen does the dragged element is always being reverted back to the first place ?

@hlandao
Copy link
Author

hlandao commented Nov 3, 2013

I think I found out the problem - Originally I was using angularjs 1.2 rc, the moment I switched to angularjs 1.0.7 the problem disappeared.

Do you have any idea regarding possible changes in angular1.2 causing such issues ?

Thanks,
Hadar.

@kfiil
Copy link

kfiil commented Nov 5, 2013

I experience something similar using AngularJS 1.2 RC3, if I fork the pen you have posted @thgreasi and change it to use AngularJS 1.2 it has problems to. I believe it must be related to something about both jQuery UI and Angular updating the DOM because the scope values are correct.

P.S.
I have a Plunker showing it in my case but I will not hijack this issue post.

@kfiil
Copy link

kfiil commented Nov 6, 2013

If you remove line 61

ngModel.$modelValue.splice(ui.item.index(), 0, ui.item.sortable.moved);

then the model is of cause not in sync but the view is correct so that call makes a duplicated item in the view when updating the model value

@filipkis
Copy link

filipkis commented Nov 7, 2013

I have a different problem that I think has the same cause - some change in angular 1.2.0rc3 (as it worked with 1.2.0rc2). In my case sometimes when I move elements between the connected list, the list I drop to losses all the items after the drop position.

Debugging reveals that line 69 is to blame:

ui.item.sortable.moved = ngModel.$modelValue.splice(ui.item.sortable.index, 1)[0];

Thus I think something is wrong when splicing the list of values.

Hope somebody figures out the exact problem and proposes a fix soon.

@thgreasi
Copy link
Contributor

thgreasi commented Nov 7, 2013

@kfiil if the model is not updated, then angular will not update the DOM (because of the repeater model getting dirty). So the final correct dom that occurs after removing that line is because angular didnt act at all (neither model nor Dom changes).

Since after a drop, DOM gets updated by both jquery-ui and angular, perfect synchronization is needed (with angular acting after jquery, after the stop callback). It looks like that an extra update occurs earlier in 1.2.

@kfiil
Copy link

kfiil commented Nov 7, 2013

@thgreasi of cause, when I wrote that comment I was fairly early in my debugging  The main problem is, as you say, that jQuery UI updates the DOM and then when we update the model using $modelValue Angular also updates the DOM. My problem with the duplicated item seems to come from the _enter function of animate in angular.js line 3569 - 3602 mostly line 3600 where parentNode already contains the node because of jQuery UI.

The problem you @filipkis mention, I don’t have in my project but if I run the demo with angular 1.2 rc3 I see the same thing and sometimes when I sort a list all the elements gets reordered after a drop(makes sense?) and if I sort between two lists the element gets add twice.

@thgreasi any ideas on how we can solve the problems? I will gladly help if I can! Right now I have a quick fix where I remove the jQuery element from the DOM in the stop event if it is a sort between lists but that feels hackish 😃

@thgreasi
Copy link
Contributor

thgreasi commented Nov 9, 2013

Updated the title to indicate this is related to angular 1.2

@thgreasi
Copy link
Contributor

thgreasi commented Dec 1, 2013

Opened PR #65 that handles placeholders better.
Please test with your use case (example pen).

Possible 'hard points' of your pen:

  • placeholder class equals the class of repeatable's items
  • DOM manipulation inside start & stop

@kfiil
Copy link

kfiil commented Dec 2, 2013

@thgreasi it seems to be working for me! I had "helper: "clone"" added to the configuration that do not work it inserts double elements but without it works. Thanks a lot for the effort!!

@thgreasi thgreasi closed this as completed Dec 2, 2013
thgreasi added a commit that referenced this issue Dec 2, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants