Sortable messes up ng-repeat comments (ng1.2)

[CyborgMaster]

The ng-repeat directive uses comments to mark the beginning and end of repeat sections (at least in the latest version: v1.2.0-rc3). jQuery Sortable (and really all of jQuery for that matter) doesn't pay attention to comments and therefore clobbers them. Some clever work arounds are needed to get them to play together well.

For example, an ng-repeat generated section of the DOM might look as follows:

<div ui-sortable="" ng-model="communicationSetting.filters" class="ui-sortable">
  <!-- ngRepeat: filter in communicationSetting.filters -->
  <div ng-repeat="filter in communicationSetting.filters">
    Group Start
  </div>
  <!-- end ngRepeat: filter in communicationSetting.filters -->
  <div ng-repeat="filter in communicationSetting.filters">
    Future Appointment
  </div>
  <!-- end ngRepeat: filter in communicationSetting.filters -->
  <div ng-repeat="filter in communicationSetting.filters">
    Group End
  </div>
  <!-- end ngRepeat: filter in communicationSetting.filters -->
</div>

After dragging things around a couple of times, the DOM now looks like this:

<div ui-sortable="" ng-model="communicationSetting.filters" class="ui-sortable">
  <div ng-repeat="filter in communicationSetting.filters">
    Group Start
  </div>
  <div ng-repeat="filter in communicationSetting.filters">
    Future Appointment
  </div>
  <div ng-repeat="filter in communicationSetting.filters">
    Group End
  </div>
  <!-- ngRepeat: filter in communicationSetting.filters -->
  <!-- end ngRepeat: filter in communicationSetting.filters -->
  <!-- end ngRepeat: filter in communicationSetting.filters -->
  <!-- end ngRepeat: filter in communicationSetting.filters -->
</div>

Notice how all the ngRepeat comments have ended up at the bottom of the containing div. This causes ng-repeat to get very confused, throw javascript errors and crash when the model is modified. For example: If we now delete the "Group Start" filter, the following error is thrown:

TypeError: Cannot call method 'insertBefore' of null

and all of the filters are removed from the DOM (even though the model is still intact with only the "Group Start" filter removed).

Note: the above experiment was performed even with calling element.sortable('cancel') inside of every sort (which is why the filters ended in the same order), but jQuery.sortable still messed up the comments.

