r66658 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r66657‎ \| r66658 \| r66659 >
Date:	21:18, 19 May 2010
Author:	adam
Status:	deferred (Comments)
Tags:
Comment:	Adding match highlighting to the suggestions plugin
Modified paths:	/trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php (modified) (history) /trunk/extensions/UsabilityInitiative/css/combined.css (modified) (history) /trunk/extensions/UsabilityInitiative/css/combined.min.css (modified) (history) /trunk/extensions/UsabilityInitiative/css/suggestions.css (modified) (history) /trunk/extensions/UsabilityInitiative/js/plugins.combined.js (modified) (history) /trunk/extensions/UsabilityInitiative/js/plugins.combined.min.js (modified) (history) /trunk/extensions/UsabilityInitiative/js/plugins/jquery.suggestions.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/css/suggestions.css
—	—	@@ -39,8 +39,9 @@
40	40	margin: 0;
41	41	width: 100%;
42	42	}
43		~~-.suggestions-result span {~~
	43	+.suggestions-result div {
44	44	line-height: 1.5em;
	45	+ display: inline;
45	46	padding: 0.25em 0.25em;
46	47	}
47	48	.suggestions-result-current {
Index: trunk/extensions/UsabilityInitiative/css/combined.css
—	—	@@ -39,8 +39,9 @@
40	40	margin: 0;
41	41	width: 100%;
42	42	}
43		~~-.suggestions-result span {~~
	43	+.suggestions-result div {
44	44	line-height: 1.5em;
	45	+ display: inline;
45	46	padding: 0.25em 0.25em;
46	47	}
47	48	.suggestions-result-current {
Index: trunk/extensions/UsabilityInitiative/css/combined.min.css
—	—	@@ -37,8 +37,9 @@
38	38	margin:0;
39	39	width:100%;
40	40	}
41		~~-.suggestions-result span{~~
	41	+.suggestions-result div{
42	42	line-height:1.5em;
	43	+display:inline;
43	44	padding:0.25em 0.25em;
44	45	}
45	46	.suggestions-result-current{
Index: trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php
—	—	@@ -18,7 +18,7 @@
19	19	private static $styleFiles = array(
20	20	'base_sets' => array(
21	21	'raw' => array(
22		~~- array( 'src' => 'css/suggestions.css', 'version' => 13 ),~~
	22	+ array( 'src' => 'css/suggestions.css', 'version' => 14 ),
23	23	array( 'src' => 'css/vector.collapsibleNav.css', 'version' => 9 ),
24	24	array( 'src' => 'css/vector.footerCleanup.css', 'version' => 2 ),
25	25	array( 'src' => 'css/wikiEditor.css', 'version' => 13 ),
—	—	@@ -29,11 +29,11 @@
30	30	array( 'src' => 'css/vector/jquery-ui-1.7.2.css', 'version' => '1.7.2y' ),
31	31	),
32	32	'combined' => array(
33		~~- array( 'src' => 'css/combined.css', 'version' => 93 ),~~
	33	+ array( 'src' => 'css/combined.css', 'version' => 94 ),
34	34	array( 'src' => 'css/vector/jquery-ui-1.7.2.css', 'version' => '1.7.2y' ),
35	35	),
36	36	'minified' => array(
37		~~- array( 'src' => 'css/combined.min.css', 'version' => 93 ),~~
	37	+ array( 'src' => 'css/combined.min.css', 'version' => 94 ),
38	38	array( 'src' => 'css/vector/jquery-ui-1.7.2.css', 'version' => '1.7.2y' ),
39	39	),
40	40	)
—	—	@@ -72,7 +72,7 @@
73	73	array( 'src' => 'js/plugins/jquery.delayedBind.js', 'version' => 1 ),
74	74	array( 'src' => 'js/plugins/jquery.suggestions.js', 'version' => 18 ),
75	75	array( 'src' => 'js/plugins/jquery.expandableField.js', 'version' => 15 ),
76		~~- array( 'src' => 'js/plugins/jquery.suggestions.js', 'version' => 18 ),~~
	76	+ array( 'src' => 'js/plugins/jquery.suggestions.js', 'version' => 19 ),
77	77	array( 'src' => 'js/plugins/jquery.textSelection.js', 'version' => 33 ),
78	78	array( 'src' => 'js/plugins/jquery.wikiEditor.js', 'version' => 187 ),
79	79	array( 'src' => 'js/plugins/jquery.wikiEditor.highlight.js', 'version' => 53 ),
—	—	@@ -85,10 +85,10 @@
86	86	array( 'src' => 'js/plugins/jquery.wikiEditor.publish.js', 'version' => 5 ),
87	87	),
88	88	'combined' => array(
89		~~- array( 'src' => 'js/plugins.combined.js', 'version' => 402 ),~~
	89	+ array( 'src' => 'js/plugins.combined.js', 'version' => 403 ),
90	90	),
91	91	'minified' => array(
92		~~- array( 'src' => 'js/plugins.combined.min.js', 'version' => 402 ),~~
	92	+ array( 'src' => 'js/plugins.combined.min.js', 'version' => 403 ),
93	93	),
94	94	),
95	95	);
Index: trunk/extensions/UsabilityInitiative/js/plugins/jquery.suggestions.js
—	—	@@ -38,6 +38,8 @@
39	39	* Type: Number, Range: 1 - infinity, Default: 3
40	40	* positionFromLeft: Whether to position the suggestion box with the left attribute or the right
41	41	* Type: Boolean, Default: true
	42	+ * highlightInput: Highlights the matched text in the suggestions
	43	+ * Type: Boolean, Default: true
42	44	*/
43	45	( function( $ ) {
44	46
—	—	@@ -146,6 +148,14 @@
147	149	var expWidth = -1;
148	150	var $autoEllipseMe = $( [] );
149	151	for ( var i = 0; i < context.config.suggestions.length; i++ ) {
	152	+ var text = context.config.suggestions[i];
	153	+ if( context.config.highlightInput ) {
	154	+ var text = '<strong>'
	155	+ + text.substr( 0, context.data.prevText.length)
	156	+ + '</strong>'
	157	+ + text.substr( context.data.prevText.length, text.length );
	158	+ }
	159	+
150	160	var $result = $( '<div />' )
151	161	.addClass( 'suggestions-result' )
152	162	.attr( 'rel', i )
—	—	@@ -159,12 +169,12 @@
160	170
161	171	// Allow custom rendering
162	172	if ( typeof context.config.result.render == 'function' ) {
163		~~- context.config.result.render.call( $result, context.config.suggestions[i] );~~
	173	+ context.config.result.render.call( $result, text );
164	174	} else {
165	175	// Add <span> with text
166		~~- $result.append( $( '<span />' )~~
	176	+ $result.append( $( '<div />' )
167	177	.css( 'whiteSpace', 'nowrap' )
168		~~- .text( context.config.suggestions[i] )~~
	178	+ .html( text )
169	179	);
170	180
171	181	// Widen results box if needed
—	—	@@ -197,6 +207,7 @@
198	208	break;
199	209	case 'submitOnClick':
200	210	case 'positionFromLeft':
	211	+ case 'highlightInput':
201	212	context.config[property] = value ? true : false;
202	213	break;
203	214	}
—	—	@@ -348,7 +359,8 @@
349	360	'delay': 120,
350	361	'submitOnClick': false,
351	362	'maxExpandFactor': 3,
352		~~- 'positionFromLeft': true~~
	363	+ 'positionFromLeft': true,
	364	+ 'highlightInput': true
353	365	}
354	366	};
355	367	}
Index: trunk/extensions/UsabilityInitiative/js/plugins.combined.js
—	—	@@ -5795,6 +5795,8 @@
5796	5796	* Type: Number, Range: 1 - infinity, Default: 3
5797	5797	* positionFromLeft: Whether to position the suggestion box with the left attribute or the right
5798	5798	* Type: Boolean, Default: true
	5799	+ * highlightInput: Highlights the matched text in the suggestions
	5800	+ * Type: Boolean, Default: true
5799	5801	*/
5800	5802	( function( $ ) {
5801	5803
—	—	@@ -5903,6 +5905,14 @@
5904	5906	var expWidth = -1;
5905	5907	var $autoEllipseMe = $( [] );
5906	5908	for ( var i = 0; i < context.config.suggestions.length; i++ ) {
	5909	+ var text = context.config.suggestions[i];
	5910	+ if( context.config.highlightInput ) {
	5911	+ var text = '<strong>'
	5912	+ + text.substr( 0, context.data.prevText.length)
	5913	+ + '</strong>'
	5914	+ + text.substr( context.data.prevText.length, text.length );
	5915	+ }
	5916	+
5907	5917	var $result = $( '<div />' )
5908	5918	.addClass( 'suggestions-result' )
5909	5919	.attr( 'rel', i )
—	—	@@ -5916,12 +5926,12 @@
5917	5927
5918	5928	// Allow custom rendering
5919	5929	if ( typeof context.config.result.render == 'function' ) {
5920		~~- context.config.result.render.call( $result, context.config.suggestions[i] );~~
	5930	+ context.config.result.render.call( $result, text );
5921	5931	} else {
5922	5932	// Add <span> with text
5923		~~- $result.append( $( '<span />' )~~
	5933	+ $result.append( $( '<div />' )
5924	5934	.css( 'whiteSpace', 'nowrap' )
5925		~~- .text( context.config.suggestions[i] )~~
	5935	+ .html( text )
5926	5936	);
5927	5937
5928	5938	// Widen results box if needed
—	—	@@ -5954,6 +5964,7 @@
5955	5965	break;
5956	5966	case 'submitOnClick':
5957	5967	case 'positionFromLeft':
	5968	+ case 'highlightInput':
5958	5969	context.config[property] = value ? true : false;
5959	5970	break;
5960	5971	}
—	—	@@ -6105,7 +6116,8 @@
6106	6117	'delay': 120,
6107	6118	'submitOnClick': false,
6108	6119	'maxExpandFactor': 3,
6109		~~- 'positionFromLeft': true~~
	6120	+ 'positionFromLeft': true,
	6121	+ 'highlightInput': true
6110	6122	}
6111	6123	};
6112	6124	}
Index: trunk/extensions/UsabilityInitiative/js/plugins.combined.min.js
—	—	@@ -397,11 +397,15 @@
398	398	if(delayed){context.data.timerID=setTimeout(maybeFetch,context.config.delay);}else{maybeFetch();}
399	399	$.suggestions.special(context);},special:function(context){if(typeof context.config.special.render=='function'){setTimeout(function(){$special=context.data.$container.find('.suggestions-special');context.config.special.render.call($special,context.data.$textbox.val());},1);}},configure:function(context,property,value){switch(property){case'fetch':case'cancel':case'special':case'result':case'$region':context.config[property]=value;break;case'suggestions':context.config[property]=value;if(typeof context.data!=='undefined'){if(context.data.$textbox.val().length==0){context.data.$container.hide();}else{context.data.$container.show();var newCSS={'top':context.config.$region.offset().top+context.config.$region.outerHeight(),'bottom':'auto','width':context.config.$region.outerWidth(),'height':'auto'}
400	400	if(context.config.positionFromLeft){newCSS['left']=context.config.$region.offset().left;newCSS['right']='auto';}else{newCSS['left']='auto';newCSS['right']=$('body').width()-(context.config.$region.offset().left+context.config.$region.outerWidth());}
401		-context.data.$container.css(newCSS);var $results=context.data.$container.children('.suggestions-results');$results.empty();var expWidth=-1;var $autoEllipseMe=$([]);for(var i=0;i<context.config.suggestions.length;i++){var $result=$('<div />').addClass('suggestions-result').attr('rel',i).data('text',context.config.suggestions[i]).mouseover(function(e){$.suggestions.highlight(context,$(this).closest('.suggestions-results div'),false);}).appendTo($results);if(typeof context.config.result.render=='function'){context.config.result.render.call($result,context.config.suggestions[i]);}else{$result.append($('<span />').css('whiteSpace','nowrap').text(context.config.suggestions[i]));var $span=$result.children('span');if($span.outerWidth()>$result.width()&&$span.outerWidth()>expWidth){expWidth=$span.outerWidth();}
	401	+context.data.$container.css(newCSS);var $results=context.data.$container.children('.suggestions-results');$results.empty();var expWidth=-1;var $autoEllipseMe=$([]);for(var i=0;i<context.config.suggestions.length;i++){var text=context.config.suggestions[i];if(context.config.highlightInput){var text='<strong>'
	402	++text.substr(0,context.data.prevText.length)
	403	++'</strong>'
	404	++text.substr(context.data.prevText.length,text.length);}
	405	+var $result=$('<div />').addClass('suggestions-result').attr('rel',i).data('text',context.config.suggestions[i]).mouseover(function(e){$.suggestions.highlight(context,$(this).closest('.suggestions-results div'),false);}).appendTo($results);if(typeof context.config.result.render=='function'){context.config.result.render.call($result,text);}else{$result.append($('<div />').css('whiteSpace','nowrap').html(text));var $span=$result.children('span');if($span.outerWidth()>$result.width()&&$span.outerWidth()>expWidth){expWidth=$span.outerWidth();}
402	406	$autoEllipseMe=$autoEllipseMe.add($result);}}
403	407	if(expWidth>context.data.$container.width()){var maxWidth=context.config.maxExpandFactor*context.data.$textbox.width();context.data.$container.width(Math.min(expWidth,maxWidth));}
404	408	$autoEllipseMe.autoEllipsis({hasSpan:true,tooltip:true});}}
405		-break;case'maxRows':context.config[property]=Math.max(1,Math.min(100,value));break;case'delay':context.config[property]=Math.max(0,Math.min(1200,value));break;case'maxExpandFactor':context.config[property]=Math.max(1,value);break;case'submitOnClick':case'positionFromLeft':context.config[property]=value?true:false;break;}},highlight:function(context,result,updateTextbox){var selected=context.data.$container.find('.suggestions-result-current');if(!result.get\|\|selected.get(0)!=result.get(0)){if(result=='prev'){if(selected.is('.suggestions-special')){result=context.data.$container.find('.suggestions-results div:last')}else{result=selected.prev();if(selected.length==0){if(context.data.$container.find('.suggestions-special').html()!=""){result=context.data.$container.find('.suggestions-special');}else{result=context.data.$container.find('.suggestions-results div:last');}}}}else if(result=='next'){if(selected.length==0){result=context.data.$container.find('.suggestions-results div:first');if(result.length==0&&context.data.$container.find('.suggestions-special').html()!=""){result=context.data.$container.find('.suggestions-special');}}else{result=selected.next();if(selected.is('.suggestions-special')){result=$([]);}else if(result.length==0&&context.data.$container.find('.suggestions-special').html()!=""){result=context.data.$container.find('.suggestions-special');}}}
	409	+break;case'maxRows':context.config[property]=Math.max(1,Math.min(100,value));break;case'delay':context.config[property]=Math.max(0,Math.min(1200,value));break;case'maxExpandFactor':context.config[property]=Math.max(1,value);break;case'submitOnClick':case'positionFromLeft':case'highlightInput':context.config[property]=value?true:false;break;}},highlight:function(context,result,updateTextbox){var selected=context.data.$container.find('.suggestions-result-current');if(!result.get\|\|selected.get(0)!=result.get(0)){if(result=='prev'){if(selected.is('.suggestions-special')){result=context.data.$container.find('.suggestions-results div:last')}else{result=selected.prev();if(selected.length==0){if(context.data.$container.find('.suggestions-special').html()!=""){result=context.data.$container.find('.suggestions-special');}else{result=context.data.$container.find('.suggestions-results div:last');}}}}else if(result=='next'){if(selected.length==0){result=context.data.$container.find('.suggestions-results div:first');if(result.length==0&&context.data.$container.find('.suggestions-special').html()!=""){result=context.data.$container.find('.suggestions-special');}}else{result=selected.next();if(selected.is('.suggestions-special')){result=$([]);}else if(result.length==0&&context.data.$container.find('.suggestions-special').html()!=""){result=context.data.$container.find('.suggestions-special');}}}
406	410	selected.removeClass('suggestions-result-current');result.addClass('suggestions-result-current');}
407	411	if(updateTextbox){if(result.length==0){$.suggestions.restore(context);}else{context.data.$textbox.val(result.data('text'));context.data.$textbox.change();}
408	412	context.data.$textbox.trigger('change');}
—	—	@@ -409,7 +413,7 @@
410	414	preventDefault=true;break;case 38:if(wasVisible){$.suggestions.highlight(context,'prev',false);}
411	415	preventDefault=wasVisible;break;case 27:context.data.$container.hide();$.suggestions.restore(context);$.suggestions.cancel(context);context.data.$textbox.trigger('change');preventDefault=wasVisible;break;case 13:context.data.$container.hide();preventDefault=wasVisible;selected=context.data.$container.find('.suggestions-result-current');if(selected.size()==0){$.suggestions.cancel(context);context.config.$region.closest('form').submit();}else if(selected.is('.suggestions-special')){if(typeof context.config.special.select=='function'){context.config.special.select.call(selected,context.data.$textbox);}}else{if(typeof context.config.result.select=='function'){$.suggestions.highlight(context,selected,true);context.config.result.select.call(selected,context.data.$textbox);}else{$.suggestions.highlight(context,selected,true);}}
412	416	break;default:$.suggestions.update(context,true);break;}
413		-if(preventDefault){e.preventDefault();e.stopImmediatePropagation();}}};$.fn.suggestions=function(){var returnValue=null;var args=arguments;$(this).each(function(){var context=$(this).data('suggestions-context');if(typeof context=='undefined'\|\|context==null){context={config:{'fetch':function(){},'cancel':function(){},'special':{},'result':{},'$region':$(this),'suggestions':[],'maxRows':7,'delay':120,'submitOnClick':false,'maxExpandFactor':3,'positionFromLeft':true}};}
	417	+if(preventDefault){e.preventDefault();e.stopImmediatePropagation();}}};$.fn.suggestions=function(){var returnValue=null;var args=arguments;$(this).each(function(){var context=$(this).data('suggestions-context');if(typeof context=='undefined'\|\|context==null){context={config:{'fetch':function(){},'cancel':function(){},'special':{},'result':{},'$region':$(this),'suggestions':[],'maxRows':7,'delay':120,'submitOnClick':false,'maxExpandFactor':3,'positionFromLeft':true,'highlightInput':true}};}
414	418	if(args.length>0){if(typeof args[0]=='object'){for(var key in args[0]){$.suggestions.configure(context,key,args[0][key]);}}else if(typeof args[0]=='string'){if(args.length>1){$.suggestions.configure(context,args[0],args[1]);}else if(returnValue==null){returnValue=(args[0]in context.config?undefined:context.config[args[0]]);}}}
415	419	if(typeof context.data=='undefined'){context.data={'timerID':null,'prevText':null,'visibleResults':0,'mouseDownOn':$([]),'$textbox':$(this)};var newCSS={'top':Math.round(context.data.$textbox.offset().top+context.data.$textbox.outerHeight()),'width':context.data.$textbox.outerWidth(),'display':'none'}
416	420	if(context.config.positionFromLeft){newCSS['left']=context.config.$region.offset().left;newCSS['right']='auto';}else{newCSS['left']='auto';newCSS['right']=$('body').width()-(context.config.$region.offset().left+context.config.$region.outerWidth());}

Follow-up revisions

Revision	Commit summary	Author	Date
r66662	Follow up to r66658. Not sure if this fixes everything but i can not figure o...	adam	22:23, 19 May 2010
r67741	UsabilityInitiative: Temporarily revert r66658, r66662, r66690 so UsabilityIn...	catrope	18:42, 9 June 2010

Comments

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

Broke auto elipsing and auto expansion with the switch to a div.

#Comment by Catrope (talk | contribs) 21:32, 19 May 2010

+							var text = context.config.suggestions[i];
+							if( context.config.highlightInput ) {
+								var text = '<strong>' 
+									+ text.substr( 0, context.data.prevText.length) 
+									+ '</strong>'
+									+  text.substr( context.data.prevText.length, text.length );
+							}

Don't do this, use .text() properly: consider what would happen if text were to contain HTML tags (yes, this is legal).

Also, you shouldn't try to declare the text variable twice.

Why did you use a div, is it because you can't put a strong in a span? In that case, maybe try to put styled spans in the span?

#Comment by Adammiller~mediawikiwiki (talk | contribs) 14:32, 20 July 2010

No longer a problem. Reverted to the previous way it was working ( using spans and .text() ) and approached the problem a different way.

Status & tagging log

19:39, 21 November 2010 Reedy (talk | contribs) changed the status of r66658 [removed: new added: deferred]
14:32, 20 July 2010 Adammiller~mediawikiwiki (talk | contribs) changed the status of r66658 [removed: fixme added: new]
21:27, 19 May 2010 Adammiller~mediawikiwiki (talk | contribs) changed the status of r66658 [removed: new added: fixme]