r50207 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r50206‎ \| r50207 \| r50208 >
Date:	23:07, 4 May 2009
Author:	rainman
Status:	resolved (Comments)
Tags:
Comment:	Redesign of the "new search UI" per Trevor's design plans from usability wiki: * simplify everything, move search box to top * move header below it and make it google-style * move styles from monobook into shared since people didn't really customize it * remove some old unused stuff like default+project namespaces and such Things to do still: * smarter detection of prefix searches * javascript to link to correct link when typing in a query, but clicking on one of the search headers * prev/next still uses MW default navigation, should be replaced with tango icons? * test a bit more with default search backend (i.e. mysql)
Modified paths:	/trunk/phase3/includes/DefaultSettings.php (modified) (history) /trunk/phase3/includes/SearchEngine.php (modified) (history) /trunk/phase3/includes/specials/SpecialSearch.php (modified) (history) /trunk/phase3/languages/messages/MessagesEn.php (modified) (history) /trunk/phase3/maintenance/language/messages.inc (modified) (history) /trunk/phase3/skins/common/shared.css (modified) (history) /trunk/phase3/skins/monobook/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/language/messages.inc
—	—	@@ -812,6 +812,7 @@
813	813	'showingresults',
814	814	'showingresultsnum',
815	815	'showingresultstotal',
	816	+ 'showingresultsheader',
816	817	'nonefound',
817	818	'search-nonefound',
818	819	'powersearch',
Index: trunk/phase3/skins/monobook/main.css
—	—	@@ -1542,23 +1542,6 @@
1543	1543	font-size: 90%;
1544	1544	}
1545	1545
1546		~~-/** Special:Search stuff */~~
1547		~~-div#mw-search-interwiki-caption {~~
1548		~~- text-align: center;~~
1549		~~- font-weight: bold;~~
1550		~~- font-size: 95%;~~
1551		-}
1552		-
1553		~~-.mw-search-interwiki-project {~~
1554		~~- font-size: 97%;~~
1555		~~- text-align: left;~~
1556		~~- padding-left: 0.2em;~~
1557		~~- padding-right: 0.15em;~~
1558		~~- padding-bottom: 0.2em;~~
1559		~~- padding-top: 0.15em;~~
1560		~~- background: #cae8ff;~~
1561		-}
1562		-
1563	1546	/* God-damned hack for the crappy layout */
1564	1547	.os-suggest {
1565	1548	font-size: 127%;
Index: trunk/phase3/skins/common/shared.css
—	—	@@ -102,19 +102,55 @@
103	103	}
104	104
105	105	/* Search results */
	106	+.searchresults {
	107	+}
	108	+
	109	+.searchresults p {
	110	+ margin-left: 0.4em;
	111	+ margin-top: 1em;
	112	+ margin-bottom: 1.2em;
	113	+}
	114	+
106	115	div.searchresult {
107	116	font-size: 95%;
108		~~- width:38em;~~
	117	+ width:38em;
109	118	}
	119	+
	120	+.mw-search-results {
	121	+ margin-left: 0.4em;
	122	+}
	123	+
110	124	.mw-search-results li {
111	125	padding-bottom: 1em;
112	126	list-style:none;
113	127	}
	128	+.mw-search-results li a {
	129	+ font-size: 108%;
	130	+}
	131	+
114	132	.mw-search-result-data {
115	133	color: green;
116	134	font-size: 97%;
117	135	}
118	136
	137	+.mw-search-formheader {
	138	+ background-color:#ECECEC;
	139	+ margin:1em 0pt;
	140	+ padding:0.02em ;
	141	+ border-top:1px solid #BBBBBB;
	142	+}
	143	+
	144	+.mw-search-formheader div {
	145	+ float:left;
	146	+ padding:4px 0.5em 0px 4px;
	147	+}
	148	+
	149	+.mw-search-formheader p {
	150	+ margin:0.1em 0pt;
	151	+ padding:0.2em 0.4em 0.2em 0.2em;
	152	+ text-align:right;
	153	+}
	154	+
119	155	td#mw-search-menu {
120	156	padding-left:6em;
121	157	font-size:85%;
—	—	@@ -138,6 +174,24 @@
139	175	font-size: 90%;
140	176	}
141	177
	178	+div#mw-search-interwiki-caption {
	179	+ text-align: center;
	180	+ font-weight: bold;
	181	+ font-size: 95%;
	182	+}
	183	+
	184	+.mw-search-interwiki-project {
	185	+ font-size: 97%;
	186	+ text-align: left;
	187	+ padding-left: 0.2em;
	188	+ padding-right: 0.15em;
	189	+ padding-bottom: 0.2em;
	190	+ padding-top: 0.15em;
	191	+ background-color:#ececec;
	192	+ border-top:1px solid #BBBBBB;
	193	+}
	194	+
	195	+
