From 7a5f4e46d10ea2079dc3adbfe346836e3212124a Mon Sep 17 00:00:00 2001 From: MatthiasDD Date: Sat, 22 Aug 2015 23:46:27 +0200 Subject: [PATCH] jquery.tablesorter: Add ability for cells with colspan in tbody * Add manageColspans() to handle colspaned cells in table body. Add config.columns - number of colums with extended colspan, inclusive unsortable columns buildParserCache() iterate for all columns (not only the first body row, Bug: T105731) buildCache() also (Bug: T74534) Rows with not enougth cells get after first click additional empty cells. * Clear unused header data 'sortDisabled' . * Add $.tablesorter.getParsers() for better table diagnosis. fix 3 litle bugs: * Improve multi column sorting with colspan in header (add columnToHeader[..] to s[0] ) * Unsortable headers get after sorting no title tag. ($headers contain only sortable headers) * Parser detection in tables with max. 5 rows and empty cells works now. Bug: T105731 Bug: T74534 Change-Id: I518029616d4c10a48eeaad8e92962f4e580f9413 --- resources/src/jquery/jquery.tablesorter.js | 166 +++++++++++++----- .../jquery/jquery.tablesorter.test.js | 129 +++++++++----- 2 files changed, 207 insertions(+), 88 deletions(-) diff --git a/resources/src/jquery/jquery.tablesorter.js b/resources/src/jquery/jquery.tablesorter.js index 23474c0c51..eaa138b937 100644 --- a/resources/src/jquery/jquery.tablesorter.js +++ b/resources/src/jquery/jquery.tablesorter.js @@ -105,18 +105,25 @@ } } - function detectParserForColumn( table, rows, cellIndex ) { + function detectParserForColumn( table, rows, column ) { var l = parsers.length, + cellIndex, nodeValue, // Start with 1 because 0 is the fallback parser i = 1, + lastRowIndex = -1, rowIndex = 0, concurrent = 0, + empty = 0, needed = ( rows.length > 4 ) ? 5 : rows.length; while ( i < l ) { - if ( rows[ rowIndex ] && rows[ rowIndex ].cells[ cellIndex ] ) { - nodeValue = $.trim( getElementSortKey( rows[ rowIndex ].cells[ cellIndex ] ) ); + if ( rows[ rowIndex ] ) { + if ( rowIndex !== lastRowIndex ) { + lastRowIndex = rowIndex; + cellIndex = $( rows[ rowIndex ] ).data( 'columnToCell' )[ column ]; + nodeValue = $.trim( getElementSortKey( rows[ rowIndex ].cells[ cellIndex ] ) ); + } } else { nodeValue = ''; } @@ -134,13 +141,22 @@ i++; rowIndex = 0; concurrent = 0; + empty = 0; } } else { // Empty cell + empty++; rowIndex++; - if ( rowIndex > rows.length ) { - rowIndex = 0; + if ( rowIndex >= rows.length ) { + if ( concurrent >= rows.length - empty ) { + // Confirmed the parser for all filled cells + return parsers[ i ]; + } + // Check next parser, reset rows i++; + rowIndex = 0; + concurrent = 0; + empty = 0; } } } @@ -150,24 +166,22 @@ } function buildParserCache( table, $headers ) { - var sortType, cells, len, i, parser, + var sortType, len, j, parser, rows = table.tBodies[ 0 ].rows, + config = $( table ).data( 'tablesorter' ).config, parsers = []; if ( rows[ 0 ] ) { - - cells = rows[ 0 ].cells; - len = cells.length; - - for ( i = 0; i < len; i++ ) { + len = config.columns; + for ( j = 0; j < len; j++ ) { parser = false; - sortType = $headers.eq( i ).data( 'sortType' ); + sortType = $headers.eq( config.columnToHeader[ j ] ).data( 'sortType' ); if ( sortType !== undefined ) { parser = getParserById( sortType ); } if ( parser === false ) { - parser = detectParserForColumn( table, rows, i ); + parser = detectParserForColumn( table, rows, j ); } parsers.push( parser ); @@ -181,15 +195,16 @@ function buildCache( table ) { var i, j, $row, cols, totalRows = ( table.tBodies[ 0 ] && table.tBodies[ 0 ].rows.length ) || 0, - totalCells = ( table.tBodies[ 0 ].rows[ 0 ] && table.tBodies[ 0 ].rows[ 0 ].cells.length ) || 0, config = $( table ).data( 'tablesorter' ).config, parsers = config.parsers, + len = parsers.length, + cellIndex, cache = { row: [], normalized: [] }; - for ( i = 0; i < totalRows; ++i ) { + for ( i = 0; i < totalRows; i++ ) { // Add the table data to main data array $row = $( table.tBodies[ 0 ].rows[ i ] ); @@ -205,8 +220,9 @@ cache.row.push( $row ); - for ( j = 0; j < totalCells; ++j ) { - cols.push( parsers[ j ].format( getElementSortKey( $row[ 0 ].cells[ j ] ), table, $row[ 0 ].cells[ j ] ) ); + for ( j = 0; j < len; j++ ) { + cellIndex = $row.data( 'columnToCell' )[ j ]; + cols.push( parsers[ j ].format( getElementSortKey( $row[ 0 ].cells[ cellIndex ] ) ) ); } cols.push( cache.normalized.length ); // add position for rowCache @@ -249,7 +265,7 @@ * After this, it will look at all rows at the bottom for footer rows * And place these in a tfoot using similar rules. * - * @param {jQuery} $table jQuery object for a + * @param {jQuery} $table object for a
*/ function emulateTHeadAndFoot( $table ) { var $thead, $tfoot, i, len, @@ -294,12 +310,13 @@ maxSeen = 0, colspanOffset = 0, columns, - i, + k, $cell, rowspan, colspan, headerCount, longestTR, + headerIndex, exploded, $tableHeaders = $( [] ), $tableRows = $( 'thead:eq(0) > tr', table ); @@ -348,29 +365,15 @@ // as each header can span over multiple columns (using colspan=N), // we have to bidirectionally map headers to their columns and columns to their headers - $tableHeaders.each( function ( headerIndex ) { + config.columnToHeader = []; + config.headerToColumns = []; + config.headerList = []; + headerIndex = 0; + $tableHeaders.each( function () { $cell = $( this ); columns = []; - for ( i = 0; i < this.colSpan; i++ ) { - config.columnToHeader[ colspanOffset + i ] = headerIndex; - columns.push( colspanOffset + i ); - } - - config.headerToColumns[ headerIndex ] = columns; - colspanOffset += this.colSpan; - - $cell.data( { - headerIndex: headerIndex, - order: 0, - count: 0 - } ); - - if ( $cell.hasClass( config.unsortableClass ) ) { - $cell.data( 'sortDisabled', true ); - } - - if ( !$cell.data( 'sortDisabled' ) ) { + if ( !$cell.hasClass( config.unsortableClass ) ) { $cell .addClass( config.cssHeader ) .prop( 'tabIndex', 0 ) @@ -378,14 +381,33 @@ role: 'columnheader button', title: msg[ 1 ] } ); + + for ( k = 0; k < this.colSpan; k++ ) { + config.columnToHeader[ colspanOffset + k ] = headerIndex; + columns.push( colspanOffset + k ); + } + + config.headerToColumns[ headerIndex ] = columns; + + $cell.data( { + headerIndex: headerIndex, + order: 0, + count: 0 + } ); + + // add only sortable cells to headerList + config.headerList[ headerIndex ] = this; + headerIndex++; } - // add cell to headerList - config.headerList[ headerIndex ] = this; + colspanOffset += this.colSpan; } ); - return $tableHeaders; + // number of columns with extended colspan, inclusive unsortable + // parsers[j], cache[][j], columnToHeader[j], columnToCell[j] have so many elements + config.columns = colspanOffset; + return $tableHeaders.not( '.' + config.unsortableClass ); } function isValueInArray( v, a ) { @@ -638,6 +660,49 @@ } } + /** + * Build index to handle colspanned cells in the body. + * Set the cell index for each column in an array, + * so that colspaned cells set multiple in this array. + * columnToCell[collumnIndex] point at the real cell in this row. + * + * @param {jQuery} $table object for a
+ */ + function manageColspans( $table ) { + var i, j, k, $row, + $rows = $table.find( '> tbody > tr' ), + totalRows = $rows.length || 0, + config = $table.data( 'tablesorter' ).config, + columns = config.columns, + columnToCell, cellsInRow, index; + + for ( i = 0; i < totalRows; i++ ) { + + $row = $rows.eq( i ); + // if this is a child row, continue to the next row (as buildCache()) + if ( $row.hasClass( config.cssChildRow ) ) { + // go to the next for loop + continue; + } + + columnToCell = []; + cellsInRow = ( $row[ 0 ].cells.length ) || 0; // all cells in this row + index = 0; // real cell index in this row + for ( j = 0; j < columns; index++ ) { + if ( index === cellsInRow ) { + // Row with cells less than columns: add empty cell + $row.append( '
' ); + cellsInRow++; + } + for ( k = 0; k < $row[ 0 ].cells[ index ].colSpan; k++ ) { + columnToCell[ j++ ] = index; + } + } + // Store it in $row + $row.data( 'columnToCell', columnToCell ); + } + } + function buildCollationTable() { ts.collationTable = mw.config.get( 'tableSorterCollation' ); ts.collationRegex = null; @@ -715,12 +780,13 @@ cssChildRow: 'expand-child', sortMultiSortKey: 'shiftKey', unsortableClass: 'unsortable', - parsers: {}, + parsers: [], cancelSelection: true, sortList: [], headerList: [], headerToColumns: [], - columnToHeader: [] + columnToHeader: [], + columns: 0 }, dateRegex: [], @@ -798,6 +864,7 @@ } explodeRowspans( $table ); + manageColspans( $table ); // Try to auto detect column type, and store in tables config config.parsers = buildParserCache( table, $headers ); @@ -805,7 +872,7 @@ // Apply event handling to headers // this is too big, perhaps break it out? - $headers.not( '.' + config.unsortableClass ).on( 'keypress click', function ( e ) { + $headers.on( 'keypress click', function ( e ) { var cell, $cell, columns, newSortList, i, totalRows, j, s, o; @@ -834,7 +901,7 @@ cache = buildCache( table ); totalRows = ( $table[ 0 ].tBodies[ 0 ] && $table[ 0 ].tBodies[ 0 ].rows.length ) || 0; - if ( !table.sortDisabled && totalRows > 0 ) { + if ( totalRows > 0 ) { cell = this; $cell = $( cell ); @@ -867,7 +934,7 @@ // Reverse the sorting direction for all tables. for ( j = 0; j < config.sortList.length; j++ ) { s = config.sortList[ j ]; - o = config.headerList[ s[ 0 ] ]; + o = config.headerList[ config.columnToHeader[ s[ 0 ] ] ]; if ( isValueInArray( s[ 0 ], newSortList ) ) { $( o ).data( 'count', s[ 1 ] + 1 ); s[ 1 ] = $( o ).data( 'count' ) % 2; @@ -938,7 +1005,6 @@ // sort initially if ( config.sortList.length > 0 ) { - setupForFirstSort(); config.sortList = convertSortList( config.sortList ); $table.data( 'tablesorter' ).sort(); } @@ -999,6 +1065,10 @@ buildCollationTable(); return getParserById( id ); + }, + + getParsers: function () { // for table diagnosis + return parsers; } }; diff --git a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js index 759322a748..d4d0ce1e76 100644 --- a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js +++ b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js @@ -1,5 +1,19 @@ ( function ( $, mw ) { - var header, + var header = [ 'Planet', 'Radius (km)' ], + + // Data set "planets" + mercury = [ 'Mercury', '2439.7' ], + venus = [ 'Venus', '6051.8' ], + earth = [ 'Earth', '6371.0' ], + mars = [ 'Mars', '3390.0' ], + jupiter = [ 'Jupiter', '69911' ], + saturn = [ 'Saturn', '58232' ], + planets = [ mercury, venus, earth, mars, jupiter, saturn ], + planetsAscName = [ earth, jupiter, mars, mercury, saturn, venus ], + planetsAscRadius = [ mercury, mars, venus, earth, saturn, jupiter ], + planetsRowspan, + planetsRowspanII, + planetsAscNameLegacy, // Data set "simple" a1 = [ 'A', '1' ], @@ -13,6 +27,7 @@ simpleDescasc = [ b1, b2, b3, a1, a2, a3 ], // Data set "colspan" + header4 = [ 'column1a', 'column1b', 'column1c', 'column2' ], aaa1 = [ 'A', 'A', 'A', '1' ], aab5 = [ 'A', 'A', 'B', '5' ], abc3 = [ 'A', 'B', 'C', '3' ], @@ -20,20 +35,6 @@ caa4 = [ 'C', 'A', 'A', '4' ], colspanInitial = [ aab5, aaa1, abc3, bbc2, caa4 ], - // Data set "planets" - mercury = [ 'Mercury', '2439.7' ], - venus = [ 'Venus', '6051.8' ], - earth = [ 'Earth', '6371.0' ], - mars = [ 'Mars', '3390.0' ], - jupiter = [ 'Jupiter', '69911' ], - saturn = [ 'Saturn', '58232' ], - planets = [ mercury, venus, earth, mars, jupiter, saturn ], - planetsAscName = [ earth, jupiter, mars, mercury, saturn, venus ], - planetsAscRadius = [ mercury, mars, venus, earth, saturn, jupiter ], - planetsRowspan, - planetsRowspanII, - planetsAscNameLegacy, - // Data set "ipv4" ipv4 = [ // Some randomly generated fake IPs @@ -302,7 +303,6 @@ } // Sample data set using planets named and their radius - header = [ 'Planet', 'Radius (km)' ]; tableTest( 'Basic planet table: sorting initially - ascending by name', @@ -388,9 +388,6 @@ $table.find( '.headerSort:eq(1)' ).click().click(); } ); - - header = [ 'column1', 'column2' ]; - tableTest( 'Sorting multiple columns by passing sort list', header, @@ -500,10 +497,9 @@ } ); // Sorting with colspans - header = [ 'column1a', 'column1b', 'column1c', 'column2' ]; tableTest( 'Sorting with colspanned headers: spanned column', - header, + header4, colspanInitial, [ aaa1, aab5, abc3, bbc2, caa4 ], function ( $table ) { @@ -516,7 +512,7 @@ } ); tableTest( 'Sorting with colspanned headers: sort spanned column twice', - header, + header4, colspanInitial, [ caa4, bbc2, abc3, aab5, aaa1 ], function ( $table ) { @@ -530,7 +526,7 @@ } ); tableTest( 'Sorting with colspanned headers: subsequent column', - header, + header4, colspanInitial, [ aaa1, bbc2, abc3, caa4, aab5 ], function ( $table ) { @@ -543,7 +539,7 @@ } ); tableTest( 'Sorting with colspanned headers: sort subsequent column twice', - header, + header4, colspanInitial, [ aab5, caa4, abc3, bbc2, aaa1 ], function ( $table ) { @@ -557,18 +553,36 @@ } ); - tableTest( - 'Basic planet table: one unsortable column', - header, - planets, - planets, - function ( $table ) { - $table.find( 'tr:eq(0) > th:eq(0)' ).addClass( 'unsortable' ); + QUnit.test( 'Basic planet table: one unsortable column', 3, function ( assert ) { + var $table = tableCreate( header, planets ), + $cell; + $table.find( 'tr:eq(0) > th:eq(0)' ).addClass( 'unsortable' ); - $table.tablesorter(); - $table.find( 'tr:eq(0) > th:eq(0)' ).click(); - } - ); + $table.tablesorter(); + $table.find( 'tr:eq(0) > th:eq(0)' ).click(); + + assert.deepEqual( + tableExtract( $table ), + planets, + 'table not sorted' + ); + + $cell = $table.find( 'tr:eq(0) > th:eq(0)' ); + $table.find( 'tr:eq(0) > th:eq(1)' ).click(); + + assert.equal( + $cell.hasClass( 'headerSortUp' ) || $cell.hasClass( 'headerSortDown' ), + false, + 'after sort: no class headerSortUp or headerSortDown' + ); + + assert.equal( + $cell.attr( 'title' ), + undefined, + 'after sort: no title tag added' + ); + + } ); // Regression tests! tableTest( @@ -1262,14 +1276,14 @@ tableTestHTML( 'Rowspan exploding with colspanned cells (2)', '' + - '' + + '' + '' + - '' + - '' + + '' + + '' + '
nfoobarbazquux
nfoobarbazn2
1foobarbazquux
2foobarquux
1foobarbaz2
2foobar1
', [ - [ '1', 'foo', 'bar', 'baz', 'quux' ], - [ '2', 'foobar', 'baz', 'quux' ] + [ '2', 'foobar', 'baz', '1' ], + [ '1', 'foo', 'bar', 'baz', '2' ] ] ); @@ -1345,4 +1359,39 @@ ] ); + QUnit.test( 'bug 105731 - incomplete rows in table body', 3, function ( assert ) { + var $table, parsers; + $table = $( + '' + + '' + + '' + + '' + + '
AB
3
12
' + ); + $table.tablesorter(); + $table.find( '.headerSort:eq(0)' ).click(); + // now the first row have 2 columns + $table.find( '.headerSort:eq(1)' ).click(); + + parsers = $table.data( 'tablesorter' ).config.parsers; + + assert.equal( + parsers.length, + 2, + 'detectParserForColumn() detect 2 parsers' + ); + + assert.equal( + parsers[ 1 ].id, + 'number', + 'detectParserForColumn() detect parser.id "number" for second column' + ); + + assert.equal( + parsers[ 1 ].format( $table.find( 'tbody > tr > td:eq(1)' ).text() ), + 0, + 'empty cell is sorted as number 0' + ); + + } ); }( jQuery, mediaWiki ) ); -- 2.20.1