This can be seen either as a weakness of jQuery for not properly handling comment nodes as real nodes, or as a weakness of Angular's ng-repeat for depending on comment nodes for it's functionality (I personally don't think ng-repeat has any business using comment nodes), but either way, we need a work around.

I'm working on one now, will post it in a comment.

[CyborgMaster]

Ok. I think I have it working. The basic idea is we don't allow jQuery.sortable to mess with the DOM at all. We save it when it starts and restore it when it finishes, we then update the model based on the drag endpoint and let ng-repeat finish reordering the elements. It's a little inefficient because we have sortable reorder, then we put it back, then rg-repeat reorders again, but it's the only way I could get it to work at all. I'm open to suggestions about better ways to make it work.

Here is my current sortable.js file:

/*
 jQuery UI Sortable plugin wrapper

 @param [ui-sortable] {object} Options to pass to $.fn.sortable() merged onto ui.config
 */
angular.module('ui.sortable', [])
  .value('uiSortableConfig',{})
  .directive('uiSortable', [
    'uiSortableConfig', '$timeout', '$log',
    function(uiSortableConfig, $timeout, log) {
      return {
        require: '?ngModel',
        link: function(scope, element, attrs, ngModel) {
          var savedNodes;

          function combineCallbacks(first,second){
            if( second && (typeof second === "function") ){
              return function(e,ui){
                first(e,ui);
                second(e,ui);
              };
            }
            return first;
          }

          var opts = {};

          var callbacks = {
            receive: null,
            remove:null,
            start:null,
            stop:null,
            update:null
          };

          angular.extend(opts, uiSortableConfig);

          if (ngModel) {

            // When we add or remove elements, we need the sortable to 'refresh'
            scope.$watch(attrs.ngModel+'.length', function() {
              // Timeout to let ng-repeat modify the DOM
              $timeout(function() {
                element.sortable( "refresh" );
                // We need to make a copy of the current element's contents so
                // we can restore it after sortable has messed it up
                savedNodes = element.contents();
              });
            });

            callbacks.start = function(e, ui) {
              // Save position of dragged item
              ui.item.sortable = { index: ui.item.index() };
            };

            callbacks.update = function(e, ui) {
              // Fetch saved and current position of dropped element
              var end, start;
              start = ui.item.sortable.index;
              end = ui.item.index();

              // Cancel the sort (let ng-repeat do the sort for us)
              element.sortable('cancel');

              // Put the nodes back exactly the way they started (this is very
              // important because ng-repeat uses comment elements to delineate
              // the start and stop of repeat sections and sortable doesn't
              // respect their order (even if we cancel, the order of the
              // comments are still messed up).
              savedNodes.detach().appendTo(element);

              // Reorder array and apply change to scope
              ngModel.$modelValue.splice(end, 0, ngModel.$modelValue.splice(start, 1)[0]);
              scope.$digest();
            };

            angular.forEach(callbacks, function(value, key){
              opts[key] = combineCallbacks(value, opts[key]);
            });

          } else {
            log.info('ui.sortable: ngModel not provided!', element);
          }

          // Create sortable
          element.sortable(opts);
        }
      };
    }
  ]);

Note: this isn't a pull request because I removed some existing ui-sortable functionality as I was making it work (like the ability to provide options to jQuery.sortable through the ui-sortable html attribute), and it is almost a complete rewrite of the plug-in. I do think it is a smoother implementation than the original and should fix a couple of the other bugs that are outstanding, so if everyone thinks it is the right direction to go, I will implement the missing functionality and turn it into a pull request.

What does everyone think?

[CyborgMaster]

Just in case anyone else is looking at this code, I found a small bug that needed fixing. I didn't know this, but $apply and $digest are quite different. $apply called $digest on the root scope. So in order to make sure our model changes are propagated to every scope that needs them, I needed to change the following lines:

              // Reorder array and apply change to scope
              ngModel.$modelValue.splice(end, 0, ngModel.$modelValue.splice(start, 1)[0]);
              scope.$digest();

to

              // Reorder array and apply change to scope
              scope.$apply(function() {
                ngModel.$modelValue.splice(end, 0, ngModel.$modelValue.splice(start, 1)[0]);
              });

[smithkl42]

Out of curiosity - I'm an angular newbie, and still wrapping my head around this stuff - would this issue possibly result in the order of the items in the model getting out of sync with the order of the items in the UI? I'm rewriting the step.StepNumber in my model in the stop(e, ui) callback, like so:

var scope = ui.item.scope();
var steps = scope.$parent.campaignSteps;
for (var i = 0; i < steps.length; i++) {
    steps[i].StepNumber = i + 1;
}

In theory, then, the steps in my UI should always have ascending numbers, but I keep getting results that look like this:

[CyborgMaster]

Quite possibly. I can see two possible reasons. One might be that the locations in the DOM is out of order with the model. The other is that you might need to wrap your model changes in a .$apply. It could be that you are updating the model but the changes aren't being rendered in the UI.

[smithkl42]

I don't think it's the second, as (due to some troubleshooting) I'm wrapping the whole call in $timeout(), which calls $apply() under the covers.

I don't know enough about angular at this point to comment intelligently on your proposed solution, but I'm happy to test out your code when you get it into shape.

[CyborgMaster]

It should be in shape now. Im using it in my app.

• Jeremy

On Fri, Nov 8, 2013 at 10:42 AM, Ken Smith [email protected] wrote:

I don't think it's the second, as (due to some troubleshooting) I'm
wrapping the whole call in $timeout(), which calls $apply() under the
covers.

I don't know enough about angular at this point to comment intelligently
on your proposed solution, but I'm happy to test out your code when you get
it into shape.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/48#issuecomment-28082159
.

[smithkl42]

Just trying it out now . I added back in the bit that watched for callbacks, and once I did that, it seems to have fixed my problem, and otherwise (so far) seems to be working just fine for me.

For other folks looking here, this is the version that I'm using (with the various fixes referenced above included):

/*
 jQuery UI Sortable plugin wrapper

 @param [ui-sortable] {object} Options to pass to $.fn.sortable() merged onto ui.config
 */
angular.module('ui.sortable', [])
  .value('uiSortableConfig', {})
  .directive('uiSortable', [
    'uiSortableConfig', '$timeout', '$log',
    function (uiSortableConfig, $timeout, log) {
        return {
            require: '?ngModel',
            link: function (scope, element, attrs, ngModel) {
                var savedNodes;

                function combineCallbacks(first, second) {
                    if (second && (typeof second === "function")) {
                        return function (e, ui) {
                            first(e, ui);
                            second(e, ui);
                        };
                    }
                    return first;
                }

                var opts = {};

                var callbacks = {
                    receive: null,
                    remove: null,
                    start: null,
                    stop: null,
                    update: null
                };

                angular.extend(opts, uiSortableConfig);

                if (ngModel) {

                    // When we add or remove elements, we need the sortable to 'refresh'
                    scope.$watch(attrs.ngModel + '.length', function () {
                        // Timeout to let ng-repeat modify the DOM
                        $timeout(function () {
                            element.sortable("refresh");
                            // We need to make a copy of the current element's contents so
                            // we can restore it after sortable has messed it up
                            savedNodes = element.contents();
                        });
                    });

                    callbacks.start = function (e, ui) {
                        // Save position of dragged item
                        ui.item.sortable = { index: ui.item.index() };
                    };

                    callbacks.update = function (e, ui) {
                        // Fetch saved and current position of dropped element
                        var end, start;
                        start = ui.item.sortable.index;
                        end = ui.item.index()

                        // Cancel the sort (let ng-repeat do the sort for us)
                        element.sortable('cancel');

                        // Put the nodes back exactly the way they started (this is very
                        // important because ng-repeat uses comment elements to delineate
                        // the start and stop of repeat sections and sortable doesn't
                        // respect their order (even if we cancel, the order of the
                        // comments are still messed up).
                        savedNodes.detach().appendTo(element);

                        // Reorder array and apply change to scope
                        scope.$apply(function () {
                            ngModel.$modelValue.splice(end, 0, ngModel.$modelValue.splice(start, 1)[0]);
                        });
                    };

                    scope.$watch(attrs.uiSortable, function (newVal, oldVal) {
                        angular.forEach(newVal, function (value, key) {

                            if (callbacks[key]) {
                                // wrap the callback
                                value = combineCallbacks(callbacks[key], value);

                                if (key === 'stop') {
                                    // call apply after stop
                                    value = combineCallbacks(value, apply);
                                }
                            }

                            element.sortable('option', key, value);
                        });
                    }, true);

                    angular.forEach(callbacks, function (value, key) {
                        opts[key] = combineCallbacks(value, opts[key]);
                    });

                } else {
                    log.info('ui.sortable: ngModel not provided!', element);
                }

                // Create sortable
                element.sortable(opts);
            }
        };
    }
  ]);

Many thanks, Jeremy.

[CyborgMaster]

Any time. If this continues to work for people, we might want to adopt it
as the main implementation, replacing what is in trunk.

• Jeremy

On Fri, Nov 8, 2013 at 12:18 PM, Ken Smith [email protected] wrote:

Just trying it out now . I added back in the bit that watched for
callbacks, and once I did that, it seems to have fixed my problem, and
otherwise (so far) seems to be working just fine for me.

For other folks looking here, this is the version that I'm using (with the
various fixes referenced above included):

