From ab874bdd57e2a95a73c66b641906c29df2b82a73 Mon Sep 17 00:00:00 2001 From: WMDE-Fisch Date: Wed, 12 Jul 2017 17:03:25 +0200 Subject: [PATCH] Improve doc, naming and pointer updates Several improvments including documentation, naming and refactoring: - bundeling of methods setting, redrawing and loading new diffs - renamed and improved documentation around some methods in that area - fixed some leftovers from past refactoring not using new methods Change-Id: Idf1afcd6ce9210951d28655128a47150f6b7a2aa --- modules/ext.RevisionSlider.DiffPage.js | 12 +-- modules/ext.RevisionSlider.PointerView.js | 8 +- modules/ext.RevisionSlider.Slider.js | 2 +- modules/ext.RevisionSlider.SliderArrowView.js | 2 +- modules/ext.RevisionSlider.SliderView.js | 99 ++++++++++++------- 5 files changed, 74 insertions(+), 49 deletions(-) diff --git a/modules/ext.RevisionSlider.DiffPage.js b/modules/ext.RevisionSlider.DiffPage.js index cb4712aa..bcbb6685 100644 --- a/modules/ext.RevisionSlider.DiffPage.js +++ b/modules/ext.RevisionSlider.DiffPage.js @@ -186,21 +186,17 @@ * @param {SliderView} sliderView */ initOnPopState: function ( sliderView ) { - var self = this; window.addEventListener( 'popstate', function ( event ) { if ( event.state === null ) { return; } mw.track( 'counter.MediaWiki.RevisionSlider.event.historyChange' ); - sliderView.pointerOlder.setPosition( event.state.pointerOlderPos ); - sliderView.pointerNewer.setPosition( event.state.pointerNewerPos ); sliderView.slider.setFirstVisibleRevisionIndex( event.state.sliderPos ); - sliderView.slide( 0 ); - sliderView.resetSliderLines(); - sliderView.resetRevisionStylesBasedOnPointerPosition( - sliderView.$element.find( 'div.mw-revslider-revisions' ) + sliderView.updatePointersAndDiffView( + event.state.pointerNewerPos, + event.state.pointerOlderPos, + false ); - self.refresh( event.state.diff, event.state.oldid ); } ); }, diff --git a/modules/ext.RevisionSlider.PointerView.js b/modules/ext.RevisionSlider.PointerView.js index 49774c1d..f2e654ad 100644 --- a/modules/ext.RevisionSlider.PointerView.js +++ b/modules/ext.RevisionSlider.PointerView.js @@ -90,7 +90,7 @@ * Moves the pointer to a position * * @param {number} posInPx - * @param {number} duration + * @param {number|string} duration * @return {jQuery} */ animateTo: function ( posInPx, duration ) { @@ -105,7 +105,7 @@ * Slides the pointer to the revision it's pointing at * * @param {Slider} slider - * @param {number} duration + * @param {number|string} duration * @return {jQuery} */ slideToPosition: function ( slider, duration ) { @@ -118,7 +118,7 @@ * * @param {Slider} slider * @param {boolean} posBeforeSlider - * @param {number} duration + * @param {number|string} duration * @return {jQuery} */ slideToSide: function ( slider, posBeforeSlider, duration ) { @@ -133,7 +133,7 @@ * Decides based on its position whether the pointer should be sliding to the side or to its position * * @param {Slider} slider - * @param {number} duration + * @param {number|string} duration * @return {jQuery} */ slideToSideOrPosition: function ( slider, duration ) { diff --git a/modules/ext.RevisionSlider.Slider.js b/modules/ext.RevisionSlider.Slider.js index 5bf78188..6939de20 100644 --- a/modules/ext.RevisionSlider.Slider.js +++ b/modules/ext.RevisionSlider.Slider.js @@ -105,7 +105,7 @@ /** * Sets the new oldestVisibleRevisionIndex after sliding in a direction * - * @param {number} direction - Either -1 or 1 + * @param {number} direction - Either -1, 0 or 1 */ slide: function ( direction ) { var highestPossibleFirstRev = this.revisions.getLength() - this.revisionsPerWindow; diff --git a/modules/ext.RevisionSlider.SliderArrowView.js b/modules/ext.RevisionSlider.SliderArrowView.js index 357b8115..22b4adcd 100644 --- a/modules/ext.RevisionSlider.SliderArrowView.js +++ b/modules/ext.RevisionSlider.SliderArrowView.js @@ -125,7 +125,7 @@ return; } mw.track( 'counter.MediaWiki.RevisionSlider.event.arrowClick' ); - this.sliderView.slide( button.$element.data( 'dir' ) ); + this.sliderView.slideView( button.$element.data( 'dir' ) ); }, /** diff --git a/modules/ext.RevisionSlider.SliderView.js b/modules/ext.RevisionSlider.SliderView.js index 62058df3..a05aea09 100644 --- a/modules/ext.RevisionSlider.SliderView.js +++ b/modules/ext.RevisionSlider.SliderView.js @@ -127,7 +127,7 @@ $container.html( this.$element ); - this.slide( Math.floor( ( this.getNewerPointerPos() - 1 ) / this.slider.getRevisionsPerWindow() ), 0 ); + this.slideView( Math.floor( ( this.getNewerPointerPos() - 1 ) / this.slider.getRevisionsPerWindow() ), 0 ); this.diffPage.addHandlersToCoreLinks( this ); this.diffPage.replaceState( mw.config.get( 'extRevisionSliderNewRev' ), mw.config.get( 'extRevisionSliderOldRev' ), this ); this.diffPage.initOnPopState( this ); @@ -295,18 +295,20 @@ pointerMoved.setPosition( pos ); if ( $line.hasClass( 'mw-revslider-pointer-container-newer' ) ) { - this.refreshRevisions( + this.refreshDiffView( $clickedRev.attr( 'data-revid' ), - this.getRevElementAtPosition( $revisions, pointerOther.getPosition() ).attr( 'data-revid' ) + this.getRevElementAtPosition( $revisions, pointerOther.getPosition() ).attr( 'data-revid' ), + true ); } else { - this.refreshRevisions( + this.refreshDiffView( this.getRevElementAtPosition( $revisions, pointerOther.getPosition() ).attr( 'data-revid' ), - $clickedRev.attr( 'data-revid' ) + $clickedRev.attr( 'data-revid' ), + true ); } + this.alignPointersAndLines(); this.resetRevisionStylesBasedOnPointerPosition( $revisions ); - this.alignPointers(); }, /** @@ -343,26 +345,24 @@ self.removePointerDragCursor(); if ( self.escapePressed ) { - self.resetSliderLines(); return; } mw.track( 'counter.MediaWiki.RevisionSlider.event.pointerMove' ); + pointer.setPosition( self.slider.getOldestVisibleRevisionIndex() + relativeIndex ); - self.resetSliderLines(); - self.resetRevisionStylesBasedOnPointerPosition( $revisions ); diff = self.getRevElementAtPosition( - $revisions, self.pointerNewer.getPosition() + $revisions, self.getNewerPointerPos() ).data( 'revid' ); oldid = self.getRevElementAtPosition( $revisions, self.getOlderPointerPos() ).data( 'revid' ); - self.refreshRevisions( diff, oldid ); - - self.redrawPointerLines(); + self.refreshDiffView( diff, oldid, true ); + self.alignPointersAndLines( 0 ); + self.resetRevisionStylesBasedOnPointerPosition( $revisions ); }, drag: function ( event, ui ) { lastValidLeftPos = self.draggableDragAction( @@ -459,37 +459,55 @@ }, /** - * Refreshes the diff page to show the diff for the specified revisions + * Loads a new diff and optionally adds a state to the history * * @param {number} diff * @param {number} oldid + * @param {boolean} pushState */ - refreshRevisions: function ( diff, oldid ) { + refreshDiffView: function ( diff, oldid, pushState ) { this.diffPage.refresh( diff, oldid, this ); - this.diffPage.pushState( diff, oldid, this ); + if ( pushState ) { + this.diffPage.pushState( diff, oldid, this ); + } }, showNextDiff: function () { - this.setOlderPointerPos( this.getNewerPointerPos() ); - this.setNewerPointerPos( this.getNewerPointerPos() + 1 ); - this.resetAndRefreshRevisions(); + this.updatePointersAndDiffView( + this.getNewerPointerPos() + 1, + this.getNewerPointerPos(), + true + ); }, showPrevDiff: function () { - this.setNewerPointerPos( this.getOlderPointerPos() ); - this.setOlderPointerPos( this.getOlderPointerPos() - 1 ); - this.resetAndRefreshRevisions(); + this.updatePointersAndDiffView( + this.getOlderPointerPos(), + this.getOlderPointerPos() - 1, + true + ); }, - resetAndRefreshRevisions: function () { - this.slide( 0 ); - this.resetSliderLines(); - this.resetRevisionStylesBasedOnPointerPosition( - this.$element.find( 'div.mw-revslider-revisions' ) - ); - this.refreshRevisions( - $( '.mw-revslider-revision[data-pos="' + this.getNewerPointerPos() + '"]' ).attr( 'data-revid' ), - $( '.mw-revslider-revision[data-pos="' + this.getOlderPointerPos() + '"]' ).attr( 'data-revid' ) + /** + * Updates and moves pointers to new positions, resets styles and refreshes diff accordingly + * + * @param {number} newPointerPos + * @param {number} oldPointerPos + * @param {boolean} pushState + */ + updatePointersAndDiffView: function ( + newPointerPos, + oldPointerPos, + pushState + ) { + this.setNewerPointerPos( newPointerPos ); + this.setOlderPointerPos( oldPointerPos ); + this.alignPointersAndLines(); + this.resetRevisionStylesBasedOnPointerPosition( this.getRevisionsElement() ); + this.refreshDiffView( + $( '.mw-revslider-revision[data-pos="' + newPointerPos + '"]' ).attr( 'data-revid' ), + $( '.mw-revslider-revision[data-pos="' + oldPointerPos + '"]' ).attr( 'data-revid' ), + pushState ); }, @@ -661,7 +679,13 @@ ) * this.revisionWidth; }, - slide: function ( direction, duration ) { + /** + * Slide the view to the next chunk of older / newer revisions + * + * @param {number} direction - Either -1, 0 or 1 + * @param {number|string} [duration] + */ + slideView: function ( direction, duration ) { var animateObj, $animatedElement = this.$element.find( '.mw-revslider-revisions-container' ), self = this; @@ -703,7 +727,7 @@ } ); - this.alignPointers( duration ); + this.alignPointersAndLines( duration ); }, /** @@ -721,7 +745,12 @@ return $element.prop( 'scrollWidth' ) - $element.width() - scrollLeft; }, - alignPointers: function ( duration ) { + /** + * Visually move pointers to the positions set and reset pointer- and slider-lines + * + * @param {number|string} [duration] + */ + alignPointersAndLines: function ( duration ) { var self = this; this.fadeOutPointerLines(); @@ -950,7 +979,7 @@ expandedRevisionWindowCapacity = $slider.find( '.mw-revslider-revisions-container' ).width() / this.revisionWidth; this.slider.setRevisionsPerWindow( expandedRevisionWindowCapacity ); - this.slide( Math.floor( ( this.getNewerPointerPos() - 1 ) / expandedRevisionWindowCapacity ), 0 ); + this.slideView( Math.floor( ( this.getNewerPointerPos() - 1 ) / expandedRevisionWindowCapacity ), 0 ); }, /**