Gergő Tisza has uploaded a new change for review. https://gerrit.wikimedia.org/r/175182
Change subject: Fix some metadata panel scrolling/text truncation bugs ...................................................................... Fix some metadata panel scrolling/text truncation bugs * 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 --- M resources/mmv/ui/mmv.ui.canvas.js M resources/mmv/ui/mmv.ui.metadataPanel.js M resources/mmv/ui/mmv.ui.metadataPanelScroller.js 3 files changed, 25 insertions(+), 31 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer refs/changes/82/175182/1 diff --git a/resources/mmv/ui/mmv.ui.canvas.js b/resources/mmv/ui/mmv.ui.canvas.js index af86e3b..032e419 100644 --- a/resources/mmv/ui/mmv.ui.canvas.js +++ b/resources/mmv/ui/mmv.ui.canvas.js @@ -246,7 +246,7 @@ * @param {mw.mmv.model.ThumbnailWidth} imageWidths * @returns {boolean} Whether the image was blured or not */ - C.maybeDisplayPlaceholder = function ( size, $imagePlaceholder, imageWidths ) { + C.maybeDisplayPlaceholder = function ( size, $imagePlaceholder, imageWidths ) { var targetWidth, targetHeight, blowupFactor, diff --git a/resources/mmv/ui/mmv.ui.metadataPanel.js b/resources/mmv/ui/mmv.ui.metadataPanel.js index 320ef14..5c3bc34 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanel.js +++ b/resources/mmv/ui/mmv.ui.metadataPanel.js @@ -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(); }; /** diff --git a/resources/mmv/ui/mmv.ui.metadataPanelScroller.js b/resources/mmv/ui/mmv.ui.metadataPanelScroller.js index 0992c1e..3e7416c 100644 --- a/resources/mmv/ui/mmv.ui.metadataPanelScroller.js +++ b/resources/mmv/ui/mmv.ui.metadataPanelScroller.js @@ -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(); } -- To view, visit https://gerrit.wikimedia.org/r/175182 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I47a96d42c80e0a00d95023526ede3b5bbf18a52c Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
