r67548 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r67547‎ | r67548 | r67549 >
Date:17:49, 7 June 2010
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Changed collapsible navigation to split languages into two portals, one that's always visible for the first 5, and another that's initially collapsed for all others.
Modified paths:
  • /trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/Vector/Modules/CollapsibleNav/CollapsibleNav.i18n.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/Vector/Modules/CollapsibleNav/CollapsibleNav.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/Vector/Vector.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/vector.collapsibleNav.css (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/css/vector.collapsibleNav.css
@@ -37,26 +37,31 @@
3838 text-decoration: underline;
3939 }
4040 #panel.collapsible-nav div.portal div.body {
 41+ background: none !important;
4142 padding-top: 0px;
4243 display: none;
4344 }
 45+#panel.collapsible-nav div.persistent div.body {
 46+ display: block;
 47+}
 48+#panel.collapsible-nav div.first h5 {
 49+ display: none;
 50+}
 51+#panel.collapsible-nav div.persistent h5 {
 52+ background: none !important;
 53+ padding-left: 0.7em;
 54+}
4455 #panel.collapsible-nav div.portal div.body ul li {
4556 padding: 0.25em 0;
4657 }
47 -#panel.collapsible-nav div.portal div.body {
48 - background: none !important;
49 -}
50 -#panel.collapsible-nav #p-navigation {
 58+#panel.collapsible-nav div.first {
5159 background-image: none;
5260 margin-top: 0px;
5361 }
54 -#panel.collapsible-nav #p-navigation div.body {
 62+#panel.collapsible-nav div.persistent div.body {
5563 margin-left: 0.5em;
5664 }
57 -body.rtl #panel.collapsible-nav #p-navigation div.body {
 65+body.rtl #panel.collapsible-nav div.persistent div.body {
5866 margin-left: 0;
5967 margin-right: 0.5em;
60 -}
61 -#panel.collapsible-nav #p-navigation h5 {
62 - display: none;
63 -}
 68+}
\ No newline at end of file
Index: trunk/extensions/UsabilityInitiative/css/combined.css
@@ -104,30 +104,34 @@
105105 text-decoration: underline;
106106 }
107107 #panel.collapsible-nav div.portal div.body {
 108+ background: none !important;
108109 padding-top: 0px;
109110 display: none;
110111 }
 112+#panel.collapsible-nav div.persistent div.body {
 113+ display: block;
 114+}
 115+#panel.collapsible-nav div.first h5 {
 116+ display: none;
 117+}
 118+#panel.collapsible-nav div.persistent h5 {
 119+ background: none !important;
 120+ padding-left: 0.7em;
 121+}
111122 #panel.collapsible-nav div.portal div.body ul li {
112123 padding: 0.25em 0;
113124 }
114 -#panel.collapsible-nav div.portal div.body {
115 - background: none !important;
116 -}
117 -#panel.collapsible-nav #p-navigation {
 125+#panel.collapsible-nav div.first {
118126 background-image: none;
119127 margin-top: 0px;
120128 }
121 -#panel.collapsible-nav #p-navigation div.body {
 129+#panel.collapsible-nav div.persistent div.body {
122130 margin-left: 0.5em;
123131 }
124 -body.rtl #panel.collapsible-nav #p-navigation div.body {
 132+body.rtl #panel.collapsible-nav div.persistent div.body {
125133 margin-left: 0;
126134 margin-right: 0.5em;
127 -}
128 -#panel.collapsible-nav #p-navigation h5 {
129 - display: none;
130 -}
131 -.expandableField {
 135+}.expandableField {
132136 display: block;
133137 float: left;
134138 }
Index: trunk/extensions/UsabilityInitiative/css/combined.min.css
@@ -102,30 +102,34 @@
103103 text-decoration:underline;
104104 }
105105 #panel.collapsible-nav div.portal div.body{
 106+background:none !important;
