r83605 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r83604‎ | r83605 | r83606 >
Date:19:32, 9 March 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
JavaScript clean-up (Code conventions, 1.17 migration, cross-browser fixes and JSHint validation)
: Vector SimpleSearch
* Using the mw-alias global
* Optimizing selector (Sizzle starts at the right, about 3x faster now )
*--> see also http://jsperf.com/jquery-selector-perf-select-right-to-left )
* Using mw.config instead of legacy globals
* Before checking "foo in bar" making sure it's an array as it'll throw an exception
* typeof is not reliable for functions cross-browser, using $.isFunction
* $.fn.is() is fairly slow as it parses the string and then calls filter which will then do it's thing and eventually call hasClass(). Calling it directly now
* Except for elements that can't have children (<input>) or can't be close (<img />), the html string should be valid and have a closing tag.("<input />"-> "<input>", "<div />" -> "<div></div>"). See also http://api.jquery.com/jQuery/
* Using jQuery 1.4's new ability to set element properties in the initial function call by passing an object as second argument instead of chain().of().calls() afterwards (see docs for what properties can and can't be set this way)
* Using strict === comparison to compare to zero (faster and safer)
Modified paths:
  • /trunk/extensions/Vector/modules/ext.vector.simpleSearch.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Vector/modules/ext.vector.simpleSearch.js
@@ -1,7 +1,7 @@
22 /* JavaScript for SimpleSearch extension */
33
44 $( document ).ready( function() {
5 -
 5+
66 // Compatibility map
77 var map = {
88 'browsers': {
@@ -27,33 +27,33 @@
2828 if ( !$.client.test( map ) ) {
2929 return true;
3030 }
31 -
 31+
3232 // Disable MWSuggest if loaded
3333 if ( window.os_MWSuggestDisable ) {
3434 window.os_MWSuggestDisable();
3535 }
36 -
 36+
3737 // Placeholder text for SimpleSearch box
38 - $( 'div#simpleSearch > input#searchInput' )
39 - .attr( 'placeholder', mediaWiki.msg( 'vector-simplesearch-search' ) )
 38+ $( '#simpleSearch > input#searchInput' )
 39+ .attr( 'placeholder', mw.msg( 'vector-simplesearch-search' ) )
4040 .placeholder();
41 -
 41+
4242 // General suggestions functionality for all search boxes
4343 $( '#searchInput, #searchInput2, #powerSearchText, #searchText' )
4444 .suggestions( {
4545 fetch: function( query ) {
4646 var $this = $(this);
4747 var request = $.ajax( {
48 - url: wgScriptPath + '/api.php',
 48+ url: mw.config.get( 'wgScriptPath' ) + '/api.php',
4949 data: {
50 - 'action': 'opensearch',
51 - 'search': query,
52 - 'namespace': 0,
53 - 'suggest': ''
 50+ action: 'opensearch',
 51+ search: query,
 52+ namespace: 0,
 53+ suggest: ''
5454 },
5555 dataType: 'json',
5656 success: function( data ) {
57 - if ( data && 1 in data ) {
 57+ if ( $.isArray( data ) && 1 in data ) {
5858 $this.suggestions( 'suggestions', data[1] );
5959 }
6060 }
@@ -64,7 +64,7 @@
6565 var request = $(this).data( 'request' );
6666 // If the delay setting has caused the fetch to have not even happend yet, the request object will
6767 // have never been set
68 - if ( request && typeof request.abort == 'function' ) {
 68+ if ( request && $.isFunction( request.abort ) ) {
6969 request.abort();
7070 $(this).removeData( 'request' );
7171 }
@@ -75,7 +75,7 @@
7676 }
7777 },
7878 delay: 120,
79 - positionFromLeft: $( 'body' ).is( '.rtl' ),
 79+ positionFromLeft: $( 'body' ).hasClass( 'rtl' ),
8080 highlightInput: true
8181 } )
8282 .bind( 'paste cut drop', function( e ) {
@@ -92,15 +92,17 @@
9393 },
9494 special: {
9595 render: function( query ) {
96 - if ( $(this).children().size() == 0 ) {
 96+ if ( $(this).children().size() === 0 ) {
9797 $(this).show();
98 - $label = $( '<div />' )
99 - .addClass( 'special-label' )
100 - .text( mediaWiki.msg( 'vector-simplesearch-containing' ) )
 98+ var $label = $( '<div></div>', {
 99+ 'class': 'special-label',
 100+ text: mw.msg( 'vector-simplesearch-containing' )
 101+ })
101102 .appendTo( $(this) );
102 - $query = $( '<div />' )
103 - .addClass( 'special-query' )
104 - .text( query )
 103+ var $query = $( '<div></div>', {
 104+ 'class': 'special-query',
 105+ text: query
 106+ })
105107 .appendTo( $(this) );
106108 $query.autoEllipsis();
107109 } else {
@@ -112,11 +114,15 @@
113115 },
114116 select: function( $input ) {
115117 $input.closest( 'form' ).append(
116 - $( '<input />' ).attr( { 'type': 'hidden', 'name': 'fulltext', 'value': 1 } )
 118+ $( '<input>', {
 119+ type: 'hidden',
 120+ name: 'fulltext',
 121+ val: '1'
 122+ })
117123 );
118124 $input.closest( 'form' ).submit();
119125 }
120126 },
121127 $region: $( '#simpleSearch' )
122128 } );
123 -});
 129+});
\ No newline at end of file

Comments

#Comment by Krinkle (talk | contribs)   19:38, 9 March 2011

Also made to global variables local:

-					$label = $( '<div />' )
+					var $label = $( '<div></div>', {

-					$query = $( '<div />' )
+					var $query =

Status & tagging log