142	196	span.searchalttitle {
143	197	font-size: 95%;
144	198	}
—	—	@@ -158,13 +212,6 @@
159	213	font-weight: bold;
160	214	}
161	215
162		~~-div.searchresults {~~
163		~~- border:1px solid darkblue;~~
164		~~- padding-top: 10px;~~
165		~~- padding-bottom: 10px;~~
166		~~- padding-left: 20px;~~
167		~~- padding-right: 20px;~~
168		-}
169	216
170	217	/*
171	218	* UserRights stuff
Index: trunk/phase3/includes/SearchEngine.php
—	—	@@ -292,31 +292,17 @@
293	293	}
294	294
295	295	/**
296		~~- * An array of "project" namespaces indexes typically searched~~
297		~~- * by logged-in users~~
	296	+ * Return the help namespaces to be shown on Special:Search
298	297	*
299	298	* @return Array
300	299	*/
301		~~- public static function projectNamespaces() {~~
302		~~- global $wgNamespacesToBeSearchedDefault, $wgNamespacesToBeSearchedProject;~~
	300	+ public static function helpNamespaces() {
	301	+ global $wgNamespacesToBeSearchedHelp;
303	302
304		~~- return array_keys( $wgNamespacesToBeSearchedProject, true );~~
	303	+ return array_keys( $wgNamespacesToBeSearchedHelp, true );
305	304	}
306	305
307	306	/**
308		~~- * An array of "project" namespaces indexes typically searched~~
309		~~- * by logged-in users in addition to the default namespaces~~
310		- *
311		~~- * @return Array~~
312		~~- */~~
313		~~- public static function defaultAndProjectNamespaces() {~~
314		~~- global $wgNamespacesToBeSearchedDefault, $wgNamespacesToBeSearchedProject;~~
315		-
316		~~- return array_keys( $wgNamespacesToBeSearchedDefault +~~
317		~~- $wgNamespacesToBeSearchedProject, true);~~
318		~~- }~~
319		-
320		- /**
321	307	* Return a 'cleaned up' search string
322	308	*
323	309	* @param $text String
Index: trunk/phase3/includes/DefaultSettings.php
—	—	@@ -1976,16 +1976,14 @@
1977	1977	);
1978	1978
1979	1979	/**
1980		~~- * Additional namespaces to those in $wgNamespacesToBeSearchedDefault that~~
1981		~~- * will be added to default search for "project" page inclusive searches~~
	1980	+ * Namespaces to be searched when user clicks the "Help" tab
	1981	+ * on Special:Search
1982	1982	*
1983	1983	* Same format as $wgNamespacesToBeSearchedDefault
1984	1984	*/
1985		~~-$wgNamespacesToBeSearchedProject = array(~~
1986		~~- NS_USER => true,~~
	1985	+$wgNamespacesToBeSearchedHelp = array(
1987	1986	NS_PROJECT => true,
1988	1987	NS_HELP => true,
1989		~~- NS_CATEGORY => true,~~
1990	1988	);
1991	1989
1992	1990	$wgUseOldSearchUI = true; // temp testing variable
Index: trunk/phase3/includes/specials/SpecialSearch.php
—	—	@@ -206,14 +206,6 @@
207	207	return;
208	208	}
209	209
210		~~- // show direct page/create link~~
211		~~- if( !is_null($t) ) {~~
212		~~- if( !$t->exists() ) {~~
213		~~- $wgOut->addWikiMsg( 'searchmenu-new', wfEscapeWikiText( $t->getPrefixedText() ) );~~
214		~~- } else {~~
215		~~- $wgOut->addWikiMsg( 'searchmenu-exists', wfEscapeWikiText( $t->getPrefixedText() ) );~~
216		~~- }~~
217		~~- }~~
218	210
219	211	// Get number of results
220	212	$titleMatchesSQL = $titleMatches ? $titleMatches->numRows() : 0;
—	—	@@ -228,16 +220,17 @@
229	221	$totalRes = $numTitleMatches + $numTextMatches;
230	222
231	223	// show number of results and current offset
232		~~- if( $numSQL > 0 ) {~~
233		~~- if( $numSQL > 0 ) {~~
234		~~- $top = wfMsgExt('showingresultstotal', array( 'parseinline' ),~~
235		~~- $this->offset+1, $this->offset+$numSQL, $totalRes, $numSQL );~~
236		~~- } elseif( $numSQL >= $this->limit ) {~~
237		~~- $top = wfShowingResults( $this->offset, $this->limit );~~
	224	+ $wgOut->addHTML( $this->formHeader($term, $numSQL, $totalRes));
	225	+
	226	+ $wgOut->addHtml( "<div class='searchresults'>" );
	227	+
	228	+ // show direct page/create link
	229	+ if( !is_null($t) ) {
	230	+ if( !$t->exists() ) {
	231	+ $wgOut->addWikiMsg( 'searchmenu-new', wfEscapeWikiText( $t->getPrefixedText() ) );
238	232	} else {
239		~~- $top = wfShowingResultsNum( $this->offset, $this->limit, $numSQL );~~
	233	+ $wgOut->addWikiMsg( 'searchmenu-exists', wfEscapeWikiText( $t->getPrefixedText() ) );
240	234	}
241		~~- $wgOut->addHTML( "<p class='mw-search-numberresults'>{$top}</p>\n" );~~
242	235	}
243	236
244	237	// prev/next links
—	—	@@ -247,13 +240,13 @@
248	241	wfArrayToCGI( $this->powerSearchOptions(), array( 'search' => $term ) ),
249	242	max( $titleMatchesSQL, $textMatchesSQL ) < $this->limit
250	243	);
251		~~- $wgOut->addHTML( "<p class='mw-search-pager-top'>{$prevnext}</p>\n" );~~
	244	+ //$wgOut->addHTML( "<p class='mw-search-pager-top'>{$prevnext}</p>\n" );
252	245	wfRunHooks( 'SpecialSearchResults', array( $term, &$titleMatches, &$textMatches ) );
253	246	} else {
254	247	wfRunHooks( 'SpecialSearchNoResults', array( $term ) );
255		~~- }~~
	248	+ }