/*
jQuery UI Sortable plugin wrapper

@param [ui-sortable] {object} Options to pass to $.fn.sortable() merged onto ui.config
*/
angular.module('ui.sortable', [])
.value('uiSortableConfig', {})
.directive('uiSortable', [
'uiSortableConfig', '$timeout', '$log',
function (uiSortableConfig, $timeout, log) {
return {
require: '?ngModel',
link: function (scope, element, attrs, ngModel) {
var savedNodes;

            function combineCallbacks(first, second) {
                if (second && (typeof second === "function")) {
                    return function (e, ui) {
                        first(e, ui);
                        second(e, ui);
                    };
                }
                return first;
            }

            var opts = {};

            var callbacks = {
                receive: null,
                remove: null,
                start: null,
                stop: null,
                update: null
            };

            angular.extend(opts, uiSortableConfig);

            if (ngModel) {

                // When we add or remove elements, we need the sortable to 'refresh'
                scope.$watch(attrs.ngModel + '.length', function () {
                    // Timeout to let ng-repeat modify the DOM
                    $timeout(function () {
                        element.sortable("refresh");
                        // We need to make a copy of the current element's contents so
                        // we can restore it after sortable has messed it up
                        savedNodes = element.contents();
                    });
                });

                callbacks.start = function (e, ui) {
                    // Save position of dragged item
                    ui.item.sortable = { index: ui.item.index() };
                };

                callbacks.update = function (e, ui) {
                    // Fetch saved and current position of dropped element
                    var end, start;
                    start = ui.item.sortable.index;
                    end = ui.item.index()

                    // Cancel the sort (let ng-repeat do the sort for us)
                    element.sortable('cancel');

                    // Put the nodes back exactly the way they started (this is very
                    // important because ng-repeat uses comment elements to delineate
                    // the start and stop of repeat sections and sortable doesn't
                    // respect their order (even if we cancel, the order of the
                    // comments are still messed up).
                    savedNodes.detach().appendTo(element);

                    // Reorder array and apply change to scope
                    scope.$apply(function () {
                        ngModel.$modelValue.splice(end, 0, ngModel.$modelValue.splice(start, 1)[0]);
                    });
                };

                scope.$watch(attrs.uiSortable, function (newVal, oldVal) {
                    angular.forEach(newVal, function (value, key) {

                        if (callbacks[key]) {
                            // wrap the callback
                            value = combineCallbacks(callbacks[key], value);

                            if (key === 'stop') {
                                // call apply after stop
                                value = combineCallbacks(value, apply);
                            }
                        }

                        element.sortable('option', key, value);
                    });
                }, true);

                angular.forEach(callbacks, function (value, key) {
                    opts[key] = combineCallbacks(value, opts[key]);
                });

            } else {
                log.info('ui.sortable: ngModel not provided!', element);
            }

            // Create sortable
            element.sortable(opts);
        }
    };
}

]);

