r66148 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r66147‎ | r66148 | r66149 >
Date:17:57, 10 May 2010
Author:tparscal
Status:ok (Comments)
Tags:
Comment:
Better solution to bug #23428.
Modified paths:
  • /trunk/extensions/UsabilityInitiative/Vector/Modules/CollapsibleNav/CollapsibleNav.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/Vector/Vector.combined.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/Vector/Vector.combined.min.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/Vector/Vector.hooks.php (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/Vector/Modules/CollapsibleNav/CollapsibleNav.js
@@ -35,11 +35,22 @@
3636 .slideToggle( 'fast' );
3737 }
3838 var $headings = $j( '#panel > div.portal > h5' );
39 - var tabindex = 32767 - $headings.length;
 39+ /** Copy-pasted from jquery.wikiEditor.dialogs - :( */
 40+ // Find the highest tabindex in use
 41+ var maxTI = 0;
 42+ $j( '[tabindex]' ).each( function() {
 43+ var ti = parseInt( $j(this).attr( 'tabindex' ) );
 44+ if ( ti > maxTI )
 45+ maxTI = ti;
 46+ });
 47+ var tabIndex = maxTI + 1;
 48+ // Make it keyboard accessible
 49+ $headings.each( function() {
 50+ $j(this).attr( 'tabindex', tabIndex++ );
 51+ } );
 52+ /** End of copy-pasted section */