256	249
257		~~- $wgOut->addHtml( "<div class='searchresults'>" );~~
	250	+
258	251	if( $titleMatches ) {
259	252	if( $numTitleMatches > 0 ) {
260	253	$wgOut->wrapWikiMsg( "==$1==\n", 'titlematches' );
—	—	@@ -268,7 +261,7 @@
269	262	$wgOut->wrapWikiMsg( "==$1==\n", 'textmatches' );
270	263	} elseif( $totalRes == 0 ) {
271	264	# Don't show the 'no text matches' if we received title matches
272		~~- $wgOut->wrapWikiMsg( "==$1==\n", 'notextmatches' );~~
	265	+ # $wgOut->wrapWikiMsg( "==$1==\n", 'notextmatches' );
273	266	}
274	267	// show interwiki results if any
275	268	if( $textMatches->hasInterwikiResults() ) {
—	—	@@ -304,14 +297,14 @@
305	298	$nsAllSet = array_keys( SearchEngine::searchableNamespaces() );
306	299	if( $this->searchAdvanced )
307	300	$this->active = 'advanced';
308		~~- else if( $this->namespaces === NS_FILE \|\| $this->startsWithImage( $term ) )~~
	301	+ else if( $this->namespaces === array(NS_FILE) \|\| $this->startsWithImage( $term ) )
309	302	$this->active = 'images';
310		~~- elseif( $this->namespaces === $nsAllSet )~~
	303	+ elseif( $this->namespaces === $nsAllSet \|\| $this->startsWithAll( $term ) )
311	304	$this->active = 'all';
312	305	elseif( $this->namespaces === SearchEngine::defaultNamespaces() )
313	306	$this->active = 'default';
314		~~- elseif( $this->namespaces === SearchEngine::projectNamespaces() )~~
315		~~- $this->active = 'project';~~
	307	+ elseif( $this->namespaces === SearchEngine::helpNamespaces() )
	308	+ $this->active = 'help';
316	309	else
317	310	$this->active = 'advanced';
318	311	# Should advanced UI be used?
—	—	@@ -648,7 +641,7 @@
649	642	} */
650	643	return Xml::openElement( 'fieldset', array('id' => 'mw-searchoptions','style' => 'margin:0em;') ) .
651	644	Xml::element( 'legend', null, wfMsg('powersearch-legend') ) .
652		~~- $this->formHeader($term) . $out . $this->didYouMeanHtml .~~
	645	+ $out . $this->didYouMeanHtml .
653	646	Xml::closeElement( 'fieldset' );
654	647	}
655	648
—	—	@@ -670,11 +663,12 @@
671	664	"</script>";
672	665	}
673	666
674		~~- protected function formHeader( $term ) {~~
	667	+ protected function formHeader( $term, $resultsShown, $totalRes ) {
675	668	global $wgContLang, $wgCanonicalNamespaceNames, $wgLang;
676		-
	669	+
677	670	$sep = '   ';
678		~~- $out = Xml::openElement('div', array( 'style' => 'padding-bottom:0.5em;' ) );~~
	671	+ $out = Xml::openElement('div', array( 'class' => 'mw-search-formheader' ) );
	672	+ $out .= Xml::openElement('div');
679	673
680	674	$bareterm = $term;
681	675	if( $this->startsWithImage( $term ) )
—	—	@@ -699,19 +693,18 @@
700	694	if( $this->active == 'images' ) {
701	695	$out .= Xml::element( 'strong', array( 'title'=>$tt ), $m );
702	696	} else {
703		~~- $imageTextForm = $wgContLang->getFormattedNsText(NS_FILE).':'.$bareterm;~~
704		~~- $out .= $this->makeSearchLink( $imageTextForm, array( NS_FILE ) , $m, $tt );~~
	697	+ $out .= $this->makeSearchLink( $bareterm, array( NS_FILE ) , $m, $tt );
705	698	}
706	699	$out .= $sep;
707	700
708	701	$m = wfMsg( 'searchprofile-project' );
709	702	$tt = wfMsg( 'searchprofile-project-tooltip',
710		~~- $wgLang->commaList( SearchEngine::namespacesAsText( SearchEngine::projectNamespaces() ) ) );~~
	703	+ $wgLang->commaList( SearchEngine::namespacesAsText( SearchEngine::helpNamespaces() ) ) );
711	704	$tt = Sanitizer::decodeCharReferences( $tt ); // need to allow entities
712		~~- if( $this->active == 'project' ) {~~
	705	+ if( $this->active == 'help' ) {
713	706	$out .= Xml::element( 'strong', array( 'title'=>$tt ), $m );
714	707	} else {
715		~~- $out .= $this->makeSearchLink( $bareterm, SearchEngine::projectNamespaces(), $m, $tt );~~
	708	+ $out .= $this->makeSearchLink( $bareterm, SearchEngine::helpNamespaces(), $m, $tt );
716	709	}
717	710	$out .= $sep;
718	711
—	—	@@ -732,6 +725,16 @@
733	726	$out .= $this->makeSearchLink( $bareterm, $this->namespaces, $m, $tt, array( 'advanced' => '1' ) );
734	727	}
735	728	$out .= Xml::closeElement('div') ;
	729	+
	730	+ if( $totalRes > 0){
	731	+ $countHtml = wfMsgExt('showingresultsheader', array( 'parseinline' ),
	732	+ $this->offset+1, $this->offset+$resultsShown, $totalRes, $term, $resultsShown );
	733	+ $out .= "<p>{$countHtml}</p>\n" ;
	734	+ } else{
	735	+ $out .= "<p> </p>\n";
	736	+ }
	737	+
	738	+ $out .= Xml::closeElement('div') ;
736	739
737	740	return $out;
738	741	}
—	—	@@ -762,17 +765,14 @@
763	766	$out .= Xml::input( 'search', 50, $term, array( 'type' => 'text', 'id' => 'searchText' ) ) . "\n";
764	767	$out .= Xml::hidden( 'fulltext', 'Search' );
765	768	$out .= Xml::submitButton( wfMsg( 'searchbutton' ) );
766		~~- $out .= ' (' . wfMsgExt('searchmenu-help',array('parseinline') ) . ')';~~
	769	+ //$out .= ' (' . wfMsgExt('searchmenu-help',array('parseinline') ) . ')';
