Krinkle has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/70843


Change subject: mw.ViewPageTarget: Performance improvements for section edit 
links
......................................................................

mw.ViewPageTarget: Performance improvements for section edit links

Follows-up I4b9c47fd65a700a:
* Minor optimisation for closingBracketSymbol by re-using the
  reference we already have (this.lastChild) instead of digging
  into $brackets again and creating a new jQuery object for the
  second element (eq=1) and getting the text.
* Moving veSectionEditUri and sectionEditUri inline as it is
  only used once. This will allow it to be easily dereferenced
  (instead of likely staying in memory due to being claimed by
  remaining closures "expand" and "expandSoon" which will
  continue to claim and enjoy access to this scope.
  Also changed it to use the attr() callback so that we
  don't access `$editLink.attr( 'href' )` twice.
* Add various comments explaining this non-obvious approach.
  Though it makes sense (eventually) and more elaborate details
  are in the original commitmsg, that is no excuse for a 100 line
  function without a single comment, especially with all these
  things going on.
* Use "shrink" instead of "contract" (seems more common in
  context of web UI elements and less ambgiuous in English).
* Remove redundant toString in `new mw.Uri( veEditUri )`.
  Aside from redundant, it also had more overhead (serialising
  uri object to string and re-parsing). It revoked its ability
  to make an efficient clone by taking the mw.Uri object as input
  as opposed to having to parse it again. mw.Uri#clone does the
  same internally.
* Avoid variable names like $this, that or self. Using a more
  descriptive name instead.
* Re-use $closingBracket instead of using last() 3 times which
  makes 3 clones of the jQuery object with the same last element
  in it.

Change-Id: I5f093f2927b769fed0c6d1a40f99e73f9b653b9a
---
M modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
1 file changed, 43 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor 
refs/changes/43/70843/1

diff --git a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js 
b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
index cceba19..81afd6f 100644
--- a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
@@ -1070,37 +1070,40 @@
                $editsections = $( '#mw-content-text .mw-editsection' ),
                handler = ve.bind( this.onEditSectionLinkClick, this );
 
+       // The "visibility" css construct ensures we always occupy the same 
space in the layout.
+       // This prevents the heading from changing its wrap when the user 
toggles editSourceLink.
        $editsections.each( function () {
-               var $brackets, $expandedOnly, $middleBracket, $hiddenBracket, 
closingBracketSymbol,
-                       expandTimeout, contractTimeout,
-                       $this = $( this ),
-                       $heading = $this.closest( 'h1,h2,h3,h4,h5,h6' ),
-                       $edit = $this.find( 'a' ).eq( 0 ),
-                       $editSource = $edit.clone(),
-                       $links = $edit.add( $editSource ),
+               var $expandedOnly, $middleBracket, $hiddenBracket, 
$closingBracket, closingBracketText,
+                       expandTimeout, shrinkTimeout,
+                       $editsection = $( this ),
+                       $heading = $editsection.closest( 'h1, h2, h3, h4, h5, 
h6' ),
+                       $editLink = $editsection.find( 'a' ).eq( 0 ),
+                       $editSourceLink = $editLink.clone(),
+                       $links = $editLink.add( $editSourceLink ),
                        $divider = $( '<span>' ),
-                       dividerSymbol = $.trim( ve.msg( 'pipe-separator' ) ),
-                       veSectionEditUri = new mw.Uri( veEditUri.toString() ),
-                       sectionEditUri = new mw.Uri( $edit.attr( 'href' ) );
+                       dividerText = $.trim( ve.msg( 'pipe-separator' ) ),
+                       $brackets = $( [ this.firstChild, this.lastChild ] );
 
                function expandSoon() {
-                       clearTimeout( contractTimeout );
+                       // Cancel pending shrink, schedule expansion instead
+                       clearTimeout( shrinkTimeout );
                        expandTimeout = setTimeout( expand, 100 );
                }
 
-               function contractSoon() {
-                       clearTimeout( expandTimeout );
-                       contractTimeout = setTimeout( contract, 100 );
-               }
-
                function expand() {
-                       clearTimeout( contractTimeout );
+                       clearTimeout( shrinkTimeout );
                        $middleBracket.css( 'visibility', 'hidden' );
                        $expandedOnly.css( 'visibility', 'visible' );
                        $heading.addClass( 'mw-editsection-expanded' );
                }
 
-               function contract() {
+               function shrinkSoon() {
+                       // Cancel pending expansion, schedule shrink instead
+                       clearTimeout( expandTimeout );
+                       shrinkTimeout = setTimeout( shrink, 100 );
+               }
+
+               function shrink() {
                        clearTimeout( expandTimeout );
                        if ( !$links.is( ':focus' ) ) {
                                $middleBracket.css( 'visibility', 'visible' );
@@ -1109,37 +1112,42 @@
                        }
                }
 
-               $brackets = $( [ this.firstChild, this.lastChild ] );
-               // @until Id27555c6 - essentially does this for us
+               // TODO: Remove this (see Id27555c6 in mediawiki/core)
                if ( !$brackets.hasClass( 'mw-editsection-bracket' ) ) {
                        $brackets = $brackets
                                .wrap( $( '<span>' ).addClass( 
'mw-editsection-bracket' ) )
                                .parent();
                }
-               closingBracketSymbol = $brackets.eq( 1 ).text();
-               $expandedOnly = $divider.add( $editSource ).add( 
$brackets.last() );
-               $middleBracket = $brackets.last().clone();
-               $hiddenBracket = $brackets.last().clone().css( 'visibility', 
'hidden' );
+
+               closingBracketText = $( this.lastChild ).text();
+               $closingBracket = $brackets.last();
+               $expandedOnly = $divider.add( $editSourceLink ).add( 
$closingBracket )
+                       .css( 'visibility', 'hidden' );
+               $middleBracket = $closingBracket.clone();
+               // The hidden bracket after the devider ensures we have 
balanced space before and after
+               // divider. The space before the devider is provided by the 
original closing bracket.
+               $hiddenBracket = $closingBracket.clone().css( 'visibility', 
'hidden' );
 
                // Events
-               $heading.on( { 'mouseenter': expandSoon, 'mouseleave': 
contractSoon } );
-               $links.on( { 'focus': expand, 'blur': contract } );
-               $edit.attr( 'href', veSectionEditUri.extend( {
-                       'vesection': sectionEditUri.query.section
-               } ) );
-               $edit.click( handler );
+               $heading.on( { 'mouseenter': expandSoon, 'mouseleave': 
shrinkSoon } );
+               $links.on( { 'focus': expand, 'blur': shrink } );
+               $editLink.click( handler );
 
                // Initialization
-               $expandedOnly.css( 'visibility', 'hidden' );
-               $editSource
+               $editSourceLink
                        .addClass( 'mw-editsection-link-secondary' )
                        .text( mw.msg( 'visualeditor-ca-editsource-section' ) );
                $divider
                        .addClass( 'mw-editsection-divider' )
-                       .text( dividerSymbol );
-               $edit
+                       .text( dividerText );
+               $editLink
+                       .attr( 'href', function ( i, val ) {
+                               return new mw.Uri( veEditUri ).extend( {
+                                       'vesection': new mw.Uri( val 
).query.section
+                               } );
+                       } )
                        .addClass( 'mw-editsection-link-primary' )
-                       .after( $middleBracket, $divider, $hiddenBracket, 
$editSource );
+                       .after( $middleBracket, $divider, $hiddenBracket, 
$editSourceLink );
        } );
 };
 

-- 
To view, visit https://gerrit.wikimedia.org/r/70843
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f093f2927b769fed0c6d1a40f99e73f9b653b9a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to