r90186 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r90185‎ | r90186 | r90187 >
Date:23:21, 15 June 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
Minor fixed in ext.vector.simpleSearch
* Aliasing jQuery to $ locally instead of implying global
* Don't use "!=" for length comparison, it's loose (may cast to boolean instead of numeral, 0 == false) and slower than a strict comparison.
* "size()" is the same number of characters than .length and returns exactly the same, saves a function call
* $.isArray does not return boolean as in PHP, it's a jQuery function to account for browser compatibility (ie. indexOf), returns -1 if not found.
* Removed mouseover/focus per r89950 CR
* Replaced spaces with tabs
* Passes JSHint
Modified paths:
  • /trunk/extensions/Vector/modules/ext.vector.simpleSearch.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Vector/modules/ext.vector.simpleSearch.js
@@ -1,6 +1,6 @@
22 /* JavaScript for SimpleSearch extension */
33
4 -$( document ).ready( function() {
 4+jQuery( document ).ready( function( $ ) {
55
66 // Compatibility map
77 var map = {
@@ -40,14 +40,12 @@
4141
4242 // General suggestions functionality for all search boxes
4343 $( '#searchInput, #searchInput2, #powerSearchText, #searchText' )
44 - .mouseover( function() { $(this).focus()
45 - })
4644 .suggestions( {
4745 fetch: function( query ) {
4846 var $this = $(this);
49 - if ( query.length != 0 ) {
 47+ if ( query.length !== 0 ) {
5048 var request = $.ajax( {
51 - url: mw.config.get( 'wgScriptPath' ) + '/api.php',
 49+ url: mw.util.wikiScript( 'api' ),
5250 data: {
5351 action: 'opensearch',
5452 search: query,
@@ -56,15 +54,15 @@
5755 },
5856 dataType: 'json',
5957 success: function( data ) {
60 - if ( $.isArray( data ) && 1 in data ) {
 58+ if ( $.isArray( data ) !== -1 && 1 in data ) {
6159 $this.suggestions( 'suggestions', data[1] );
6260 }
6361 }
6462 });
6563 $this.data( 'request', request );
66 - }
 64+ }
6765 },
68 - cancel: function () {
 66+ cancel: function() {
6967 var request = $(this).data( 'request' );
7068 // If the delay setting has caused the fetch to have not even happend yet, the request object will
7169 // have never been set
@@ -96,7 +94,7 @@
9795 },
9896 special: {
9997 render: function( query ) {
100 - if ( $(this).children().size() === 0 ) {
 98+ if ( $(this).children().length === 0 ) {
10199 $(this).show();
102100 var $label = $( '<div></div>', {
103101 'class': 'special-label',

Follow-up revisions

RevisionCommit summaryAuthorDate
r92190Fix typo from r90186. It's not inArray but isArray, which obviously does retu...krinkle20:30, 14 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r89950do not sent api requests for empty inputs, e.g. after a character input follo...wikinaut08:41, 13 June 2011

Comments

#Comment by Schnark (talk | contribs)   10:50, 14 July 2011

$.isArray does return a boolean, it's $.inArray that returns an index, but this function isn't used here.

#Comment by Krinkle (talk | contribs)   20:31, 14 July 2011

Thanks, must've read too quickly. Fixed in r92190.

Status & tagging log