767	770	$out .= Xml::closeElement( 'form' );
768	771	// Add prefix link for single-namespace searches
769	772	$t = Title::newFromText( $term );
770	773	/*if( $t != null && count($this->namespaces) === 1 ) {
771	774	$out .= wfMsgExt( 'searchmenu-prefix', array('parseinline'), $term );
772	775	}*/
773		~~- return Xml::openElement( 'fieldset', array('id' => 'mw-searchoptions','style' => 'margin:0em;') ) .~~
774		~~- Xml::element( 'legend', null, wfMsg('searchmenu-legend') ) .~~
775		~~- $this->formHeader($term) . $out . $this->didYouMeanHtml .~~
776		~~- Xml::closeElement( 'fieldset' );~~
	776	+ return $out . $this->didYouMeanHtml;
777	777	}
778	778
779	779	/** Make a search link with some target namespaces */
—	—	@@ -801,7 +801,19 @@
802	802	}
803	803	return false;
804	804	}
	805	+
	806	+ /** Check if query starts with all: prefix */
	807	+ protected function startsWithAll( $term ) {
805	808
	809	+ $allkeyword = wfMsgForContent('searchall');
	810	+
	811	+ $p = explode( ':', $term );
	812	+ if( count( $p ) > 1 ) {
	813	+ return $p[0] == $allkeyword;
	814	+ }
	815	+ return false;
	816	+ }
	817	+
