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 <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits