r90990 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90989‎ | r90990 | r90991 >
Date:19:32, 28 June 2011
Author:hashar
Status:ok (Comments)
Tags:
Comment:
Find 'all' special ns regardless of its rank in the select

This is done by looking for an <option> element with an empty string value
which denote the special case 'all'.
Also removed a comment in Xml.php asking to keep 'all' as a first element
since it is no longer needed.

Follow up r90943 (comment 18872 by Brion)
Modified paths:
  • /trunk/phase3/includes/Xml.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.special/mediawiki.special.recentchanges.js (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Xml.php
——@@ -132,8 +132,6 @@
133133 }
134134
135135 if( !is_null( $all ) )
136 - # Please make sure the 'namespacesall' is the first or you will break
137 - # such an assumption (ex js: mw.special.recentchanges.updateCheckboxes)
138136 $namespaces = array( $all => wfMsg( 'namespacesall' ) ) + $namespaces;
139137 foreach( $namespaces as $index => $name ) {
140138 if( $index < NS_MAIN ) {
Index: trunk/phase3/resources/mediawiki.special/mediawiki.special.recentchanges.js
——@@ -15,8 +15,8 @@
1616 * special 'all' namespace is selected/unselected respectively.
1717 */
1818 updateCheckboxes: function() {
19 - // The 'all' namespace is the FIRST in the list.
20 - var isAllNS = $select.find( 'option' ).first().is( ':selected' );
 19+ // The option element for the 'all' namespace has an empty value
 20+ var isAllNS = ('' === $select.find('option:selected').val() );
2121
2222 // Iterates over checkboxes and propagate the selected option
2323 $.each( checkboxes, function( i, id ) {

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r90943Disable ns selector checkboxes when 'all' namespace is selected...hashar06:40, 28 June 2011

Comments

#Comment by Hashar (talk | contribs)   19:33, 28 June 2011

Qunit tests pass on my local machine.

#Comment by Brion VIBBER (talk | contribs)   22:01, 28 June 2011

Shortcut: you can just use $select.val() rather than diving in to find the selected option element manually.

#Comment by Hashar (talk | contribs)   17:16, 29 June 2011

Looks like JQuery make everything just TOO easy!

Status & tagging log