r96384 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96383‎ | r96384 | r96385 >
Date:23:09, 6 September 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
jquery.tablesorter.test: Add tests for data-sort-value
* Follows-up r86108
Modified paths:
  • /trunk/phase3/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
@@ -381,4 +381,87 @@
382382 }
383383 );
384384
 385+test( 'data-sort-value attribute, when available, should override sorting position', function() {
 386+ var $table, data;
 387+
 388+ // Simple example, one without data-sort-value which should be sorted at it's text.
 389+ $table = $(
 390+ '<table class="sortable"><thead><tr><th>Data</th></tr></thead>' +
 391+ '<tbody>' +
 392+ '<tr><td>Cheetah</td></tr>' +
 393+ '<tr><td data-sort-value="Apple">Bird</td></tr>' +
 394+ '<tr><td data-sort-value="Bananna">Ferret</td></tr>' +
 395+ '<tr><td data-sort-value="Drupe">Elephant</td></tr>' +
 396+ '<tr><td data-sort-value="Cherry">Dolphin</td></tr>' +
 397+ '</tbody></table>'
 398+ );
 399+ $table.tablesorter().find( '.headerSort:eq(0)' ).click();
 400+
 401+ data = [];
 402+ $table.find( 'tbody > tr' ).each( function( i, tr ) {
 403+ $( tr ).find( 'td' ).each( function( i, td ) {
 404+ data.push( { data: $( td ).data( 'sort-value' ), text: $( td ).text() } );
 405+ });
 406+ });
 407+
 408+ deepEqual( data, [
 409+ {
 410+ "data": "Apple",
 411+ "text": "Bird"
 412+ }, {
 413+ "data": "Bananna",
 414+ "text": "Ferret"
 415+ }, {
 416+ "data": undefined,
 417+ "text": "Cheetah"
 418+ }, {
 419+ "data": "Cherry",
 420+ "text": "Dolphin"
 421+ }, {
 422+ "data": "Drupe",
 423+ "text": "Elephant"
 424+ }
 425+ ] );
 426+
 427+ // Another example
 428+ $table = $(
 429+ '<table class="sortable"><thead><tr><th>Data</th></tr></thead>' +
 430+ '<tbody>' +
 431+ '<tr><td>D</td></tr>' +
 432+ '<tr><td data-sort-value="E">A</td></tr>' +
 433+ '<tr><td>B</td></tr>' +
 434+ '<tr><td>G</td></tr>' +
 435+ '<tr><td data-sort-value="F">C</td></tr>' +
 436+ '</tbody></table>'
 437+ );
 438+ $table.tablesorter().find( '.headerSort:eq(0)' ).click();
 439+
 440+ data = [];
 441+ $table.find( 'tbody > tr' ).each( function( i, tr ) {
 442+ $( tr ).find( 'td' ).each( function( i, td ) {
 443+ data.push( { data: $( td ).data( 'sort-value' ), text: $( td ).text() } );
 444+ });
 445+ });
 446+
 447+ deepEqual( data, [
 448+ {
 449+ "data": undefined,
 450+ "text": "B"
 451+ }, {
 452+ "data": undefined,
 453+ "text": "D"
 454+ }, {
 455+ "data": "E",
 456+ "text": "A"
 457+ }, {
 458+ "data": "F",
 459+ "text": "C"
 460+ }, {
 461+ "data": undefined,
 462+ "text": "G"
 463+ }
 464+ ] );
 465+
 466+});
 467+
385468 })();

Sign-offs

UserFlagDate
Brion VIBBERinspected22:10, 7 September 2011
Brion VIBBERtested22:10, 7 September 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r96509Followup to r86108, r86854, r96384: table sorter fetch of 'data-sort-value' a...brion22:03, 7 September 2011
r96842REL1_18: MFT r93288, r94212, r96261, r96263, r96384reedy14:42, 12 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86108Followup ro r86088: Use data-sort-type instead of classes to specify the pars...diebuche08:23, 15 April 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   21:42, 7 September 2011

Looks like getElementText() is failing to pick up the data-sort-value attribute value on IE 6/7, so it's sorting all the rows by their text values.

#Comment by Brion VIBBER (talk | contribs)   21:57, 7 September 2011

IE 6/7 lack the DOM hasAttribute / getAttribute methods on HTML elements. Switching the code to use jQuery's .attr() seems to work around this nicely; committing...

#Comment by Brion VIBBER (talk | contribs)   22:04, 7 September 2011

r96509 fixes this -- marking for 1.18 merge.

Status & tagging log