Mottie / tablesorter

Github fork of Christian Bach's tablesorter plugin + awesomeness ~
https://mottie.github.io/tablesorter/docs/
2.61k stars 754 forks source link

Tablesorter scroller widget with turbolinks #968

Open fab-girard opened 9 years ago

fab-girard commented 9 years ago

Hello, I've found a problem with tablesorter widget used with turbolinks gem activated on my Ruby on Rails project. After having a look on turbolinks' documentation, it seems to be used to improve js performances by not recompiling some js code between each page change. In fact when turbolinks is active, scroller widget doesn't work at all (the defect occurs for instance when I'm clicking on a link to display a new page containing a tablesorter with scroller). For information, if I refresh the page, turbolinks don't avoid recompiling js and tablesorter/scroller works fine.

I've founded a correction to be able to use scroller widget with turbolinks. In widget-scroller.js I've moved this code :

var style = '<style>' +
            /* measure scroll bar width */
            '.' + tscss.scrollerWrap + 'Measure { width: 100px; height: 100px; overflow: scroll; position: absolute; top: -9999px; }' +
            /* reset width to get accurate measurements after window resize */
            '.' + tscss.scrollerReset + ' { width: auto !important; min-width: auto !important; max-width: auto !important; }' +
            /* overall wrapper & table section wrappers */
            '.' + tscss.scrollerWrap + ' { position: relative; overflow: hidden; }' +
            '.' + tscss.scrollerHeader + ', .' + tscss.scrollerFooter + ' { overflow: hidden; }' +
            '.' + tscss.scrollerHeader + ' table.' + tscss.table + ' { margin-bottom: 0; }' +
            '.' + tscss.scrollerFooter + ' table.' + tscss.table + ' thead { visibility: hidden, height: 0; overflow: hidden; }' +
            /* always leave the scroll bar visible for tbody, or table overflows into the scrollbar when height < max height (filtering) */
            '.' + tscss.scrollerTable + ' { overflow-y: scroll; }' +
            '.' + tscss.scrollerTable + ' table.' + tscss.table + ' { border-top: 0; margin-top: 0; margin-bottom: 0; overflow-y: scroll; }' +
            /* hide filter row in clones */
            '.' + tscss.scrollerTable + ' .' + ( tscss.filterRow || 'tablesorter-filter-row' ) + ',.' + tscss.scrollerFooter + ' .' +
                ( tscss.filterRow || 'tablesorter-filter-row' ) + ',.' + tscss.scrollerTable + ' tfoot { display: none; }' +
            '.' + tscss.scrollerWrap + ' .' + tscss.scrollerFixed + ' { position: absolute; top: 0; z-index: 1; left: 0 } ' +
            '.' + tscss.scrollerWrap + ' .' + tscss.scrollerFixed + '.' + tscss.scrollerRtl + ' { left: auto; right: 0 } ' +
            /* visibly hide header rows in clones, so we can still set a width on it and still effect the rest of the column */
            '.' + tscss.scrollerTable + ' table.' + tscss.table + ' thead tr.' + tscss.headerRow + ' *, .' + tscss.scrollerFooter +
                ' table.' + tscss.table + ' thead * { line-height: 0; height: 0; border: none; background-image: none; padding-top: 0;' +
                ' padding-bottom: 0; margin-top: 0; margin-bottom: 0; overflow: hidden; }' +

            /*** fixed column ***/
            '.' + tscss.scrollerFixed + ' { pointer-events: none; }' +
            /* add horizontal scroll bar */
            '.' + tscss.scrollerWrap + '.' + tscss.scrollerHasFix + ' > .' + tscss.scrollerTable + ' { overflow-x: scroll; }' +
            /* need to position the tbody & tfoot absolutely to hide the scrollbar & move the footer below the horizontal scrollbar */
            '.' + tscss.scrollerFixed + ' .' + tscss.scrollerFooter + ' { position: absolute; bottom: 0; }' +
            /* hide fixed tbody scrollbar - see http://goo.gl/VsLe6n */
            '.' + tscss.scrollerFixed + ' .' + tscss.scrollerTable + ' { position: relative; left: 0; overflow-x: hidden; overflow-y: scroll; -ms-overflow-style: none; }' +
            '.' + tscss.scrollerFixed + ' .' + tscss.scrollerTable + '::-webkit-scrollbar { display: none; }' +
            /* remove right border of fixed header tables to hide the boundary */
            '.' + tscss.scrollerWrap + ' .' + tscss.scrollerFixed + ' table { border-right-color: transparent; padding-right: 0; }' +
            '.' + tscss.scrollerWrap + ' .' + tscss.scrollerFixed + '.' + tscss.scrollerRtl + ' table { border-left-color: transparent; padding-left: 0; }' +
            '</style>';
        $( style ).appendTo( 'body' );

inside the setup function (instead of having it outside in an anonymous function).

But is my modification correct ? I'm not sure... maybe there are some impacts I don't see. And what about rest of code outside setup... like : $.extend( ts.css, { scrollerWrap : 'tablesorter-scroller', scrollerHeader : 'tablesorter-scroller-header', scrollerTable : 'tablesorter-scroller-table', scrollerFooter : 'tablesorter-scroller-footer', scrollerFixed : 'tablesorter-scroller-fixed', scrollerHasFix : 'tablesorter-scroller-has-fixed-columns', scrollerReset : 'tablesorter-scroller-reset', scrollerRtl : 'tablesorter-scroller-rtl' });

Thanks for your suggestion... Fabien

Mottie commented 9 years ago

The reason that code is outside of the setup function is because I only wanted that additional style to be added once. If the code is within the setup, then if there are multiple tables on the page using the scroller widget, multiple copies of that same css stylesheet will be added to the page. Maybe I should just add a separate css file, but then the ability to change class names used by the scroller widget will be lost.

The code to extend the css is also outside of the setup because it only needs to be extended on the $.tablesorter.css object once.

Both blocks of code are scoped within a self-executing anonymous function, and the style sheet is wrapped in an additional document ready function. Does the anonymous function cause issues with turbolinks?

The scroller widget does have to make up to five clones of portions of the table: thead, tbody & tfoot cloned separately, then if there is a fixed column, three more clones are made. This might be a problem with third party scripts that bind to portions of the table, unless they can be set to bind to delegated events.

fab-girard commented 9 years ago

Hello, thk u ! Thanks to your answer, I've found a gem/lib : https://github.com/kossnocorp/jquery.turbolinks which corrects my problem... the js source of this gem/lib is quite simple.

To answer you : yes I think the anonymous functions could cause some troubles with turbolinks but it's not the only use of case ... in my devtools if I add a breakpoint in : var ts = $.tablesorter, tscss = ts.css; and // Add extra scroller css $( function() { var style = '<style>' + ... }); I break only in case of turbolinks is NOT active between pages...

If I add gem jquery-turbolinks, I break in second breakpoint (but not in the first one). I think it's a correct behavior since ts and tscss variables are set from previous page in case of page change with turbolink. Let me now the solution you prefer : avoid having an anonymous function or suggest to use jquery-turbolinks gem ?

So jquery-turbolinks corrects my problem. But unfortunately another problem appears now scroller is working with turbolinks ! :( If I resize browser window the tablesorter disappears... When I have a look in console, a problem occurs in resize function of scroller widget source code. In fact in columns loop, the test if ( $this.css( 'box-sizing' ) === 'border-box' ) is false (css of this/hdr is '' instead of 'border-box' like when turbolinks is not active). I've searched where this css should be set but I'm not sure to understand the entire code of widget...

Regards, Fabien

Mottie commented 9 years ago

Are you using the jquery-tablesorter-rails gem or using the plugin as-is? Honestly, I don't know if that gem changes the wrapping method or fixes the issues you mention.

The border-box definition setting is highly recommended, especially for alignment issues. The scroller widget is the only widget in which this box-sizing is applied (ref) to maintain alignment. You can read more about it in this blog post. The only reason box-sizing would not be set to border-box is in older browsers like IE8. So are you referring to an older browser with alignment issues?

Is there any way you could set up a demo of this issue? My time is limited, so setting up a demo would just take away from my troubleshooting time.