r66621 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r66620‎ | r66621 | r66622 >
Date:19:58, 18 May 2010
Author:adam
Status:resolved (Comments)
Tags:
Comment:
Adding the expandableFields plugin for prototyping expandable search
Modified paths:
  • /trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/js/plugins/jquery.expandableField.js (added) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php
@@ -70,6 +70,7 @@
7171 array( 'src' => 'js/plugins/jquery.color.js', 'version' => 1 ),
7272 array( 'src' => 'js/plugins/jquery.cookie.js', 'version' => 4 ),
7373 array( 'src' => 'js/plugins/jquery.delayedBind.js', 'version' => 1 ),
 74+ array( 'src' => 'js/plugins/jquery.expandableField.js', 'version' => 15 ),
7475 array( 'src' => 'js/plugins/jquery.suggestions.js', 'version' => 15 ),
7576 array( 'src' => 'js/plugins/jquery.textSelection.js', 'version' => 33 ),
7677 array( 'src' => 'js/plugins/jquery.wikiEditor.js', 'version' => 187 ),
Index: trunk/extensions/UsabilityInitiative/js/plugins/jquery.expandableField.js
@@ -0,0 +1,119 @@
 2+/**
 3+ * This plugin provides functionallity to expand a text box on focus to double it's current width
 4+ *
 5+ * Usage:
 6+ *
 7+ * Set options:
 8+ * $('#textbox').expandableField( { option1: value1, option2: value2 } );
 9+ * $('#textbox').expandableField( option, value );
 10+ * Get option:
 11+ * value = $('#textbox').expandableField( option );
 12+ * Initialize:
 13+ * $('#textbox').expandableField();
 14+ *
 15+ * Options:
 16+ *
 17+ */
 18+( function( $ ) {
 19+
 20+$.expandableField = {
 21+ /**
 22+ * Cancel any delayed updateSuggestions() call and inform the user so
 23+ * they can cancel their result fetching if they use AJAX or something
 24+ */
 25+ expandField: function( e, context ) {
 26+ context.data.$field
 27+ .css( { 'display' : 'inline-block' } )
 28+ .animate( { 'width': context.data.expandedWidth } );
 29+ },
 30+ /**
 31+ * Restore the text the user originally typed in the textbox, before it was overwritten by highlight(). This
 32+ * restores the value the currently displayed suggestions are based on, rather than the value just before
 33+ * highlight() overwrote it; the former is arguably slightly more sensible.
 34+ */
 35+ condenseField: function( e, context ) {
 36+ context.data.$field
 37+ .css( { 'display' : 'inline-block' } )
 38+ .animate( { 'width': context.data.condensedWidth, 'display': 'inline'} );
 39+ },
 40+ /**
 41+ * Sets the value of a property, and updates the widget accordingly
 42+ * @param {String} property Name of property
 43+ * @param {Mixed} value Value to set property with
 44+ */
 45+ configure: function( context, property, value ) {
 46+ // Validate creation using fallback values
 47+ switch( property ) {
 48+ default:
 49+ context.config[property] = value;
 50+ break;
 51+ }
 52+ }
 53+
 54+};
 55+$.fn.expandableField = function() {
 56+
 57+ // Multi-context fields
 58+ var returnValue = null;
 59+ var args = arguments;
 60+
 61+ $( this ).each( function() {
 62+
 63+ /* Construction / Loading */
 64+
 65+ var context = $( this ).data( 'expandableField-context' );
 66+ if ( context == null ) {
 67+ context = {
 68+ config: {
 69+ }
 70+ };
 71+ }
 72+
 73+ /* API */
 74+
 75+ // Handle various calling styles
 76+ if ( args.length > 0 ) {
 77+ if ( typeof args[0] == 'object' ) {
 78+ // Apply set of properties
 79+ for ( var key in args[0] ) {
 80+ $.suggestions.configure( context, key, args[0][key] );
 81+ }
 82+ } else if ( typeof args[0] == 'string' ) {
 83+ if ( args.length > 1 ) {
 84+ // Set property values
 85+ $.suggestions.configure( context, args[0], args[1] );
 86+ } else if ( returnValue == null ) {
 87+ // Get property values, but don't give access to internal data - returns only the first
 88+ returnValue = ( args[0] in context.config ? undefined : context.config[args[0]] );
 89+ }
 90+ }
 91+ }
 92+
 93+ /* Initialization */
 94+
 95+ if ( typeof context.data == 'undefined' ) {
 96+ context.data = {
 97+ // The width of the field in it's condensed state
 98+ 'condensedWidth': $( this ).width(),
 99+ // The width of the field in it's expanded state
 100+ 'expandedWidth': $( this ).width() * 2,
 101+ // Reference to the field
 102+ '$field': $( this )
 103+ };
 104+
 105+ $( this )
 106+ .addClass( 'expandableField-condensed' )
 107+ .focus( function( e ) {
 108+ $.expandableField.expandField( e, context );
 109+ } )
 110+ .blur( function( e ) {
 111+ $.expandableField.condenseField( e, context );
 112+ } );
 113+ }
 114+ // Store the context for next time
 115+ $( this ).data( 'expandableField-context', context );
 116+ } );
 117+ return returnValue !== null ? returnValue : $(this);
 118+};
 119+
 120+} )( jQuery );

Comments

#Comment by Catrope (talk | contribs)   20:14, 18 May 2010
+	/**
+	 * Cancel any delayed updateSuggestions() call and inform the user so
+	 * they can cancel their result fetching if they use AJAX or something
+	 */
+	expandField: function( e, context ) {
+		context.data.$field
+		.css( { 'display' : 'inline-block' } )
+		.animate( { 'width': context.data.expandedWidth } );
+	},
+	/**
+	 * Restore the text the user originally typed in the textbox, before it was overwritten by highlight(). This
+	 * restores the value the currently displayed suggestions are based on, rather than the value just before
+	 * highlight() overwrote it; the former is arguably slightly more sensible.
+	 */
+	condenseField: function( e, context ) {
+		context.data.$field
+			.css( { 'display' : 'inline-block' } )
+			.animate( { 'width': context.data.condensedWidth, 'display': 'inline'} );
+	},

These comments don't seem to match the code beneath them.

+					$.suggestions.configure( context, key, args[0][key] );
[...]
+					$.suggestions.configure( context, args[0], args[1] );

Should be $.expandableField

+		if ( typeof context.data == 'undefined' ) {
[...]
+		}
+		// Store the context for next time
+		$( this ).data( 'expandableField-context', context );

The context should only be set once. Any updates to the context after that will use a reference.

+		var context = $( this ).data( 'expandableField-context' );
+		if ( context == null ) {
+			context = {
+				config: {
+				}
+			};
+		}
[code handling call arguments]
+		if ( typeof context.data == 'undefined' ) {
+			context.data = {

It's not clear to me why the initialization is so far down; it also seems like this potentially overrides stuff set before and makes certain calls fail.

#Comment by Adammiller~mediawikiwiki (talk | contribs)   21:16, 18 May 2010

Sorry, I should really make a not in my commit message when I'm committing only so I can switch computers, and not because I think my code is perfect and ready for lashing.


#Comment by Adammiller~mediawikiwiki (talk | contribs)   21:17, 18 May 2010

and of course by saying "make a not in my commit message", I in fact meant "make a note in my commit message".

#Comment by Adammiller~mediawikiwiki (talk | contribs)   20:00, 1 June 2010

All issues should be resolved as of r66854

Status & tagging log