Mottie/tablesorter

Page size lost when table size is changed in background

h3llrais3r opened this issue ยท 18 comments

I'm having a problem with keeping the selected page size when using the tablesorter with the pager plugin. (I'm using version 2.25.3 of your plugin)

image

// Enable tablesorter and tablesorterPager for wanteditems table
$(document).ready(function () {
    $("#wanteditems")
        .tablesorter({
            // Enable widgets
            widgets: ['reflow', 'filter', 'zebra'],
            widgetOptions: {
                // No column filters
                filter_columnFilters: false,
                // External filter selector (will match any column)
                filter_external: '#wanteditemsfilter',
                // Search faster (default 300)
                filter_searchDelay: 50
            },
            // Use date format 'ddmmyyyy'
            dateFormat: 'ddmmyyyy',
            // Force text sorter in first column (this is needed due to img in table cell)
            // See https://github.com/Mottie/tablesorter/issues/1149
            headers: {
                0: {sorter: 'text'}
            },
            // Sort default by time desc
            sortList: [[8, 1]]
        })
        .tablesorterPager({
            container: $("#wanteditemspager"),
            output: '{startRow} to {endRow} ({filteredRows})'
        });
});

On each page load the table is reloaded with data from the server, meaning looping a list and creating a row for each item.
Since the content/size of the list can be changed in the meantime, it's possible that there are more or less items in the list after a page reload.
However, if my page size is set to 'all', and the size has been changed in the meantime, the 'all' option is not longer selected.

image

I've already debugged the code and it's because the 'lastPageSize' is still the old value.
Since the 'all' option is only set when the size equals the totalRows, it's not selected anymore.
Snapshot from your code:

// set to either set or get value
        parsePageSize = function( p, size, mode ) {
            var s = parseInt( size, 10 ) || p.size || p.settings.size || 10,
                // if select does not contain an "all" option, use size
                setAll = p.$size.find( 'option[value="all"]' ).length ? 'all' : p.totalRows;
            return /all/i.test( size ) || s === p.totalRows ?
                // "get" to get `p.size` or "set" to set `p.$size.val()`
                ( mode === 'get' ? p.totalRows : setAll ) :
                ( mode === 'get' ? s : p.size );
        },

Any idea if there is a solution for this? Or is this a bug when the number of rows of the table is changed after a page reload?

Hi @h3llrais3r!

Sorry for the delay in responding.

Does the "all" option include a value set to "all" (<option value="all">All</option>)?

It's difficult for me to The "pagerLastSize" value is saved to jQuery data and does not persist on page reload, so please check the page size in local storage.

var values = $.tablesorter.storage(table, "tablesorter-pager");
// values = { page: #, size: # }

Or, maybe to fix this problem, you could update the page size immediately after ajax completes. In this example, the size is set to the total if the last size is plus or minus 9 of the total number of pages (untested code):

ajaxProcessing: function(result, table, xhr) {
  var c = table.config,
    last = $.tablesorter.storage(table, "tablesorter-pager") || {},
    totalPages = result.total;
  // process data
  // ....
  // set size to total pages if last page size is +/- 9 of total
  if (Math.abs((last.size || 0) - totalPages) < 10) {
    c.pager.size = totalPages;
  }
}

Hmm, or maybe I need to save "all" as the last size instead of an actual number...

Hi @Mottie.

No problem, thanks for replying in the first place.

Does the "all" option include a value set to "all" (All)?

Yes I have this all option.

            <div id="wanteditemspager" class="pager">
                <img src="/images/tablesorter/first.png" class="first"/>
                <img src="/images/tablesorter/prev.png" class="prev"/>
                <span class="pagedisplay"></span>
                <!-- this can be any element, including an input -->
                <img src="/images/tablesorter/next.png" class="next"/>
                <img src="/images/tablesorter/last.png" class="last"/>
                <select class="pagesize" title="Select page size">
                    <option value="10">10</option>
                    <option value="20">20</option>
                    <option value="30">30</option>
                    <option value="40">40</option>
                    <option value="50">50</option>
                    <option value="all">All</option>
                </select>
                <select class="gotoPage" title="Select page number"></select>
            </div>

so please check the page size in local storage.

I've checked this while debugging.
At first page load, the size is 41.
If I select the 'all' option, it shows all.
If I reload the page (submit, so no ajax processing), the size of my list is changed to 40 on the server, so only 40 items are still rendered.
The page size is still 41 in the local storage. Because the actual size doesn't match the size in the storage, it doesn't keep the 'all' option enabled.
However, if I should have selected the '10' option, it stays selected.

So I think your last remark will be the solution. If you select the 'all' option, you should save 'all' and not the actual size.

Thanks for having a look ๐Ÿ˜‰

DOH, I forgot to address this issue in the latest release. I'll try to spend some time on it this weekend.

๐Ÿ˜„ No problem, thanks!

Version 2.26.0 was just released with this change. I made a minor version bump because ajax requests will now pass "all" as a page size, so it may break some current server side methods.

Additionally, I don't have any unit tests set up for the pager (neither the addon nor widget), so this update was not thoroughly tested with ajax methods. Please let me know if you encounter any issues.

