Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pager requests AjaxUrl before initial filters are applied #388

Closed
sbine opened this issue Oct 14, 2013 · 16 comments
Closed

Pager requests AjaxUrl before initial filters are applied #388

sbine opened this issue Oct 14, 2013 · 16 comments

Comments

@sbine
Copy link

sbine commented Oct 14, 2013

Hello again Mottie! Hopefully seeing my name in an issue doesn't immediately fill you with dread now... :) Unfortunately I do have another AJAX-related issue to report.

Using all server-side options and your filter saving example, the pager addon makes two requests to the AjaxUrl when it initializes - one before filters have applied, and one after.

?page=0&size=25&filter&sort[0]=1
?page=0&size=25&filter[0]=1373781600&filter[1]=800&sort[0]=1

Each AJAX request performs a rather expensive database query in my application, so it's important for me to find a solution to this issue.

Digging around in the source leads me to believe that if the pager addon had a 'priority' value similar to the filter widget, it could initialize after filters have been applied.
Would it be possible for the pager to be rewritten as a proper widget? If not, how can I delay the pager's initial AJAX request until filters have been applied?

Once again, thank you very much for your assistance. I realize I'm using your plugin in an unusual manner (it's basically a view for my database), and I appreciate your extensive support.

EDIT: I've been thinking about this some more, and doesn't it make more sense for the AJAX options to be part of the core tablesorter code anyway? It's not really a pager-specific feature, and it would probably solve this and other problems.

@Mottie
Copy link
Owner

Mottie commented Oct 14, 2013

Hi @sbine!

Actually, I've been working on tablesorter v3 and basically what I did was make the pager into a widget. I'm still working on the part where the ajax is a separate module which can be included - it's how the build table widget came into being for this current version.

I apologize because I haven't done extensive ajax testing with the table because quite frankly I have no clue how to set up a database or write any kind of server side coding LOL. Sad huh? Anyway, I do appreciate your feedback :)

So, for now, maybe you can use the filterInit event to initialize the pager? Something like this:

var $table = $('table');
$table.on('filterInit', function(){
    $table.tablesorterPager(pagerOptions);
});

$table.tablesorter({
    widgets : ['filter']
});

I know it feels a bit hacky, but if you need it sooner, I can see what I can do about making my v3 pager widget work with v2.

@sbine
Copy link
Author

sbine commented Oct 14, 2013

Awesome, I am definitely looking forward to v3!

I tried your suggestion and the table stops loading before it gets to filterInit -- I think this might be related to the following in appendToTable:

if (isEmptyObject(c2)) { return; }
...
if (!init) { ts.applyWidget(table); }

since my AJAX tables are always empty until the pager addon requests data. However, it could also be related to the changes I've made to the plugin so I'll try it again on a fresh copy when I get a chance.

