r113254 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r113253‎ | r113254 | r113255 >
Date:17:31, 7 March 2012
Author:netbrain
Status:deferred (Comments)
Tags:
Comment:
added i18n
removed unused functions and tests
fixed failing tests
minor change to javascript
minor change to css
Modified paths:
  • /trunk/extensions/SideBarMenu/SideBarMenu.body.php (deleted) (history)
  • /trunk/extensions/SideBarMenu/SideBarMenu.hooks.php (modified) (history)
  • /trunk/extensions/SideBarMenu/SideBarMenu.i18n.php (modified) (history)
  • /trunk/extensions/SideBarMenu/SideBarMenu.php (modified) (history)
  • /trunk/extensions/SideBarMenu/css/ext.sidebarmenu.css (modified) (history)
  • /trunk/extensions/SideBarMenu/includes/MenuParser.php (modified) (history)
  • /trunk/extensions/SideBarMenu/js/ext.sidebarmenu.js (modified) (history)
  • /trunk/extensions/SideBarMenu/test/MenuItemTest.php (modified) (history)
  • /trunk/extensions/SideBarMenu/test/MenuParserTest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SideBarMenu/test/MenuParserTest.php
——@@ -15,16 +15,6 @@
1616 $this->assertTrue(MenuParser::isValidInput("+MenuItem"));
1717 }
1818
19 - public function testIsRoot(){
20 - $isRoot = MenuParser::isRoot("MenuItem");
21 - $this->assertTrue($isRoot);
22 - }
23 -
24 - public function testIsNotRoot(){
25 - $isRoot = MenuParser::isRoot("*MenuItem");
26 - $this->assertFalse($isRoot);
27 - }
28 -
2919 public function testGetLevelWhenNull(){
3020 $this->assertEquals(0,MenuParser::getLevel(null));
3121 }
Index: trunk/extensions/SideBarMenu/test/MenuItemTest.php
——@@ -69,7 +69,7 @@
7070 $menuItemChild->setText("MenuItem1");
7171 $this->menuItem->addChild($menuItemChild);
7272 $html = $this->menuItem->toHTML();
73 - $this->assertEquals('<ul class="sidebar-menu sidebar-menu-0"><li class="sidebar-menu-item sidebar-menu-item-1 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem1</div></li></ul>',$html);
 73+ $this->assertEquals('<ul class="sidebar-menu sidebar-menu-0"><li class="sidebar-menu-item sidebar-menu-item-1"><div class="sidebar-menu-item-text-container"><span class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem1</span></div></li></ul>',$html);
7474 }
7575
7676 public function testToHTMLOnSeveralMenuItems(){
——@@ -82,7 +82,7 @@
8383 $this->menuItem->addChild($menuItemChild2);
8484
8585 $html = $this->menuItem->toHTML();
86 - $this->assertEquals('<ul class="sidebar-menu sidebar-menu-0"><li class="sidebar-menu-item sidebar-menu-item-1 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem1</div></li><li class="sidebar-menu-item sidebar-menu-item-1 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem2</div></li></ul>',$html);
 86+ $this->assertEquals('<ul class="sidebar-menu sidebar-menu-0"><li class="sidebar-menu-item sidebar-menu-item-1"><div class="sidebar-menu-item-text-container"><span class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem1</span></div></li><li class="sidebar-menu-item sidebar-menu-item-1"><div class="sidebar-menu-item-text-container"><span class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem2</span></div></li></ul>',$html);
8787 }
8888
8989 public function testToHTMLOnSeveralMenuItemsWithSublevels(){
——@@ -99,6 +99,6 @@
100100 $subLevel1->setParent($menuItemChild2);
101101
102102 $html = $this->menuItem->toHTML();
103 - $this->assertEquals('<ul class="sidebar-menu sidebar-menu-0"><li class="sidebar-menu-item sidebar-menu-item-1 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem1</div></li><li class="sidebar-menu-item sidebar-menu-item-1 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem2</div><ul class="sidebar-menu sidebar-menu-1"><li class="sidebar-menu-item sidebar-menu-item-2 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text sidebar-menu-item-text-2">SubMenuItem1</div></li></ul></li></ul>',$html);
 103+ $this->assertEquals('<ul class="sidebar-menu sidebar-menu-0"><li class="sidebar-menu-item sidebar-menu-item-1"><div class="sidebar-menu-item-text-container"><span class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem1</span></div></li><li class="sidebar-menu-item sidebar-menu-item-1 sidebar-menu-item-collapsed"><div class="sidebar-menu-item-text-container"><span class="sidebar-menu-item-text sidebar-menu-item-text-1">MenuItem2</span><span class="sidebar-menu-item-controls"></span></div><ul class="sidebar-menu sidebar-menu-1"><li class="sidebar-menu-item sidebar-menu-item-2"><div class="sidebar-menu-item-text-container"><span class="sidebar-menu-item-text sidebar-menu-item-text-2">SubMenuItem1</span></div></li></ul></li></ul>',$html);
104104 }
105105 }
Index: trunk/extensions/SideBarMenu/SideBarMenu.i18n.php
——@@ -0,0 +1,8 @@
 2+<?php
 3+$messages = array();
 4+$messages['en'] = array(
 5+ 'controls.show' => '[show]',
 6+ 'controls.hide' => '[hide]',
 7+ 'parser.input-error' => 'FATAL ERROR: parser returned with error: $1',
 8+ 'parser.syntax-error' => 'Could not parse "$1", make sure the syntax is correct.'
 9+);
