Best way to order items #172

redgeoff · 2014-05-10T17:32:34Z

I want to start off by saying that I really appreciate the work that has been done on this project!

I've searched the issues and #113 and #70 are somewhat relevant, but don't answer my question exactly and the answer is something that I consider to be very important about this project.

I'm trying to contribute to the hoodie project which is basically an open source version of firebase. I'm using the hoodie angular binding.

As far as I know, to support sorting via ui-sortable we need to modify the hoodie angular binding, which I proposed via elmarburke/hoodie-plugin-angularjs#14, however as per the comments, I agree that sorting should really be handled via angular filters.

To make this specific to the context of ui-sortable only, perhaps someone can tell me if the following code presents the only way to currently support this type of sorting in ui-sortable.

<!DOCTYPE html>
<html ng-app="todos">
  <head>
    <meta charset="utf-8">
    <title>Todos</title>
  </head>
  <body>

    <h2>Todos</h2>
    <div ng-controller="TodoCtrl">
      <ul ng-model="todos" ui-sortable>
        <li ng-repeat="todo in todos">
          <span style="cursor:move;">{{todo.txt}}</span>
        </li>
      </ul>
    </div>

    <script src="http://ajax.googleapis.com/ajax/libs/jquery/2.0.3/jquery.min.js"></script>
    <script src="http://ajax.googleapis.com/ajax/libs/jqueryui/1.10.3/jquery-ui.min.js"></script>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.4/angular.min.js"></script>
    <script src="https://rawgithub.com/angular-ui/ui-sortable/master/src/sortable.js"></script>

    <script>

      angular.module('todos', ['ui.sortable'])

      .controller('TodoCtrl', function($scope, orderByFilter) {

        $scope.todos = [{txt: 'two', i: 1}, {txt: 'one', i: 0}, {txt: 'three', i: 2}];
        $scope.todos = orderByFilter($scope.todos, ['i']);

        $scope.$watchCollection('todos', function () {
          for (var index in $scope.todos) {
            $scope.todos[index].i = index;
          }
        });

      });

    </script>
  </body>
</html>

If I am correct and this is the only way, then is it on the roadmap for ui-sortable to support angular filters? If not, should I consider adding it ui-sortable?

redgeoff · 2014-05-10T20:43:26Z

Just to clarify, here is how I would hope to write this code:

<!DOCTYPE html>
<html ng-app="todos">
  <head>
    <meta charset="utf-8">
    <title>Todos</title>
  </head>
  <body>

    <h2>Todos</h2>
    <div ng-controller="TodoCtrl">
      <ul ng-model="todos" ui-sortable="sortableOptions">
        <li ng-repeat="todo in todos | orderBy:'i'">
          <span style="cursor:move;">{{todo.txt}}</span>
        </li>
      </ul>
    </div>

    <script src="http://ajax.googleapis.com/ajax/libs/jquery/2.0.3/jquery.min.js"></script>
    <script src="http://ajax.googleapis.com/ajax/libs/jqueryui/1.10.3/jquery-ui.min.js"></script>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.4/angular.min.js"></script>
    <script src="https://rawgithub.com/angular-ui/ui-sortable/master/src/sortable.js"></script>

    <script>

      angular.module('todos', ['ui.sortable'])

      .controller('TodoCtrl', function($scope) {

        $scope.todos = [{txt: 'two', i: 1}, {txt: 'one', i: 0}, {txt: 'three', i: 2}]
          // uncomment the following code for example to work
          // .sort(function (a, b) {
          //   return a.i > b.i;
          // });

        $scope.sortableOptions = {
          stop: function(e, ui) {
            for (var index in $scope.todos) {
              $scope.todos[index].i = index;
            }
          },
          axis: 'y'
        };

      });

    </script>
  </body>
</html>

The problem is that on the first drag, the ordering goes back to the original order. Subsequent ordering appears to work properly.

If we then uncomment the following lines:

          // .sort(function (a, b) {
          //   return a.i > b.i;
          // });

everything appears to work properly, but when we then work with angular bindings that initially load this scope data behind the scenes, we probably won't have access to perform this initial sort. Is there a better way to handle this in ui-sortable?

thgreasi · 2014-05-11T10:27:50Z

First of all, lets assume that we can build (more on this later) a directive that changes it's behavior based on a view-level filter (orderBy in our case). So, moving the first item to the end of the list would have a totally different result (aka affect ng-model in different way) if someone removed or changed the ordering filter in the view. Do you think that this follows the MV* principles?

Moreover, take a look at tips 2&3 of section Tuning guidelines in this article. | orderBy:'i' is just syntactic sugar, but under the hook you can bet that it's a method call. Performance tips similar to the above (applying filtering and ordering in the controller), are advertised by the angularJS core team.
In my opinion, view-level filters should only be used for presentation and not change the behavior of an app/directive/controller. In this case, we should not base functionality on them.

I have created two pens, based on the code from your posts. Lets call it penA and penB.
The solution I tend to use in my projects is the one from penA, which is very close to the code demonstrated in README's examples section with the title ordering.

You can see that penB is somewhat broken. Moving element one under element three, actually moves items two. This would be more obvious with a bigger list.
One of the main requirements stated in the README is that:

