r89951 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89950‎ | r89951 | r89952 >
Date:09:38, 13 June 2011
Author:wikinaut
Status:resolved (Comments)
Tags:
Comment:
fix for bug29368 which may have consequences. Please code reviewers, pls. check for potential side effects - I cannot detect any, currently.
Modified paths:
  • /trunk/phase3/resources/jquery/jquery.suggestions.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/jquery/jquery.suggestions.js
@@ -66,20 +66,26 @@
6767 },
6868 /**
6969 * Ask the user-specified callback for new suggestions. Any previous delayed call to this function still pending
70 - * will be canceled. If the value in the textbox hasn't changed since the last time suggestions were fetched, this
 70+ * will be canceled. If the value in the textbox is or hasn't changed since the last time suggestions were fetched, this
7171 * function does nothing.
7272 * @param {Boolean} delayed Whether or not to delay this by the currently configured amount of time
7373 */
7474 update: function( context, delayed ) {
75 - // Only fetch if the value in the textbox changed
 75+ // Only fetch if the value in the textbox changed and is not empty
7676 function maybeFetch() {
77 - if ( context.data.$textbox.val() !== context.data.prevText ) {
 77+ if ( ( context.data.$textbox.val().length != 0 ) && ( context.data.$textbox.val() !== context.data.prevText ) ) {
7878 context.data.prevText = context.data.$textbox.val();
7979 if ( typeof context.config.fetch == 'function' ) {
8080 context.config.fetch.call( context.data.$textbox, context.data.$textbox.val() );
8181 }
8282 }
8383 }
 84+
 85+ // clear result div if the value in the text is empty
 86+ if ( context.data.$textbox.val().length != 0 ) {
 87+ context.data.$container.hide();
 88+ }
 89+
8490 // Cancel previous call
8591 if ( context.data.timerID != null ) {
8692 clearTimeout( context.data.timerID );

Follow-up revisions

RevisionCommit summaryAuthorDate
r89952corrected commentwikinaut09:41, 13 June 2011
r89954removed code for clearing result div in case of empty input textbox. This is ...wikinaut09:47, 13 June 2011
r89955essage=corrected my mistake when comparing if textbox length is empty, or not.wikinaut09:56, 13 June 2011

Comments

#Comment by Wikinaut (talk | contribs)   09:42, 13 June 2011

in 89952: corrected comment s/is or/is empty or/

#Comment by Alphos (talk | contribs)   09:49, 13 June 2011

+ // clear result div if the value in the text is empty + if ( context.data.$textbox.val().length != 0 ) { Is it me or does the code do the exact opposite of the comment right above it ?

+ context.data.$container.hide(); Jquery.hide() is not equivalent to Jquery.empty() : the former hides it using CSS, whereas the latter does the actual clearing of its contents. Should the div be hidden (while retaining the same content) or should the whole content of the div be shredded ?

#Comment by Wikinaut (talk | contribs)   09:50, 13 June 2011

I copied the part I am using from similar line 133 in configure I think, this is fully correct to recycle working code

#Comment by Wikinaut (talk | contribs)   09:52, 13 June 2011

see lines 130++ :

if ( typeof context.data !== 'undefined' ) { if ( context.data.$textbox.val().length == 0 ) { // Hide the div when no suggestion exist context.data.$container.hide(); } else { // Rebuild the suggestions list

#Comment by Wikinaut (talk | contribs)   09:52, 13 June 2011

see lines 130++ :

  • if ( typeof context.data !== 'undefined' ) {
  • if ( context.data.$textbox.val().length == 0 ) {
  • // Hide the div when no suggestion exist
            • >>> context.data.$container.hide();
  • } else {
  • // Rebuild the suggestions list
#Comment by Wikinaut (talk | contribs)   09:57, 13 June 2011

corrected condition in s/!= 0/== 0/

Status & tagging log