\ No newline at end of file
Index: trunk/extensions/SideBarMenu/SideBarMenu.php
——@@ -35,6 +35,9 @@
3636 $wgAutoloadClasses['MenuParser'] = $wgMyExtensionIncludes . '/MenuParser.php';
3737 $wgAutoloadClasses['MenuItem'] = $wgMyExtensionIncludes . '/MenuItem.php';
3838
 39+//i18n
 40+$wgExtensionMessagesFiles['SideBarMenu'] = dirname( __FILE__ ) . '/SideBarMenu.i18n.php';
 41+
3942 //Resources
4043 $wgResourceModules['ext.sidebarmenu.core'] = array(
4144 'scripts' => array(
——@@ -44,7 +47,9 @@
4548 'css/ext.sidebarmenu.css'
4649 ),
4750 'dependencies' => array (
48 - 'jquery.ui.core'
 51+ 'jquery.ui.core',
 52+ 'jquery.effects.core',
 53+
4954 ),
5055 'group' => 'ext.sidebarmenu',
5156 'localBasePath' => dirname( __FILE__ ),
——@@ -52,5 +57,5 @@
5358 );
5459
5560 //default settings
56 -$wgSideBarMenuConfigShowHTML = '[show]';
57 -$wgSideBarMenuConfigHideHTML = '[hide]';
\ No newline at end of file
 61+$wgSideBarMenuConfigShowHTML = null;
 62+$wgSideBarMenuConfigHideHTML = null;
\ No newline at end of file
Index: trunk/extensions/SideBarMenu/SideBarMenu.hooks.php
——@@ -17,7 +17,7 @@
1818 return $parser->recursiveTagParse($menuHTML,$frame);
1919 }catch(Exception $x){
2020 wfDebug("An error occured during parsing of: '$input' caught exception: $x");
21 - return "FATAL ERROR: Could not parse the following input:</br><pre>$input</pre>";
 21+ return wfMsg('parser.input-error',$x->getMessage());
2222 }
2323 }
2424
——@@ -40,8 +40,8 @@
4141
4242 public static function javascriptConfigVars(&$vars){
4343 global $wgSideBarMenuConfigShowHTML,$wgSideBarMenuConfigHideHTML;
44 - $vars['wgSideBarMenuConfigShowHTML'] = $wgSideBarMenuConfigShowHTML;
45 - $vars['wgSideBarMenuConfigHideHTML'] = $wgSideBarMenuConfigHideHTML;
 44+ $vars['wgSideBarMenuConfigShowHTML'] = isset($wgSideBarMenuConfigShowHTML) ? $wgSideBarMenuConfigShowHTML : wfMsg('controls.show');
 45+ $vars['wgSideBarMenuConfigHideHTML'] = isset($wgSideBarMenuConfigHideHTML) ? $wgSideBarMenuConfigHideHTML : wfMsg('controls.hide');
4646 return true;
4747 }
4848
Index: trunk/extensions/SideBarMenu/includes/MenuParser.php
——@@ -1,11 +1,6 @@
22 <?php
33 class MenuParser {
44
5 - public static function isRoot($line)
6 - {
7 - return !self::startsWith($line, '*');
8 - }
9 -
105 public static function isValidInput($data)
116 {
127 return !(is_null($data) || strlen($data) == 0);
——@@ -77,7 +72,7 @@
7873 $levelArray[$level][] = $line;
7974 }else{
8075 //syntax error
81 - throw new InvalidArgumentException();
 76+ throw new InvalidArgumentException(wfMsg('parser.syntax-error',$line));
8277 }
8378 $prevLevel = $level;
8479 }
Index: trunk/extensions/SideBarMenu/css/ext.sidebarmenu.css
——@@ -4,11 +4,15 @@
55 padding: 5px;
66 float: left;
77 clear: left;
 8+ margin: 5px;
