From d006c26f65d3aa170dc2a5b3340d1e561c90cb85 Mon Sep 17 00:00:00 2001 From: thiemowmde Date: Tue, 30 Jan 2024 15:53:39 +0100 Subject: [PATCH] Merge a small piece of code duplication in SliderView The method is only called from two places, and both do very similar things before the call. This duplication can be merged. One notable difference is that .scrollLeft() was possibly called twice before. The first call was pointless anyway. The argument for .scrollLeft() is absolute, not relative. This code cleanup is motivated by T352169, but doesn't solve it. Bug: T352169 Change-Id: I75e9ffc77ef6331f14e074921c78c28233e60840 --- modules/ext.RevisionSlider.SliderView.js | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/modules/ext.RevisionSlider.SliderView.js b/modules/ext.RevisionSlider.SliderView.js index d6a946aa..17b3e94f 100644 --- a/modules/ext.RevisionSlider.SliderView.js +++ b/modules/ext.RevisionSlider.SliderView.js @@ -906,14 +906,9 @@ $.extend( SliderView.prototype, { this.backwardArrowButton.setDisabled( this.slider.isAtStart() ); this.forwardArrowButton.setDisabled( this.slider.isAtEnd() ); - const animateObj = { scrollLeft: this.slider.getOldestVisibleRevisionIndex() * this.revisionWidth }; - if ( this.dir === 'rtl' ) { - animateObj.scrollLeft = this.getRtlScrollLeft( $animatedElement, animateObj.scrollLeft ); - } - // eslint-disable-next-line no-jquery/no-animate $animatedElement.animate( - animateObj, + { scrollLeft: this.getScrollLeft( $animatedElement ) }, duration, null, function () { @@ -939,11 +934,11 @@ $.extend( SliderView.prototype, { /** * @private * @param {jQuery} $element - * @param {number} scrollLeft * @return {number} */ - getRtlScrollLeft: function ( $element, scrollLeft ) { - if ( this.rtlScrollLeftType === 'reverse' ) { + getScrollLeft: function ( $element ) { + const scrollLeft = this.slider.getOldestVisibleRevisionIndex() * this.revisionWidth; + if ( this.dir !== 'rtl' || this.rtlScrollLeftType === 'reverse' ) { return scrollLeft; } if ( this.rtlScrollLeftType === 'negative' ) { @@ -1146,11 +1141,7 @@ $.extend( SliderView.prototype, { const revIdNew = this.getRevElementAtPosition( $revisions, this.getNewerPointerPos() ).data( 'revid' ); this.diffPage.replaceState( revIdNew, revIdOld, this ); - const scrollLeft = this.slider.getOldestVisibleRevisionIndex() * this.revisionWidth; - $revisionContainer.scrollLeft( scrollLeft ); - if ( this.dir === 'rtl' ) { - $revisionContainer.scrollLeft( this.getRtlScrollLeft( $revisionContainer, scrollLeft ) ); - } + $revisionContainer.scrollLeft( this.getScrollLeft( $revisionContainer ) ); if ( this.shouldExpandSlider( $slider ) ) { this.expandSlider( $slider );