106107 padding-top:0px;
107108 display:none;
108109 }
 110+#panel.collapsible-nav div.persistent div.body{
 111+display:block;
 112+}
 113+#panel.collapsible-nav div.first h5{
 114+display:none;
 115+}
 116+#panel.collapsible-nav div.persistent h5{
 117+background:none !important;
 118+padding-left:0.7em;
 119+}
109120 #panel.collapsible-nav div.portal div.body ul li{
110121 padding:0.25em 0;
111122 }
112 -#panel.collapsible-nav div.portal div.body{
113 -background:none !important;
114 -}
115 -#panel.collapsible-nav #p-navigation{
 123+#panel.collapsible-nav div.first{
116124 background-image:none;
117125 margin-top:0px;
118126 }
119 -#panel.collapsible-nav #p-navigation div.body{
 127+#panel.collapsible-nav div.persistent div.body{
120128 margin-left:0.5em;
121129 }
122 -body.rtl #panel.collapsible-nav #p-navigation div.body{
 130+body.rtl #panel.collapsible-nav div.persistent div.body{
123131 margin-left:0;
124132 margin-right:0.5em;
125 -}
126 -#panel.collapsible-nav #p-navigation h5{
127 -display:none;
128 -}
129 -.expandableField{
 133+}.expandableField{
130134 display:block;
131135 float:left;
132136 }
Index: trunk/extensions/UsabilityInitiative/Vector/Modules/CollapsibleNav/CollapsibleNav.i18n.php
@@ -13,6 +13,7 @@
1414 */
1515 $messages['en'] = array(
1616 'vector-collapsiblenav-preference' => 'Enable collapsible left navigation menu',
 17+ 'vector-collapsiblenav-more' => 'More languages',
1718 );
1819
1920 /** Gheg Albanian (Gegë)
Index: trunk/extensions/UsabilityInitiative/Vector/Modules/CollapsibleNav/CollapsibleNav.js
@@ -29,15 +29,17 @@
3030 if ( !$j.wikiEditor.isSupported( mod ) ) {
3131 return true;
3232 }
33 -
 33+ // Create a new portal for overflow languages
 34+ $j( '#p-lang' )
 35+ .after( '<div id="p-lang-more" class="portal"><h5></h5><div class="body"><ul></ul></div></div>' )
 36+ .addClass( 'persistent' );
 37+ $j( '#panel > div.portal:first' )
 38+ .addClass( 'first persistent' );
 39+ $j( '#p-lang-more h5' ).text( mw.usability.getMsg( 'vector-collapsiblenav-more' ) );
 40+ // Apply a class to the entire panel to activate styles
3441 $j( '#panel' ).addClass( 'collapsible-nav' );
35 - // Always show the first portal
36 - $j( '#panel > div.portal:first' )
37 - .addClass( 'expanded' )
38 - .find( 'div.body' )
39 - .show();
40 - // Remember which portals to hide and show
41 - $j( '#panel > div.portal:not(:first)' )
 42+ // Use cookie data to restore preferences of what to show and hide
 43+ $j( '#panel > div.portal:not(.persistent)' )
4244 .each( function( i ) {
4345 var state = $j.cookie( 'vector-nav-' + $j(this).attr( 'id' ) );
4446 if ( state == 'true' || ( state == null && i < 1 ) ) {
@@ -53,6 +55,7 @@
5456 $j.cookie( 'vector-nav-' + $j(this).attr( 'id' ), state, { expires: 30, path: '/' } );
5557 }
5658 } );
 59+
5760 // Use the same function for all navigation headings - don't repeat yourself
5861 function toggle( $element ) {
5962 $j.cookie( 'vector-nav-' + $element.parent().attr( 'id' ), $element.parent().is( '.collapsed' ), { expires: 30, path: '/' } );
@@ -63,7 +66,7 @@
6467 .find( 'div.body' )
6568 .slideToggle( 'fast' );
6669 }
67 - var $headings = $j( '#panel > div.portal > h5' );
 70+ var $headings = $j( '#panel > div.portal:not(.persistent) > h5' );
6871 /** Copy-pasted from jquery.wikiEditor.dialogs - :( */
6972 // Find the highest tabindex in use
7073 var maxTI = 0;
@@ -93,4 +96,47 @@
9497 $j(this).blur();
9598 return false;
9699 } );
 100+ // Split the language lists, showing the first 5 in the original portal and all others in the overflow portal
 101+ var limit = 5;
 102+ var count = 0;
 103+ $more = $j( '#p-lang-more ul' );
 104+ $j( '#p-lang li' ).each( function() {
 105+ if ( count++ >= limit ) {
 106+ $j(this).remove().appendTo( $more );
 107+ }
 108+ } );
 109+ /*
 110+ * It may be clever to use something like this to steer which languages get shown by default...
 111+ var wikipediaProjectSizes = {
 112+ 'en': 1, 'fr': 2, 'de': 3, 'es': 4, 'pt': 5, 'it': 6, 'ru': 7, 'ja': 8, 'nl': 9, 'pl': 10, 'zh': 11, 'sv': 12,
 113+ 'ar': 13, 'tr': 14, 'uk': 15, 'fi': 16, 'no': 17, 'ca': 18, 'ro': 19, 'hu': 20, 'ksh': 21, 'id': 22, 'he': 23,
 114+ 'cs': 24, 'vi': 25, 'ko': 26, 'sr': 27, 'fa': 28, 'da': 29, 'eo': 30, 'sk': 31, 'th': 32, 'lt': 33, 'vo': 34,
 115+ 'bg': 35, 'sl': 36, 'hr': 37, 'hi': 38, 'et': 39, 'mk': 40, 'simple': 41, 'new': 42, 'ms': 43, 'nn': 44,
 116+ 'gl': 45, 'el': 46, 'eu': 47, 'ka': 48, 'tl': 49, 'bn': 50, 'lv': 51, 'ml': 52, 'bs': 53, 'te': 54, 'la': 55,
 117+ 'az': 56, 'sh': 57, 'war': 58, 'br': 59, 'is': 60, 'mr': 61, 'be-x-old': 62, 'sq': 63, 'cy': 64, 'lb': 65,
 118+ 'ta': 66, 'zh-classical': 67, 'an': 68, 'jv': 69, 'ht': 70, 'oc': 71, 'bpy': 72, 'ceb': 73, 'ur': 74,
 119+ 'zh-yue': 75, 'pms': 76, 'scn': 77, 'be': 78, 'roa-rup': 79, 'qu': 80, 'af': 81, 'sw': 82, 'nds': 83, 'fy': 84,
 120+ 'lmo': 85, 'wa': 86, 'ku': 87, 'hy': 88, 'su': 89, 'yi': 90, 'io': 91, 'os': 92, 'ga': 93, 'ast': 94, 'nap': 95,
 121+ 'vec': 96, 'gu': 97, 'cv': 98, 'bat-smg': 99, 'kn': 100, 'uz': 101, 'zh-min-nan': 102, 'si': 103, 'als': 104,
 122+ 'yo': 105, 'li': 106, 'gan': 107, 'arz': 108, 'sah': 109, 'tt': 110, 'bar': 111, 'gd': 112, 'tg': 113,
 123+ 'kk': 114, 'pam': 115, 'hsb': 116, 'roa-tara': 117, 'nah': 118, 'mn': 119, 'vls': 120, 'gv': 121, 'mi': 122,
 124+ 'am': 123, 'ia': 124, 'co': 125, 'ne': 126, 'fo': 127, 'nds-nl': 128, 'glk': 129, 'mt': 130, 'ang': 131,
 125+ 'wuu': 132, 'dv': 133, 'km': 134, 'sco': 135, 'bcl': 136, 'mg': 137, 'my': 138, 'diq': 139, 'tk': 140,
 126+ 'szl': 141, 'ug': 142, 'fiu-vro': 143, 'sc': 144, 'rm': 145, 'nrm': 146, 'ps': 147, 'nv': 148, 'hif': 149,
 127+ 'bo': 150, 'se': 151, 'sa': 152, 'pnb': 153, 'map-bms': 154, 'lad': 155, 'lij': 156, 'crh': 157, 'fur': 158,
 128+ 'kw': 159, 'to': 160, 'pa': 161, 'jbo': 162, 'ba': 163, 'ilo': 164, 'csb': 165, 'wo': 166, 'xal': 167,
 129+ 'krc': 168, 'ckb': 169, 'pag': 170, 'ln': 171, 'frp': 172, 'mzn': 173, 'ce': 174, 'nov': 175, 'kv': 176,
 130+ 'eml': 177, 'gn': 178, 'ky': 179, 'pdc': 180, 'lo': 181, 'haw': 182, 'mhr': 183, 'dsb': 184, 'stq': 185,
 131+ 'tpi': 186, 'arc': 187, 'hak': 188, 'ie': 189, 'so': 190, 'bh': 191, 'ext': 192, 'mwl': 193, 'sd': 194,
 132+ 'ig': 195, 'myv': 196, 'ay': 197, 'iu': 198, 'na': 199, 'cu': 200, 'pi': 201, 'kl': 202, 'ty': 203, 'lbe': 204,
 133+ 'ab': 205, 'got': 206, 'sm': 207, 'as': 208, 'mo': 209, 'ee': 210, 'zea': 211, 'av': 212, 'ace': 213, 'kg': 214,
 134+ 'bm': 215, 'cdo': 216, 'cbk-zam': 217, 'kab': 218, 'om': 219, 'chr': 220, 'pap': 221, 'udm': 222, 'ks': 223,
 135+ 'zu': 224, 'rmy': 225, 'cr': 226, 'ch': 227, 'st': 228, 'ik': 229, 'mdf': 230, 'kaa': 231, 'aa': 232, 'fj': 233,
 136+ 'srn': 234, 'tet': 235, 'or': 236, 'pnt': 237, 'bug': 238, 'ss': 239, 'ts': 240, 'pcd': 241, 'pih': 242,
 137+ 'za': 243, 'sg': 244, 'lg': 245, 'bxr': 246, 'xh': 247, 'ak': 248, 'ha': 249, 'bi': 250, 've': 251, 'tn': 252,
 138+ 'ff': 253, 'dz': 254, 'ti': 255, 'ki': 256, 'ny': 257, 'rw': 258, 'chy': 259, 'tw': 260, 'sn': 261, 'tum': 262,
 139+ 'ng': 263, 'rn': 264, 'mh': 265, 'ii': 266, 'cho': 267, 'hz': 268, 'kr': 269, 'ho': 270, 'mus': 271, 'kj': 272
 140+ };
 141+ */
 142+ */