Thanks!

Thanks for the quick fix. I'll try to test it asap and I'll keep you posted. ๐Ÿ‘

@Mottie I confirm that the fix is working. ๐Ÿ‘
Thanks again for your great plugin.

I think this causes a pagination issue when using ajax where tablesorter thinks "all" rows have been retrieved when they haven't.

Let's say there are 100 rows in the database table and tablesorter is set to grab 5 rows at a time.

Line 175 in widget-pager.js sets p.totalRows to be equal to the rows retrieved (5):

            p.totalRows = c.$tbodies.eq( 0 )
                .children( 'tr' )
                .not( wo.pager_countChildRows ? '' : '.' + c.cssChildRow )
                .length;

And then line 1118 of the same file sets the p.size to "all" if p.totaRows equals the size:

            return /all/i.test( size ) || s === p.totalRows ?
                // "get" to set `p.size` or "set" to set `p.$size.val()`
                'all' : ( mode === 'get' ? s : p.size );

The result is that with no filters tablesorter always thinks it has returned all rows and so won't let the user advance the page since it thinks there is only one page. Is this a bug or do I need to change something?

Hi @lindonb!

That definition at line 175 only sets the totalRows on initialization. If you're using ajax that number should be returned the ajax data. The snippet from line 1118 only deals with "all" values when setting the page select value. Internally, the totalRows should still be set to the value returned from the ajax data.

If you are encountering an issue, please modify this demo to show the problem.

Hi @Mottie!

Difficult to translate what I've got to the demo but I'm not really seeing any functional differences. I do use ajax data to return the total. What about this: line 1210 of widget-pager.js is setting p.totalPages to 1 because p.size is set to "all":

p.totalPages = p.size === 'all' ? 1 : Math.ceil( tsp.getTotalPages( c, p ) / p.size );

When all rows are showing, there should only be one page. This was meant to have the ajax retrieve all rows (hopefully there aren't thousands) when the "all" setting was used.

When you say "I think this causes a pagination issue" does that mean you encountered a problem? Please provide the steps to reproduce this issue.

Hi @Mottie!

Yes, I do have an issue and was trying to guess the cause. Sorry but would have to provide you access to a site to show the problem, but need a little time to set that up. Not sure it will help you much but below is a portion of the tablesorter code I have (left out the pager_customAjaxUrl part`):

    $('table#adminusers1').tablesorter({
        headerTemplate: '{content} {icon}', 
        showProcessing: true, 
        widgets : ['stickyHeaders','filter','pager','columnSelector'], 
        serverSideSorting: true, 
        sortMultiSortKey : 'none', 
        sortList : [[1,0]], 
        widgetOptions : {
            stickyHeaders : 'ts-stickyHeader', 
            resizable : true, 
            filter_cssFilter : 'form-control', 
            filter_selectSourceSeparator : '|', 
            filter_serversideFiltering : true, 
            filter_reset : 'button#adminusers1-filterreset', 
            pager_size: 5, 
            pager_css: {
                container: 'ts-pager'
            }, 
            pager_selectors: {
                container : 'div#adminusers1-pager'
            }, 
            pager_output: '{startRow} - {endRow} / {filteredRows} ({totalRows})', 
            pager_processAjaxOnInit: false, 
            pager_initialRows: {total: 62, filtered: 62}, 
            pager_ajaxObject: {dataType: 'html'}, 
            pager_ajaxUrl : 'tiki-adminusers.php?{sort:sort}&{filter:filter}', 
            pager_savePages: false, 
            pager_ajaxProcessing: function(data, table){
                var parsedpage = $.parseHTML(data), table = $(parsedpage).find('table#adminusers1'), r = {};
                r.rows = $(table).find('tbody tr');

                r.total = '62';
                r.filteredRows = $(table).data('count');
                r.headers = null;
                return r;
            }, 
(left out the customAjaxUrl part)

Ok, that all looks good. So what are you expecting/doing when the user sets the page size to "all"? You know you don't have to provide that option, right?

I don't have an "all" setting, but the size variable seems to get set to "all" in the code. In the size drop down I just have 5, 10, 15, 20, but none of them are selected when the table is loaded.

Hmm, try clearing out your localStorage. Enter this into the console:

$.tablesorter.storage($('#adminusers1'), 'tablesorter-pager', '')

That didn't seem to work but I went to local storage through the browser and deleted tablesorter-pager, but I still have the same issue. After the page loads and I select a value, like 5, then everything is okay, ie the page dropdown has the right number of pages. But unless I select a size it thinks there's only one page.

There was a problem with the pager widget code being executed twice on initialization, the second time a call would go out to the server to get new data even when the processAjaxOnInit was false. I just patched that in the master branch.

I also found the problem, upon initialization, the parsePageSize function was seeing the number of rows & size setting as the same and setting the size to "all". Once it got set to "all" it didn't revert after the totalRows was change to the pager_initialRows setting. It's all fixed in the master branch now (demo).

I plan to sort out the issues with the select2 filter formatter before the next release. If I can't get it figured out by tomorrow, I'll release the next version anyway.

Did a quick test - looks like that does it! Many thanks for the quick work.