nytimes/pourover

Sort `attr` property has no effect.

Closed this issue · 2 comments

From the Advanced View example, setting attr in a new sort will specify "what it sorts with respect to." But, as noted as an aside in #27, it doesn't seem to do anything.

Here's a jsFiddle setting attr to uqbar and the sort still working: http://jsfiddle.net/mhkeller/vLur82dq/

The relevant code:

var TitleSort = PourOver.Sort.extend({
    attr: 'uqbar',
    fn: function(a,b){
        if (b.title < a.title){
          return -1;
        } else if (b.title > a.title){
          return 1;
        } else {
          return 0;
        }
    }
});

Similarly, it seems that the way to set the attr by which to sort objects is by setting the name on a and b.

The Advanced View example, however, does not have anything set on a and b.

var RevNameSort = PourOver.Sort.extend({
    attr: "name",
    fn: function(a,b){
        if (b < a){
          return -1;
        } else if (b > a){
          return 1;
        } else {
          return 0;
        }
    }
});

That is correct. Attr is included for possible future uses. I have updated the docs.

Updated the docs! Thanks for the catch.

On Tue, Nov 25, 2014 at 1:02 PM, Michael Keller notifications@github.com
wrote:

The docs refer to this as the proper usage so you might want to add a note
saying you have to refer to the property from within your sort function:
http://nytimes.github.io/pourover/examples/examples_build/advanced_views.html#section-4


Reply to this email directly or view it on GitHub
#42 (comment).