97143 } );
Index: trunk/extensions/UsabilityInitiative/Vector/Vector.hooks.php
@@ -12,7 +12,7 @@
1313
1414 static $scripts = array(
1515 'raw' => array(
16 - array( 'src' => 'Modules/CollapsibleNav/CollapsibleNav.js', 'version' => 15 ),
 16+ array( 'src' => 'Modules/CollapsibleNav/CollapsibleNav.js', 'version' => 16 ),
1717 array( 'src' => 'Modules/CollapsibleTabs/CollapsibleTabs.js', 'version' => 8 ),
1818 array( 'src' => 'Modules/ExpandableSearch/ExpandableSearch.js', 'version' => 2 ),
1919 array( 'src' => 'Modules/EditWarning/EditWarning.js', 'version' => 8 ),
@@ -20,10 +20,10 @@
2121 array( 'src' => 'Modules/SimpleSearch/SimpleSearch.js', 'version' => 15 ),
2222 ),
2323 'combined' => array(
24 - array( 'src' => 'Vector.combined.js', 'version' => 37 ),
 24+ array( 'src' => 'Vector.combined.js', 'version' => 38 ),
2525 ),
2626 'minified' => array(
27 - array( 'src' => 'Vector.combined.min.js', 'version' => 37 ),
 27+ array( 'src' => 'Vector.combined.min.js', 'version' => 38 ),
2828 ),
2929 );
3030 static $modules = array(
@@ -39,6 +39,9 @@
4040 ),
4141 ),
4242 ),
 43+ 'messages' => array(
 44+ 'vector-collapsiblenav-more',
 45+ ),