In the meantime, I don't want to cause extra/unnecessary work for you, so unless backporting the v3 pager widget is really easy I'll work on finding another solution (I'm primarily a backend developer so I feel your pain in reverse!)

On that note, I will also try to set up some sort of mockjax demo for you so we can communicate about this more easily in the future. Thanks again!

@stanislavprokopov
Copy link

I have encountered the same problem and managed to find a dirty fix.

Step 1: Set the filter values using currentFilters so that ajax could use them on initial request:

.tablesorterPager({
  ajaxUrl : '/someurl?page={page}&size={size}&{filterList:filter}&{sortList:column}',
  currentFilters: {
    3: "default filter value"
  }
})

Step 2: Provide headers with sorters

$(table).tablesorter({
    sortList   : [[3,1]],
    headers: { 0: {sorter: "text" }, 1: {sorter: "text" }, 2: {sorter: "text" }, 3: {sorter: "shortDate" } }
});

Step 3: In some cases, the initial request could return no rows, so this will break the table and it will not display the filter`s. To fix this, we need to change the way the parser cache is build. Now it depends on the rows, but we will change this and allow it to create the cache based on the headers, this is why we need headers to be set in step 2.
In "jquery.tablesorter.js":

function buildParserCache(table) {
    ...
    //if (rows[0]) {
    if (c.$headers) {
        list = [];
        //l = rows[0].cells.length;
        l = c.$headers.length;

@Mottie Mottie closed this as completed in c4f10de Oct 18, 2013
@Mottie
Copy link
Owner

Mottie commented Oct 18, 2013

Hopefully the latest changes have taken care of this issue.

@sbine... I added a new pager widget (ajax demo) but I left it in "beta"... it's essentially the same as the plugin (which I also updated) along with the filter widget to allow it to initialize on an empty table (thanks @stanislavprokopov for the idea!).

crosses fingers

@sbine
Copy link
Author

sbine commented Nov 1, 2013

Update:
Using the pager addon, sorting is broken on AJAX tables (it will only sort in one direction even if you toggle the column headers repeatedly).
Using the new pager widget, a request is sent before filters are applied, and once again with filters (i.e. the original problem reported). This appears to be due to the moveToPage call (which requests AJAX data) in the init function:

if ( typeof(wo.pager_ajaxUrl) === 'string' ) {
  ...
  tsp.moveToPage(table, p);

I was able to alleviate this somewhat by setting a pager_currentFilters option and changing the following in widgets-pager.js:

currentFilters: [],

to

currentFilters: wo.pager_currentFilters,

which sets the filters correctly on the first request. However, this causes saveSort to break (my theory is this has something to do with the filter widget's check for duplicate filter requests).

Ultimately this issue boils down to different configuration options being set/used in different widgets at different times. Frequent table updates don't matter as much for client-side tables, but significantly affect server-side behavior. Ideally all options would be set before the first AJAX request is made; I'm sure your buildTable widget will solve it once and for all!

@Mottie
Copy link
Owner

Mottie commented Nov 2, 2013

Hi @sbine!

Did you try changing the pager widget priority value? It's set at 5 right now, but you should be able to set it to something like 55 so it initializes after the filter widget... or did you solve this by the default currentFilters setting you shared?

I was just thinking, what is we forced the pager to not make any ajax calls until after the table-initialized event occurs? It would allow all of the widgets to finish initializing (on an empty table), maybe something like this change within the moveToPage function:

if (p.ajax ) {
    if ( table.hasInitialized ) {
        tsp.getAjax(table, c);
    } else {
        c.$table
            .unbind('tablesorter-initialized.pager')
            .bind('tablesorter-initialized.pager', function(){
                tsp.moveToPage(table, p, flag);
            });
    }
} else if (!p.ajax) {
    tsp.renderTable(table, c.rowsCopy);
}

@Mottie Mottie reopened this Nov 2, 2013
@sbine
Copy link
Author

sbine commented Nov 4, 2013

Thanks @Mottie! Setting priority to 55 fixes the saveSort issue, but breaks saved filters. For reference, here is my filter saving code:

[...].bind('filterInit', function () {
    if ($.tablesorter.storage) {
        // get saved filters
        var f = $.tablesorter.storage( this, 'tablesorter-filters' ) || [];
        $.tablesorter.setFilters( this, f, false );
    }
})

Saved filters break because the pager's currentFilters value does not read from the table's $filters, it's only updated on filterStart which does not appear to trigger in these conditions.

I also tried copying over the filter widget's getFilters method into the pager plugin and using it in getAjaxUrl, but since filterInit doesn't trigger until after the pager is initialized (even with 55 priority), the filters still aren't applied on the first AJAX request. Any suggestions?

It's also worth noting that I gave up on using the filter formatter widget (my use for it wasn't completely necessary) and am now just trying to get AJAX requests to play nicely with saved filters/sorts. Hopefully we can figure this out!

@Mottie
Copy link
Owner

Mottie commented Nov 9, 2013

Hi @sbine!

I think this next update will resolve all of these issues. I've checked to make sure that there is only one ajax call while initializing & changing pages, default filter settings (in the data-value attribute) are added to the table and I added some pager debugging messages to make it easier to troubleshoot.

Mottie added a commit that referenced this issue Nov 9, 2013
@Mottie
Copy link
Owner

Mottie commented Nov 9, 2013

Hey @sbine!

I've updated the pager widget, and I did end up setting the priority to 55. Everything appears to work smoothly now. So all I can do is await your acquiescence with bated breath ;)

@sbine
Copy link
Author

sbine commented Nov 11, 2013

@Mottie I'm testing the new version now and it is so much better! Even minor bugs I hadn't reported before are fixed (like my "reset" button triggering 2 AJAX calls due to $('#queue').trigger("saveSortReset"); and $.tablesorter.setFilters( $("#queue"), [ filters ], true );). I haven't experienced any duplicate requests, and all filters/sorts/paging appears to function perfectly.

Quick question - the code I was using before to save filters does not seem to work anymore, has this changed with the update?

[...].bind('filterInit', function () {
        if ($.tablesorter.storage) {
            // get saved filters
            var f = $.tablesorter.storage( this, 'tablesorter-filters' ) || [];
            $.tablesorter.setFilters( this, f, false );
        }
    }).bind('filterEnd', function () {
        if ($.tablesorter.storage) {
            // save current filters
            var f = $.tablesorter.getFilters( this );
            $.tablesorter.storage( this, 'tablesorter-filters', f );
        }
        highlightFilteredInputs();
    })

Thank you again for your support and committment to resolving this, I know it wasn't the easiest bug report to deal with. :)

@Mottie
Copy link
Owner

Mottie commented Nov 11, 2013

Hi @sbine!

Whew! I'm glad "almost" everything is working!

The only big difference is in the setFilters function is that now it triggers a namespaced change event (ref):

valid = c && c.$filters ? c.$filters.each(function(indx, el) {
    $(el).find('.tablesorter-filter').val(filter[indx] || '');
}).trigger('change.tsfilter') || false : false;

Other than that, I tested the above code and it appears to be working properly... except that with the latest updates, you shouldn't need any of that filterInit code because the pager plugin/widget already does that for you.

@Mottie
Copy link
Owner

Mottie commented Nov 11, 2013

Hmm, wait... what I meant to say is that the pager plugin/widget grabs the default filters from the header cells (data-value attribute; set using the filter_defaultAttrib widget option)... I guess I could modify it so it grabs stored filter data first... I'll need to add an option to save filters to storage then :P

Mottie added a commit that referenced this issue Nov 20, 2013
@sbine
Copy link
Author

sbine commented Nov 20, 2013

My apologies, I didn't get an email notification about that commit. Everything works perfectly now, thank you for all your help! I think we can finally put this one to rest. :)

@sbine sbine closed this as completed Nov 20, 2013
@Mottie
Copy link
Owner

Mottie commented Nov 20, 2013

Whew that's a relief! :)

@danozard
Copy link

Hi Mottie,

I thought i'd alert you to an IE 8 error with the fix #388 particularly on line 588 of tablesorter,pager.js . The c.widgets.indexOf function seems to be causing the error. e.g.

image

This simply can be reproduced by running the doc page tablesorter-master\docs\example-pager.html on IE 8. I was able to remove the error by either commenting out line 588 to 593 or just by removing "&& c.widgets.indexOf('filter') >= 0" from line 588.

Also just to confirm, reverting back to an earlier form of tablesorter.pager.js last updated on 5/27/2013 eliminates the issue.

Regards,
Owen

@Mottie
Copy link
Owner

Mottie commented Nov 22, 2013

Hi @danozard!

Thanks!... that error is because IE8 doesn't support indexOf on arrays... I'll have that fixed ASAP!

For now, just change line 588 to:

if (ts.filter && $.inArray('filter', c.widgets) >= 0) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants