wikimedia/mediawiki-extensions-MultimediaViewer
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/MultimediaViewer synced 2024-09-29 05:07:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix some metadata panel scrolling/text truncation bugs

Browse source 
* isOpening was calculated too early, so whenever the panel was
  partially open and opened up fully via the forceDirection
  parameter of toggle(), it was logged as the wrong direction.
  (I think the only way for this to happen is via clicking on
  "view terms" while the panel is partially open.)
* scrolling did not go all the way to the bottom when text was
  truncated as the target position was calculated before untruncating.
* panel position was not preserved in some cases where it could have
  been because the attempt to restore it happened before untruncating
  the text, when the panel was not high enough.

Change-Id: I47a96d42c80e0a00d95023526ede3b5bbf18a52c
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/983
This commit is contained in:
Gergő Tisza 2014-11-22 00:16:36 +00:00
parent e7f720506f
commit 0997b82650
3 changed files with 25 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
resources/mmv/ui/mmv.ui.metadataPanel.js
View file

@ -61,6 +61,9 @@
.add( this.$authorAndSource )
.add( this.title.$ellipsis )
.add( this.creditField.$ellipsis )
.each( function () {
$( this ).tipsy( 'enable' );
} )
.on( 'click.mmv-mp', function ( e ) {
var clickTargetIsLink = $( e.target ).is( 'a' ),
clickTargetIsTruncated = !!$( e.target ).closest( '.mw-mmv-ttf-truncated' ).length,
@ -76,23 +79,15 @@
panel.scroller.toggle( 'up' );
} );
$( this.$container ).on( 'mmv-metadata-open', function () {
$( this.$container ).on( 'mmv-metadata-open.mmv-mp mmv-metadata-reveal-truncated-text.mmv-mp', function () {
panel.revealTruncatedText();
} ).on( 'mmv-metadata-close', function () {
} ).on( 'mmv-metadata-close.mmv-mp', function () {
panel.hideTruncatedText();
} );
this.handleEvent( 'jq-fullscreen-change.lip', function() {
panel.hideTruncatedText();
} );
this.$title
.add( this.title.$ellipsis )
.add( this.$authorAndSource )
.add( this.creditField.$ellipsis )
.each( function () {
$( this ).tipsy( 'enable' );
} );
};
MPP.unattach = function() {
@ -104,12 +99,13 @@
.add( this.creditField.$ellipsis )
.each( function () {
$( this ).tipsy( 'hide' ).tipsy( 'disable' );
} );
} )
.off( 'click.mmv-mp' );
$( this.$container ).off( '.mmv-mp' );
this.scroller.unattach();
this.buttons.unattach();
this.$title.add( this.$authorAndSource ).off( 'click.mmv-mp' );
this.clearEvents();
};
@ -671,8 +667,8 @@
this.setUserPageLink( repoData, imageData.lastUploader, user.gender );
}
this.scroller.unfreezeHeight();
this.resetTruncatedText();
this.scroller.unfreezeHeight();
};
/**

30
resources/mmv/ui/mmv.ui.metadataPanelScroller.js
View file

@ -156,34 +156,32 @@
* @param {string} [forceDirection] 'up' or 'down' makes the panel move on that direction (and is a noop
* if the panel is already at the upmost/bottommost position); without the parameter, the panel position
* is toggled. (Partially open counts as open.)
* @param {number} [duration] duration of the scroll animation; defaults to #toggleScrollDuration.
* Use 0 to avoid animating altogether.
* @return {jQuery.Deferred} a deferred which resolves after the animation has finished.
*/
MPSP.toggle = function ( forceDirection, duration ) {
MPSP.toggle = function ( forceDirection ) {
var deferred = $.Deferred(),
scrollTopWhenOpen = this.getScrollTopWhenOpen(),
scrollTopWhenClosed = 0,
scrollTop = $.scrollTo().scrollTop(),
panelIsOpen = scrollTop > scrollTopWhenClosed,
scrollTopTarget = panelIsOpen ? scrollTopWhenClosed : scrollTopWhenOpen,
isOpening = scrollTopTarget === scrollTopWhenOpen;
if ( duration === null || duration === undefined ) {
duration = this.toggleScrollDuration;
}
if ( forceDirection ) {
scrollTopTarget = forceDirection === 'down' ? scrollTopWhenClosed : scrollTopWhenOpen;
}
direction = forceDirection || ( panelIsOpen ? 'down' : 'up' ),
scrollTopTarget = ( direction === 'up' ) ? scrollTopWhenOpen : scrollTopWhenClosed;
// don't log / animate if the panel is already in the end position
if ( scrollTopTarget === scrollTop ) {
deferred.resolve();
} else {
mw.mmv.actionLogger.log( isOpening ? 'metadata-open' : 'metadata-close' );
$.scrollTo( scrollTopTarget, duration, {
mw.mmv.actionLogger.log( direction === 'up' ? 'metadata-open' : 'metadata-close' );
if ( direction === 'up' && !panelIsOpen ) {
// FIXME nasty. This is not really an event but a command sent to the metadata panel;
// child UI elements should not send commands to their parents. However, there is no way
// to calculate the target scrollTop accurately without revealing the text, and the event
// which does that (metadata-open) is only triggered later in the process, when the panel
// actually scrolled, so we cannot use it here without risking triggering it multiple times.
this.$container.trigger( 'mmv-metadata-reveal-truncated-text' );
scrollTopTarget = this.getScrollTopWhenOpen();
}
$.scrollTo( scrollTopTarget, this.toggleScrollDuration, {
onAfter: function () {
deferred.resolve();
}