4346 ),
4447 'collapsibletabs' => array(
4548 ),
Index: trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php
@@ -19,7 +19,7 @@
2020 'base_sets' => array(
2121 'raw' => array(
2222 array( 'src' => 'css/suggestions.css', 'version' => 14 ),
23 - array( 'src' => 'css/vector.collapsibleNav.css', 'version' => 9 ),
 23+ array( 'src' => 'css/vector.collapsibleNav.css', 'version' => 10 ),
2424 array( 'src' => 'css/vector.expandableSearch.css', 'version' => 2 ),
2525 array( 'src' => 'css/vector.footerCleanup.css', 'version' => 2 ),
2626 array( 'src' => 'css/wikiEditor.css', 'version' => 14 ),
@@ -30,11 +30,11 @@
3131 array( 'src' => 'css/vector/jquery-ui-1.7.2.css', 'version' => '1.7.2y' ),
3232 ),
3333 'combined' => array(
34 - array( 'src' => 'css/combined.css', 'version' => 96 ),
 34+ array( 'src' => 'css/combined.css', 'version' => 98 ),
3535 array( 'src' => 'css/vector/jquery-ui-1.7.2.css', 'version' => '1.7.2y' ),
3636 ),
3737 'minified' => array(
38 - array( 'src' => 'css/combined.min.css', 'version' => 96 ),
 38+ array( 'src' => 'css/combined.min.css', 'version' => 98 ),
3939 array( 'src' => 'css/vector/jquery-ui-1.7.2.css', 'version' => '1.7.2y' ),
4040 ),
4141 )