The items of ng-model must match the indexes of the generated DOM elements.

As a result the scope variable provided to ng-model and the repeater should be the same (and not have any view-level filter applied to it).

Let's create penB1, to be the result of adding .sort to penB. Now everything works as you stated.
We can even remove | orderBy:'i' from the view and the example will continue to work, let's call it penB2. As you can see penB1 is doing the same job (sorting) two times, once in the controller and once more in the view.

Comparing the two working approaches, penA to penB2 (and it's intermediate steps) we can deduce that they are somewhat equivalent (in logic and behavior):

using orderByFilter in penA has the same effect with .sort in penB. orderByFilter provides just a different syntax + it returns a new object instead of changing the original.
$watchCollection in penA has the same effect with the provided stop callback in penB. Obviously stop might be more efficient, since it runs only after a sorting, in comparison to the watch expression that might get evaluated for changes more often.

Possible implementation of inferring the ordering from the view will be discussed later today... Have to go now, sorry :)

thgreasi · 2014-05-11T14:06:05Z

Not yet implemented but possible ways to work this out.

Solution 1
We add an extra property to ui-sortable's configuration options object, that would specify the ordering property/method. eg:

$scope.sortableOptions = {
  uiOrderBy: 'i',
  axis: 'y'
};

Pros:

Single place to specify the ordering.
No need to use $watchCollection or a stop callback to apply the changes to the ordering property of the model (in the case a sorting property is provided and not a sorting method).

Cons:

In this case, specifying an ordering filter in the view would be redundant and just consume cpu cycles.
people who love angular's view-level filters will still not be happy. (Even though orderByFilter is an angular filter...)

Solution 2
We change the ui-sortable implementation to extract the applied ordering filter from the HTML code of the first generated item of the repeater (if there is one) and then apply it to the provided ngModel.
Pros:

People stop complaining for "bug in the angular-ui sortable code" since the view-level filters seem to work.

Cons:

ngModel will be modified even if no filtering is issued. Thats because "the scope variable provided to ng-model and the repeater should be the same" and as a result will will not be able to use a directive wide temporary variable.
You might still need to use $watchCollection or the stop callback to update the ordering property of your model.
Even though the repeater orderBy filter is used to specify the ordering, the sorting on the view will still occur and waste cpu cycles as the model is pre-sorted by the directive.

thgreasi · 2014-05-11T16:44:49Z

Here is a pen using a proof-of-concept fork of ui-sortable I created to demonstrate the above Solution 2 concept.

Summarizing:

we don't need to specify anything to sortableOptions
the sorting (property or function) is inferred from the HTML of the first element generated by the repeater
if orderBy used a property, then after a sorting the properties of ngModel get updated automatically (zero-based).
orderBy runs once inside the directive (and sorts the provided ngModel) and once in the view (wasting cpu cycles, as the model is already sorted)

This proof-of-concept works with at most one orderBy filter in the repeater.
Also, it looks like we are emulating ng-repeat's filter parsing mechanism.

redgeoff · 2014-05-11T21:10:27Z

This is great! Just to clarify, @thgreasi does this mean that you'll be merging the code from proof-of-concept fork into the master?

thgreasi · 2014-05-11T22:08:38Z

This was just a draft and a proof that its possible. The code currently has some duplications and its performance is by definition lower than the master branch. Moreover, people will start asking to be able to combine ANY number of filters and ANY kind of them (not just orderBy and possibly even custom ones).
As a result, I can't merge it right now and especially:

not by my own and
not enabled by default (an extra sortableOption should remove the performance hit for those willing to use prepare their model in the controller).

References: #70, #113, #122.

redgeoff · 2014-05-11T22:12:38Z

Will you be adding this to you list of things to do? If not, how would you like to handle this? It would be a shame to see this feature die here. You are far more knowledgable about this technology and how to use it properly. I could try to finish the implementation, but I have a feeling that you could devise a much better implementation and in far less time than me.

thgreasi added the question label May 11, 2014

redgeoff mentioned this issue May 11, 2014

added support for sorting array elmarburke/hoodie-plugin-angularjs#14

Closed

thgreasi closed this as completed May 11, 2014

thgreasi added enhancement labels May 11, 2014

thgreasi mentioned this issue Jun 17, 2014

Updating alternative index value #204

Closed

thgreasi mentioned this issue Jul 12, 2016

orderBy problem #472

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Best way to order items #172

Best way to order items #172

redgeoff commented May 10, 2014

redgeoff commented May 10, 2014

thgreasi commented May 11, 2014

thgreasi commented May 11, 2014

thgreasi commented May 11, 2014

redgeoff commented May 11, 2014

thgreasi commented May 11, 2014

redgeoff commented May 11, 2014

Best way to order items #172

Best way to order items #172

Comments

redgeoff commented May 10, 2014

redgeoff commented May 10, 2014

thgreasi commented May 11, 2014

thgreasi commented May 11, 2014

thgreasi commented May 11, 2014

redgeoff commented May 11, 2014

thgreasi commented May 11, 2014

redgeoff commented May 11, 2014