Many thanks, Jeremy.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/48#issuecomment-28089659
.

[CyborgMaster]

So it turns out that angular messes with the DOM way more often than I thought, and ng-repeat keeps internal pointers to the actual DOM objects, as opposed to reinspecting the DOM when it needs to do it things. The result is I needed to snapshot the DOM on start as opposed to model change. The following patch fixed a couple intermittent bugs:

               // Timeout to let ng-repeat modify the DOM
               $timeout(function() {
                 element.sortable( "refresh" );
-                // We need to make a copy of the current element's contents so
-                // we can restore it after sortable has messed it up
-                savedNodes = element.contents();
               });
             });

             callbacks.start = function(e, ui) {
               // Save position of dragged item
               ui.item.sortable = { index: ui.item.index() };
+
+              // We need to make a copy of the current element's contents so
+              // we can restore it after sortable has messed it up
+              savedNodes = element.contents().not(
+                //Don't inlcude the placeholder
+                element.find('.ui-sortable-placeholder'));
             };

[rogerfar]

I just ran in the exact same problem today, it seems like that your patch fixed. When I added new items (when the list was messed up), it would add the in the middle of the list instead at the end.

[filipkis]

I faced the same problem, but had connected lists, thus your solution was not enough for me, so I expended it to work for connected list as well. Here is the result:

/*
 jQuery UI Sortable plugin wrapper

 @param [ui-sortable] {object} Options to pass to $.fn.sortable() merged onto ui.config
 */