Follow-up revisions

RevisionCommit summaryAuthorDate
r67566Fixed issues in r67548 to do with declaring vars nicely and using appendTo ra...tparscal21:09, 7 June 2010
r67728Declare some more vars, fix for r67548catrope15:28, 9 June 2010

Comments

#Comment by Catrope (talk | contribs)   18:14, 7 June 2010
+	var limit = 5;
+	var count = 0;
+	$more = $j( '#p-lang-more ul' );

You should use var $more here as well to prevent exporting $more to the global scope.

+			$j(this).remove().appendTo( $more );

It's better to just do .appendTo( $more ), without the .remove(). The former will deattach the element from its current parent, if any, before moving it to its new home, whereas the latter goes through some trouble to kill event handlers and do all sorts of other things to thoroughly eradicate all traces of the element ever having existed. In my experience with profiling JS, .remove() takes a lot of time and I've tried to remove its uses where avoidable.

#Comment by Trevor Parscal (WMF) (talk | contribs)   21:10, 7 June 2010

resolved in r67566

#Comment by Adammiller~mediawikiwiki (talk | contribs)   18:53, 7 June 2010
+		'ng': 263, 'rn': 264, 'mh': 265, 'ii': 266, 'cho': 267, 'hz': 268, 'kr': 269, 'ho': 270, 'mus': 271, 'kj': 272
+	};
+	*/
+	 */
} );

You've got an unclosed comment block in CollapsibleNav.js which is throwing errors.

#Comment by Virgolette (talk | contribs)   19:00, 7 June 2010

"More languages" will show only 1 or 2 links when there are 6 or 7 interwikis in total. That's terrible! It would be better something like this: - with 10 languages, show them all - with 11 languages, show the first 5 and hide the other 6 in "More languages"

#Comment by Trevor Parscal (WMF) (talk | contribs)   19:42, 7 June 2010

Resolved in r67559.

Status & tagging log