806	818	protected function namespaceTables( $namespaces, $rowsPerTable = 3 ) {
807	819	global $wgContLang;
808	820	// Group namespaces into rows according to subject.
Index: trunk/phase3/languages/messages/MessagesEn.php
—	—	@@ -1450,9 +1450,9 @@
1451	1451	'searchhelp-url' => 'Help:Contents',
1452	1452	'searchmenu-prefix' => '[[Special:PrefixIndex/$1\|Browse pages with this prefix]]',
1453	1453	'searchmenu-help' => '[[{{MediaWiki:Searchhelp-url}}\|{{int:help}}]]?', # do not translate or duplicate this message to other languages
1454		~~-'searchprofile-articles' => 'Content pages',~~
1455		~~-'searchprofile-project' => 'Project pages',~~
1456		~~-'searchprofile-images' => 'Files',~~
	1454	+'searchprofile-articles' => 'Articles',
	1455	+'searchprofile-project' => 'Help',
	1456	+'searchprofile-images' => 'Multimedia',
1457	1457	'searchprofile-everything' => 'Everything',
1458	1458	'searchprofile-advanced' => 'Advanced',
1459	1459	'searchprofile-articles-tooltip' => 'Search in $1',
—	—	@@ -1478,6 +1478,7 @@
1479	1479	'showingresults' => "Showing below up to {{PLURAL:$1\|'''1''' result\|'''$1''' results}} starting with #'''$2'''.",
1480	1480	'showingresultsnum' => "Showing below {{PLURAL:$3\|'''1''' result\|'''$3''' results}} starting with #'''$2'''.",
1481	1481	'showingresultstotal' => "Showing below {{PLURAL:$4\|result '''$1''' of '''$3'''\|results '''$1 - $2''' of '''$3'''}}",
	1482	+'showingresultsheader' => "{{PLURAL:$5\|Result '''$1''' of '''$3'''\|Results '''$1 - $2''' of '''$3'''}} for '''$4'''",
1482	1483	'nonefound' => "'''Note''': Only some namespaces are searched by default.
1483	1484	Try prefixing your query with ''all:'' to search all content (including talk pages, templates, etc), or use the desired namespace as prefix.",
1484	1485	'search-nonefound' => 'There were no results matching the query.',

Follow-up revisions

Revision	Commit summary	Author	Date
r50282	Follow-up for r50207: mysql won't estimate total number of hits, so don't sho...	rainman	21:05, 6 May 2009
r51228	More work on new search UI (follow-up to r50207):...	rainman	13:22, 31 May 2009
r51246	Lastest followup to r50207: show create article link only for content and eve...	rainman	17:10, 31 May 2009

Comments

#Comment by Brion VIBBER (talk | contribs) 21:44, 5 May 2009

Looks great!

One problem I noticed, using the default search backend... the total result count is wrong, giving the number of hits returned within our search limit -- eg: "Results 21 - 24 of 4 for the" for the second page of results for the word "the".

#Comment by Aude (talk | contribs) 16:47, 6 May 2009

The new search interface looks good. Though, one thing that has long bothered me about the search page is having no toggle for mass selecting or deselecting all the namespace checkboxes when doing an "advanced" search. I know you can do that with the shift key, but it doesn't work ideally for me, so I propose adding toggle buttons to do that for the advanced search page.

Links to patch and live demo:

User:Aude/Search_page (I can't figure out how to do external links here, they are formatting funny when I preview my comments)

I have commit rights, but am a new developer so want to make sure there is consensus for my proposed change. If so, tweaks to my code might be necessary or ideas to better implement the buttons (design-wise).

#Comment by Aude (talk | contribs) 16:49, 6 May 2009

If you want to try the advanced search on my demo, good keywords to try would be "art", "gallery", "museum", etc.

#Comment by Rainman (talk | contribs) 22:48, 6 May 2009

The result count issue fixed in r50282.

As for javascript toggle, yes, looks like a good idea, I'll add it to the rest of the JS I have planned.

#Comment by Tim Starling (talk | contribs) 13:52, 26 May 2009

Please remove the commented-out code.

Status & tagging log

09:29, 4 June 2009 Tim Starling (talk | contribs) changed the status of r50207 [removed: fixme added: resolved]
21:44, 5 May 2009 Brion VIBBER (talk | contribs) changed the status of r50207 [removed: new added: fixme]