r45060 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r45059‎ | r45060 | r45061 >
Date:22:00, 26 December 2008
Author:raymond
Status:ok (Comments)
Tags:
Comment:
* (bug 16754) Making arbitrary rows of sortable tables sticky:
|- class="unsortable"
Patch by René Kijewski
Modified paths:
  • /trunk/phase3/CREDITS (modified) (history)
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/skins/common/wikibits.js (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/wikibits.js
@@ -637,13 +637,16 @@
638638 var reverse = (span.getAttribute("sortdir") == 'down');
639639
640640 var newRows = new Array();
 641+ var staticRows = new Array();
641642 for (var j = rowStart; j < table.rows.length; j++) {
642643 var row = table.rows[j];
643 - var keyText = ts_getInnerText(row.cells[column]);
644 - var oldIndex = (reverse ? -j : j);
645 - var preprocessed = preprocessor( keyText );
 644+ if((" "+row.className+" ").indexOf(" unsortable ") < 0) {
 645+ var keyText = ts_getInnerText(row.cells[column]);
 646+ var oldIndex = (reverse ? -j : j);
 647+ var preprocessed = preprocessor( keyText );
646648
647 - newRows[newRows.length] = new Array(row, preprocessed, oldIndex);
 649+ newRows[newRows.length] = new Array(row, preprocessed, oldIndex);
 650+ } else staticRows[staticRows.length] = new Array(row, false, j-rowStart);
648651 }
649652
650653 newRows.sort(sortfn);
@@ -658,6 +661,11 @@
659662 span.setAttribute('sortdir','down');
660663 }
661664
 665+ for(var i in staticRows) {
 666+ var row = staticRows[i];
 667+ newRows.splice(row[2], 0, row);
 668+ }
 669+
662670 // We appendChild rows that already exist to the tbody, so it moves them rather than creating new ones
663671 // don't do sortbottom rows
664672 for (var i = 0; i < newRows.length; i++) {
Index: trunk/phase3/CREDITS
@@ -74,6 +74,7 @@
7575 * Olaf Lenz
7676 * Paul Copperman
7777 * RememberTheDot
 78+* René Kijewski
7879 * ST47
7980
8081 == Translators ==
Index: trunk/phase3/includes/DefaultSettings.php
@@ -1444,7 +1444,7 @@
14451445 * to ensure that client-side caches don't keep obsolete copies of global
14461446 * styles.
14471447 */
1448 -$wgStyleVersion = '192';
 1448+$wgStyleVersion = '193';
14491449
14501450
14511451 # Server-side caching:
Index: trunk/phase3/RELEASE-NOTES
@@ -250,6 +250,8 @@
251251 * Special:Log: Add 'change protection' link for unprotected pages too
252252 * Special:Log: Add log type specific CSS classes 'mw-logline-$logtype' to
253253 'li' elements
 254+* (bug 16754) Making arbitrary rows of sortable tables sticky:
 255+ |- class="unsortable"
254256
255257 === Bug fixes in 1.14 ===
256258

Comments

#Comment by Aaron Schulz (talk | contribs)   04:52, 27 December 2008

'(" "+row.className+" ").indexOf' ?

#Comment by Revolus (talk | contribs)   13:55, 27 December 2008

It's by effect the same as /\s*unsortable\s*/.test(row.className). I think it's a little bit faster than the regexp variant as the regexp has to be parsed in every loop iteration (at least opera does so). This construction is already used in the code earlier, so one could think about change it all over?

#Comment by Revolus (talk | contribs)   13:57, 27 December 2008

not /\s*unsortable\s*/ but /(?:^|\s*)unsortable(?:$|\s*)/

#Comment by Brion VIBBER (talk | contribs)   17:39, 31 December 2008

You only have to test once per cell in the first row of each sortable table, so it's not super-time-critical, though you don't want to be super slow of course. :)

Given that it's already used in this form for other checks, and you really have to go out of your way to put non-space whitespace in there (literal tab and newline in attribute markup are normalized to spaces) I don't think there's any need to change it right now.

Status & tagging log