angular.module('ui.sortable', [])
  .value('uiSortableConfig', {})
  .directive('uiSortable', [
    'uiSortableConfig', '$timeout', '$log',
    function (uiSortableConfig, $timeout, log) {
        return {
            require: '?ngModel',
            link: function (scope, element, attrs, ngModel) {
                var savedNodes;

                function combineCallbacks(first, second) {
                    if (second && (typeof second === "function")) {
                        return function (e, ui) {
                            first(e, ui);
                            second(e, ui);
                        };
                    }
                    return first;
                }

                var opts = {};

                var callbacks = {
                    receive: null,
                    remove: null,
                    start: null,
                    stop: null,
                    update: null
                };

                angular.extend(opts, uiSortableConfig);

                if (ngModel) {

                    // When we add or remove elements, we need the sortable to 'refresh'
                    scope.$watch(attrs.ngModel + '.length', function () {
                        // Timeout to let ng-repeat modify the DOM
                        $timeout(function () {
                            element.sortable("refresh");
                        });
                    });

                    callbacks.start = function (e, ui) {
                        // Save position of dragged item
                        ui.item.sortable = { index: ui.item.index() };


                         // We need to make a copy of the current element's contents so
                         // we can restore it after sortable has messed it up
                         savedNodes = element.contents().not(
                           //Don't inlcude the placeholder
                           element.find("." + element.sortable('option','placeholder').element().attr('class')));
                    };

                    callbacks.update = function (e, ui) {

                        // Save current drop position but only if this is not a second 
                        // update that happens when moving between lists because then
                        // the value will be overwritten with the old value
                        if(!ui.item.sortable.relocate){
                          ui.item.sortable.dropindex = ui.item.index()
                        }

                        // Cancel the sort (let ng-repeat do the sort for us)
                        element.sortable('cancel');

                        // Put the nodes back exactly the way they started (this is very
                        // important because ng-repeat uses comment elements to delineate
                        // the start and stop of repeat sections and sortable doesn't
                        // respect their order (even if we cancel, the order of the
                        // comments are still messed up).
                        savedNodes.detach().appendTo(element);

                        // If relocate is true then we add the new item to this list
                        // otherwise we delay the list update until stop where we will 
                        // know if it was a resort or item was moved to another list
                        if(ui.item.sortable.relocate) {
                          scope.$apply(function () {
                            ngModel.$modelValue.splice(ui.item.sortable.dropindex, 0, ui.item.sortable.moved);
                          });
                        }

                    };  

                    callbacks.receive = function(e, ui) {
                      ui.item.sortable.relocate = true;
                      // // added item to array into correct position and set up flag
                      // //ngModel.$modelValue.splice(ui.item.index(), 0, ui.item.sortable.moved);
                      savedNodes = element.contents().not(
                           //Don't inlcude the placeholder
                           element.find('.ui-sortable-placeholder'));
                    };

                    callbacks.remove = function(e, ui) {
                      // copy data into item
                      if (ngModel.$modelValue.length === 1) {
                        ui.item.sortable.moved = ngModel.$modelValue.splice(0, 1)[0];
                      } else {
                        ui.item.sortable.moved = ngModel.$modelValue.splice(ui.item.sortable.index, 1)[0];
                      }
                    };

                    callbacks.stop = function(e, ui) {

                      // Resort items if it was not moved
                      if(!ui.item.sortable.relocate){

                        // Reorder array and apply change to scope
                        scope.$apply(function () {
                            ngModel.$modelValue.splice(ui.item.sortable.dropindex, 0, ngModel.$modelValue.splice(ui.item.sortable.index, 1)[0]);
                        });
                      }

                    }


                    scope.$watch(attrs.uiSortable, function (newVal, oldVal) {
                        angular.forEach(newVal, function (value, key) {

                            if (callbacks[key]) {
                                // wrap the callback
                                value = combineCallbacks(callbacks[key], value);

                            }

                            element.sortable('option', key, value);
                        });
                    }, true);

                    angular.forEach(callbacks, function (value, key) {
                        opts[key] = combineCallbacks(value, opts[key]);
                    });

                } else {
                    log.info('ui.sortable: ngModel not provided!', element);
                }

                // Create sortable
                element.sortable(opts);
            }
        };
    }
  ]);

