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