4053 // Toggle the selected menu's class and expand or collapse the menu
4154 $headings
42 - // Make it keyboard accessible
43 - .each( function() { $j(this).attr( 'tabindex', tabindex++ ); } )
4455 // Make the space and enter keys act as a click
4556 .keydown( function( event ) {
4657 if ( event.which == 13 /* Enter */ || event.which == 32 /* Space */ ) {
Index: trunk/extensions/UsabilityInitiative/Vector/Vector.hooks.php
@@ -12,17 +12,17 @@
1313
1414 static $scripts = array(
1515 'raw' => array(
16 - array( 'src' => 'Modules/CollapsibleNav/CollapsibleNav.js', 'version' => 10 ),
 16+ array( 'src' => 'Modules/CollapsibleNav/CollapsibleNav.js', 'version' => 11 ),
1717 array( 'src' => 'Modules/CollapsibleTabs/CollapsibleTabs.js', 'version' => 8 ),
1818 array( 'src' => 'Modules/EditWarning/EditWarning.js', 'version' => 8 ),
1919 array( 'src' => 'Modules/FooterCleanup/FooterCleanup.js', 'version' => 5 ),
2020 array( 'src' => 'Modules/SimpleSearch/SimpleSearch.js', 'version' => 9 ),
2121 ),
2222 'combined' => array(
23 - array( 'src' => 'Vector.combined.js', 'version' => 27 ),
 23+ array( 'src' => 'Vector.combined.js', 'version' => 28 ),
2424 ),
2525 'minified' => array(
26 - array( 'src' => 'Vector.combined.min.js', 'version' => 27 ),
 26+ array( 'src' => 'Vector.combined.min.js', 'version' => 28 ),
2727 ),
2828 );
2929 static $modules = array(
Index: trunk/extensions/UsabilityInitiative/Vector/Vector.combined.js
@@ -35,11 +35,22 @@
3636 .slideToggle( 'fast' );
3737 }
3838 var $headings = $j( '#panel > div.portal > h5' );
39 - var tabindex = 32767 - $headings.length;
 39+ /** Copy-pasted from jquery.wikiEditor.dialogs - :( */
 40+ // Find the highest tabindex in use
 41+ var maxTI = 0;
 42+ $j( '[tabindex]' ).each( function() {
 43+ var ti = parseInt( $j(this).attr( 'tabindex' ) );
 44+ if ( ti > maxTI )
 45+ maxTI = ti;
 46+ });
 47+ var tabIndex = maxTI + 1;
 48+ // Make it keyboard accessible
 49+ $headings.each( function() {
 50+ $j(this).attr( 'tabindex', tabIndex++ );
 51+ } );
 52+ /** End of copy-pasted section */
4053 // Toggle the selected menu's class and expand or collapse the menu
4154 $headings
42 - // Make it keyboard accessible
43 - .each( function() { $j(this).attr( 'tabindex', tabindex++ ); } )
4455 // Make the space and enter keys act as a click
4556 .keydown( function( event ) {
4657 if ( event.which == 13 /* Enter */ || event.which == 32 /* Space */ ) {
Index: trunk/extensions/UsabilityInitiative/Vector/Vector.combined.min.js
@@ -1,7 +1,8 @@
22
33 $j(document).ready(function(){if(!wgVectorEnabledModules.collapsiblenav){return true;}
44 $j('#panel').addClass('collapsible-nav');$j('#panel > div.portal:first').addClass('expanded').find('div.body').show();$j('#panel > div.portal:not(:first)').each(function(i){var state=$j.cookie('vector-nav-'+$j(this).attr('id'));if(state=='true'||(state==null&&i<1)){$j(this).addClass('expanded').find('div.body').show();}else{$j(this).addClass('collapsed');}});function toggle($element){$j.cookie('vector-nav-'+$element.parent().attr('id'),$element.parent().is('.collapsed'));$element.parent().toggleClass('expanded').toggleClass('collapsed').find('div.body').slideToggle('fast');}
5 -var $headings=$j('#panel > div.portal > h5');var tabindex=32767-$headings.length;$headings.each(function(){$j(this).attr('tabindex',tabindex++);}).keydown(function(event){if(event.which==13||event.which==32){toggle($j(this));}}).mousedown(function(){toggle($j(this));$j(this).blur();return false;});});$j(document).ready(function(){if(!wgVectorEnabledModules.collapsibletabs){return true;}
 5+var $headings=$j('#panel > div.portal > h5');var maxTI=0;$j('[tabindex]').each(function(){var ti=parseInt($j(this).attr('tabindex'));if(ti>maxTI)
 6+maxTI=ti;});var tabIndex=maxTI+1;$headings.each(function(){$j(this).attr('tabindex',tabIndex++);});$headings.keydown(function(event){if(event.which==13||event.which==32){toggle($j(this));}}).mousedown(function(){toggle($j(this));$j(this).blur();return false;});});$j(document).ready(function(){if(!wgVectorEnabledModules.collapsibletabs){return true;}
67 var rtl=$j('body').is('.rtl');$j.collapsibleTabs.moveToCollapsed=function(ele){var $moving=$j(ele);$j.collapsibleTabs.getSettings($j($j.collapsibleTabs.getSettings($moving).expandedContainer)).shifting=true;var data=$j.collapsibleTabs.getSettings($moving);var target=data.collapsedContainer;$moving.css("position","relative").css((rtl?'left':'right'),0).animate({width:'1px'},"normal",function(){$j(this).hide();$j('<span class="placeholder" style="display:none;"></span>').insertAfter(this);$j(this).remove().prependTo(target).data('collapsibleTabsSettings',data);$j(this).attr('style','display:list-item;');$j.collapsibleTabs.getSettings($j($j.collapsibleTabs.getSettings($j(ele)).expandedContainer)).shifting=false;$j.collapsibleTabs.handleResize();});};$j.collapsibleTabs.moveToExpanded=function(ele){var $moving=$j(ele);$j.collapsibleTabs.getSettings($j($j.collapsibleTabs.getSettings($moving).expandedContainer)).shifting=true;var data=$j.collapsibleTabs.getSettings($moving);var $target=$j(data.expandedContainer).find('span.placeholder:first');var expandedWidth=data.expandedWidth;$moving.css("position","relative").css((rtl?'right':'left'),0).css('width','1px');$target.replaceWith($moving.remove().css('width','1px').data('collapsibleTabsSettings',data).animate({width:expandedWidth+"px"},"normal",function(){$j(this).attr('style','display:block;');$j.collapsibleTabs.getSettings($j($j.collapsibleTabs.getSettings($moving).expandedContainer)).shifting=false;$j.collapsibleTabs.handleResize();}));};$j('#p-views ul').bind("beforeTabCollapse",function(){if($j('#p-cactions').css('display')=='none')
78 $j("#p-cactions").addClass("filledPortlet").removeClass("emptyPortlet").find('h5').css('width','1px').animate({'width':'26px'},390);}).bind("beforeTabExpand",function(){if($j('#p-cactions li').length==1)
89 $j("#p-cactions h5").animate({'width':'1px'},370,function(){$j(this).attr('style','').parent().addClass("emptyPortlet").removeClass("filledPortlet");});}).collapsibleTabs({expandCondition:function(eleWidth){if(rtl){return($j('#right-navigation').position().left+$j('#right-navigation').width()+1)<($j('#left-navigation').position().left-eleWidth);}else{return($j('#left-navigation').position().left+$j('#left-navigation').width()+1)<($j('#right-navigation').position().left-eleWidth);}},collapseCondition:function(){if(rtl){return($j('#right-navigation').position().left+$j('#right-navigation').width())>$j('#left-navigation').position().left;}else{return($j('#left-navigation').position().left+$j('#left-navigation').width())>$j('#right-navigation').position().left;}}});});$j(document).ready(function(){if(!wgVectorEnabledModules.editwarning||$j('#wpTextbox1').size()==0){return true;}

Follow-up revisions

RevisionCommit summaryAuthorDate
r661491.16wmf4: MFT r66034, r66144, r66145, r66147, r66148catrope18:10, 10 May 2010

Comments

#Comment by Catrope (talk | contribs)   18:01, 10 May 2010

Code looks OK but could just call $.wikiEditor.modules.dialogs.fn.setTabindexes( $headings ); rather than duplicating its code.

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:10, 10 May 2010

Probably a bad idea to do that. We can't assume the wikiEditor code is present. The fact that we load it together with other plug-ins is an implementation detail that will change when we have a proper script loader.

#Comment by Catrope (talk | contribs)   18:13, 10 May 2010

Hm, true. Should probably move that function to jquery.wikiEditor.js then.

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:27, 10 May 2010

Same issue - jquery.wikiEditor may or may not be present - the code we're working on here is in the Vector extension.

#Comment by Catrope (talk | contribs)   18:28, 10 May 2010

Gah, you're right. Maybe we should invent a new place for common code between them then, quite possibly wikibits.js v2

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:29, 10 May 2010

Now you're talking!

Status & tagging log