I also made it work with custom placeholder (as with custom placeholders the reloading of all items caused the placeholder to be visible after sort).

[thgreasi]

Please, since the issue tracker is not a code repo, create a fork and refer to commits in your posts.
This way the changes will be track-able and re-posts with updated code will no longer be necessary.
I would like to follow this thread and the code-base progress, but it already has 12 posts in just two days.

Moreover if this is an issue related to angular1.2 only, please update the title to include "(ng1.2)".

[CyborgMaster]

I'm doing that right now. I apologize, the trend of posting code was started by me. I didn't want to make it a pull request because it wasn't a patch as much as it was a rewrite, but I didn't think to create a fork and then refer to commits in my posts. Now that I think about it that is of course the right thing to do. I wasn't sure it was an angular 1.2 thing only, but based on the comments it appears that is true, so I'm going to update the title as well.

Again, my apologies for not following github best practices.

[CyborgMaster]

Ok. I've moved all of my code into a fork (https://github.com/CyborgMaster/ui-sortable). And merged in the changes from everyone in this thread. What do you think about it, is this something we should think about merging into the main repo?

The only disclaimer I would make is that I didn't have a good test bench to verify drag and drop between lists. I'm pretty sure that part works, but if it doesn't feel free to submit patches (@filipkis).

Note to others in this thread: if you want to use my code, use the fork for now, until we figure out how to get this code merged into the main repo.

[CyborgMaster]

As I've had 4 or 5 people contact me saying this fixes their problems, I'm changing this into a pull request. All missing features have been re-implemented (configuration using the directive, updatable callbacks, drag and drop between lists), and have been thoroughly tested. If no one objects, I move this be merged into master.

[smithkl42]

+1 from me. Thanks.

[CyborgMaster]

You're welcome.

[umbrella-mikhail-menshinskiy]

Good work! Thanks

[regexb]

Perfect! Thank you!

[kureus]

+1 from me, fixed my issues with multiple connected lists

[CyborgMaster]

Awesome! @thgreasi says he's working on some tests, after he's done with that we can hopefully merge this into master and let everyone start using the main repo instead of the fork.

[filipkis]

I have found one more bug in the code I posted, that got integrated in this version and also implemented a way to cancel the sorting for myself, so I can contribute these changes. However, I won't be able to do this until earliest tomorrow.

[CyborgMaster]

Sweet. If you could do it on an issue/pull-request in my fork that would be the most helpful.

[m0dd3r]

This approach appears to fail when combined with ng-repeat's orderBy filter. Reordering the $modelValue array doesn't work since the array order isn't what's important. I'm looking into possible solutions or workarounds, but nothing so far.

[CyborgMaster]

That is a very good point. It wouldn't work. I don't think that is in any way unique to my implementation, but rather a limit to the technique used by this library since the beginning. If you come up with any clever ideas let me know and we can work them in. I can't think of anything simple off of the top of my head.

[m0dd3r]

You're right, it doesn't look like it's unique to your implementation. The more I dig into this, the more I realize that my previous approach was working in spite of the technique used in this library, rather than because of it. However, the old implementation results in the DOM being ordered properly (until the comments get messed up and the ng-repeat breaks), whereas with your new implementation, the DOM ends up unchanged. So, with the old implementation I was able to grab the array of DOM elements after being sorted (in an update callback) and use the index of each to reassign my positioning value. Something like this:

uiArray = ui.item...sortable('toArray');
angular.forEach(elements, function(element, index) {
    element.position = uiArray.indexOf(element.id);
}

Not the cleanest solution, but it was working until I ran into the same bugs that started this issue (i.e. the comments getting moved around and things getting out of sync). However, since your new implementation reverts the DOM in the update function, my approach no longer works.
A brief look at the angular source/docs for the orderBy filter confirms that it returns a reordered copy of the array, so that explains my problem. As far as clever ideas go though, I'm drawing a blank at the moment. I'll update if I come up with anything.

[CyborgMaster]

I'll keep thinking about it too. I've had it in the back of my head since your first post and haven't gotten anywhere. You would have to either find a way to build a map back to the original array for ui-sortable to use, or somehow make it possible to go from the dom element back to the original index. Even then how would ui-sortable know what to modify about your elements to make them sort in the right order. Tough problem. Maybe there is a better approach, like allowing the use to edit whatever field you are orderBy'ing by.

[m0dd3r]

I may have an idea here. It occurred to me that my previous solution (with some slight tweaks) still works if I invert the combineCallback function so my user defined update() runs before the one defined in the library. Now I realize that's hacky as hell, and seems just as likely to cause issues as the way it is now. However, I wonder if expanding the callbacks system to provide before hooks would be worthwhile. Callbacks of the same name (update, stop, etc...) could continue to behave as they do now, as after hooks, providing backwards compatibility. Thoughts?

[smithkl42]

I also wanted to let you guys know that with the latest version from the @CyborgMaster repository, and with Angular 1.2.1, I'm still having trouble on occasion with the ng-repeat comments getting messed up, and items getting sorted incorrectly as a result. I haven't done a dive into it yet to see if I can figure out why, but just wanted to let y'all know - before scurrying home for the weekend :-). If I have a chance, I'll see if I can poke around and find out what's going on.

[thgreasi]

@CyborgMaster yesterday I merged some of my tests to the master repo. Please pull them to test your code.I also recommend testing against the commented ones (those work on ng1.0 but on ng1.2).
This still work in progress and just pushed tests that are in good shape. I'm currently also looking to start using protractor at some point for testing.

Also (a late discussion catch up on filtering/ordering):
its highly preferable (performance wise) to wrap the directive with a controller and sort/filter/manipulate your model in there and then provide the manipulated model to uiSortable and ngRepeater. For large lists the performance gains are obvious.

[m0dd3r]

@thgreasi, sorting the list in a controller and forgoing the orderBy filter is the solution I ended up going with. However, I'm only working with a very small list, so the performance gains are negligible. In the end, I think it just feels wrong to provide a sorting library that doesn't function within the structure of the built in angular ordering filters.

[CyborgMaster]

At the request of @thgreasi, I have opened a new pull request for this change onto this repository's angular1.2 branch (see: #56). As a result I am closing this pull request.

[CyborgMaster]

So I just re-read @thgreasi's request, and I realized he asked me NOT to close this pull request, so I am re-opening it. Oops... (The other pull request is still open at #56)

[thgreasi]

I'm closing this issue, as I just opened #61 as a request for collaborator reviews.
For anything new, just open new issues and don't forget to tag the branch you are using.

[clarabstract]

Just a note for people who have found this thread trying to work out interaction issues between ngRepeat and other sortables: Trying to "unconfuse" the DOM after a non-angular sortable widget has messed with it in a way that will make sense to ngRepeat is an extremely frustrating, error-prone approach. You are far better off using ngRepeat's new(ish?) track by keyword to help ngRepeat work out identities when the built-in mechanisms fail (which btw they easily can when the underlying object is "replaced-in-place" via ngResource). I believe using track by would have avoided the above problem entirely as well (though obviously a generic solution that does not rely on it is always better).

[thgreasi]

The current implementation restores the DOM elements to the state before the drag and just changes the model. So ngRepeat never knows that the DOM changed (since this is before any digest). Can you provide a proof of concept with track by?

[clarabstract]

The note was regarding other sortables (including the unmodified jQuery sortable). ui-sortable itself works fine as far as I know. They all run into pretty much the same issue being solved here though, which is how I came upon this issue's thread. Trying to apply the same fix you guys did (restoring the DOM pre-digest) is actually quite difficult (which is somewhat apparent from the revisions needed for this pull request), so my warning to people running into this problem using other sortable components is to not attempt the same approach. Using track by allows ngrepeat to track dom movement it did not initiate pretty reliably and doesn't require detailed knowledge of the dom modifications performed by your sortable.

... I realize this is not the most ideal venue for this sort of thing, but it is probably the most useful given this is the most detailed writeup on the issue (It also keeps coming up near the top of most Google queries related to it).

e: So just to clarify: I am not suggesting you alter anything about ui-sortable's currently implemented work-around.

[justforuse]

thanks a lot, it works in new version