89 }
910
1011 .sidebar-menu-item-collapsed .sidebar-menu{
1112 display: none;
1213 }
 14+.sidebar-menu-item-expanded {
 15+ display: block;
 16+}
1317
1418 .sidebar-menu-0,.sidebar-menu-1{
1519 margin: 0px !important;
——@@ -42,9 +46,13 @@
4347
4448 .sidebar-menu-item-controls{
4549 float: right;
46 - margin-left: 5px;
4750 color: #0645AD;
4851 cursor: pointer;
 52+ min-width: 45px;
 53+ text-align: right;
 54+ -moz-user-select: none;
 55+ -khtml-user-select: none;
 56+ user-select: none;
4957 }
5058
5159 .sidebar-menu-item-1{
Index: trunk/extensions/SideBarMenu/js/ext.sidebarmenu.js
——@@ -1,6 +1,30 @@
22 $(document).ready(function(){
3 - $('.sidebar-menu-item-controls').append(mw.config.get('wgSideBarMenuConfigShowHTML'));
 3+
 4+ var showText = mw.config.get('wgSideBarMenuConfigShowHTML');
 5+ var hideText = mw.config.get('wgSideBarMenuConfigHideHTML');
 6+
 7+ function initControls() {
 8+ $('.sidebar-menu-item-collapsed').children('.sidebar-menu-item-text-container').children('.sidebar-menu-item-controls').append(showText);
 9+ $('.sidebar-menu-item-expanded').children('.sidebar-menu-item-text-container').children('.sidebar-menu-item-controls').append(hideText);
 10+ }
 11+
 12+ initControls();
413 $('.sidebar-menu-item-controls').click(function(){
5 - $(this).parents('.sidebar-menu-item:first').toggleClass('sidebar-menu-item-collapsed',1500);
 14+ var currentText = $(this).text();
 15+
 16+ if(currentText == showText){
 17+ $(this).text(hideText);
 18+ }else if(currentText == hideText){
 19+ $(this).text(showText);
 20+ }
 21+
 22+ //A little "ugly" hack to prevent some gui glitches.
 23+ $(this).parents('.sidebar-menu-item:first').
 24+ toggleClass('sidebar-menu-item-collapsed sidebar-menu-item-expanded',250).children('.sidebar-menu').show(0,function(){
 25+ var _this = $(this);
 26+ setTimeout(function(){
 27+ _this.css('display','')
 28+ },250);
 29+ });
630 });
731 });
\ No newline at end of file

Comments

#Comment by Raymond (talk | contribs)   20:44, 7 March 2012

Some notes on a i18n/i10n base:

  1. Dots in message key are very uncommon. I suggest to use dashes: 'controls.show' -> 'controls-show'
  2. Prefix the message keys with the extension name: 'controls-show' -> 'sidebarmenu-controls-show' to avoid key conflicts with core and/or other extensions
  3. Please add message documentation for the newly added messages. Thanks.

At least 2.+3. are necessary to add your extension to http://translatewiki.net

#Comment by Netbrain (talk | contribs)   21:33, 7 March 2012

this should be fixed now.

Status & tagging log