javve / list.pagination.js

A pagination plugin for List.js
24 stars 23 forks source link

Cannot sort in descending order after adding pagination. #1

Closed jieter closed 10 years ago

jieter commented 10 years ago

Just a basic table with List.js, works like charm. Adding list.pagination.js seems to work fine too, but the table cannot be sorted in descending order anymore:

http://bl.ocks.org/jieter/raw/b2e9de3c8d54c2fa6cdd/

no JavaScript errors in the console...

dagjomar commented 10 years ago

Encountered the same problem today.

Reason: The pagination plugin makes a new call to the List constructor using the same ID as the original list.

pagingList = new List(list.listContainer.id, {
                listClass: options.paginationClass || 'pagination',
                item: "<li><a class='page' href='javascript:function Z(){Z=\"\"}Z()'></a></li>",
                valueNames: ['page', 'dotted']
            });

now, the list.js runs a check for elements with 'sort' class, and adds listeners to them.

//This gets run twice, and events are applied twice to the same objects
sortButtons = getByClass(list.listContainer, list.sortClass); 
    events.bind(sortButtons, 'click', sort);

Effectively, the sort buttons will do the sort function TWICE, and look like it only does the same sorting once.

The problem here, is that the getByClass() function searches the same scope both times, when it should search a more narrow scope, I guess. The Pagination-list doesn't have sorting elements.

javve commented 10 years ago

Good finding this bug and the cause of it! I've now updated the script and it should be solved! =) :beers:

dagjomar commented 10 years ago

EDIT: Oh, saw the answer above now. Ignore post.

Trying to implement a different scope when searching for .sort buttons, I got it working...

HOWEVER, now, the internal variable `sortButtons gets overwritten by the second run, so it becomes empty.

So now, the `clearPreviousSortingfunction won't work, as it has no buttons to remove classes from... aaaghh..

Can not understand why the paging "list" needs to be a ListJS object itself! Seems like an inception-kind-of-thing.

javve commented 10 years ago

@dagjomar :grin:

dagjomar commented 10 years ago

Hmm... tried the new source, but still having issues as described above... will post an issue in list.js