User "Krinkle" changed the status of MediaWiki.r90359. Old Status: new New Status: fixme
User "Krinkle" also posted a comment on MediaWiki.r90359. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/90359#c18419 Commit summary: (bug 29443) Special:Undelete should invert checkboxes without reloading Committed patch with code originally by [[b:User:Darklama]] Comment: Aside from that, marking as fixme per the next few points: * Why is this callback function returning false ? It is therewith blocking any and all other events that may be bound, use e.preventDefault() instead. * Although the difference in this particular case is quite small (since SpecialUndelete pages are usually short), the selector is ineffecient right now. It uses "input:checkbox" which means all {{tag|input|open}} elements on the page are queried and one-by-one checked to see if it matches the checkbox-profile. On my localhost a gadget puts a checkbox next to the searchbar in Vector, this is wrongly toggled when I click the Invert button now! Instead make use of the ''context''. <blockquote><source lang="javascript"> $('#mw-undelete-table') /* <= seperate find() call to speed up the ID-selector */.find('input:checkbox') /* <= only looks within the undelete-table */ </source></blockquote> * Right now there are four anonymous functions. A closure to alias $ to jQuery, a shortcut to $(document).ready with $(fn), a function passed to click(), and another one passed to .each(). The former two can be merged by using the fact that the document-ready function is called with jQuery as first argument. This saves an additional function call and a little bandwidth as well. The latter each() is redundant as jQuery's internal methods automatically do each (ie. <code>$('.foo').attr('title', 'Hello')</code> sets the attribute for each of the selected elements, no need for an each() construction. The internal looping system is a light-weight for()-loop that optionally takes a function for non-static inputs, such as "!value" like is the case here.) * Attribution is done through Wikimedia-specific interwiki links. Aside from them obviously not working in a .js file (remember, this is not a Gadget), they are also Wikimedia-specific, please use full URLs where possible. Due to the simplicity of this function though and the convention of not listing authors for individual functions, perhaps it should be removed. * Check out the [[Manual:Coding_conventions#Indenting_and_alignment|whitespace conventions]] there should be more spaces in function calls etc. <source lang="javascript"> jQuery( document ).ready( function( $ ) { $( '#mw-undelete-invert' ).click( function( e ) { e.preventDefault(); $( '#mw-undelete-table' ).find( 'input:checkbox' ).prop( 'checked', function( i, val ) { return !val; } ); }); } ); _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
