From ed2c0a8b73dcf9d2e325b2ad7d8079492c343616 Mon Sep 17 00:00:00 2001 From: thiemowmde Date: Sat, 27 Jan 2024 13:16:52 +0100 Subject: [PATCH] Merge large chunk of code duplication in SliderArrowView This was almost the exact same code, with very few differences (e.g. different CSS classes, and different icons). Another change in the same file is the removal of an extra `!== undefined` check. This is impossible. I can't tell why this was there. Maybe an artifact from when this was developed? This code cleanup is motivated by T352169, but doesn't solve it. Bug: T352169 Change-Id: I3bb7ce00f9b754f9ba58310100b855c8ee3fca4a --- modules/ext.RevisionSlider.SliderArrowView.js | 95 ++++++------------- modules/ext.RevisionSlider.SliderView.js | 4 +- 2 files changed, 31 insertions(+), 68 deletions(-) diff --git a/modules/ext.RevisionSlider.SliderArrowView.js b/modules/ext.RevisionSlider.SliderArrowView.js index 707ea0bd..edb8be83 100644 --- a/modules/ext.RevisionSlider.SliderArrowView.js +++ b/modules/ext.RevisionSlider.SliderArrowView.js @@ -16,97 +16,60 @@ $.extend( SliderArrowView.prototype, { sliderView: null, /** - * Renders the backwards arrow button, returns it - * and renders and adds the popup for it. - * + * @param {number} direction -1 or 1 * @return {OO.ui.ButtonWidget} */ - renderBackwardArrow: function () { - const backwardArrowButton = new OO.ui.ButtonWidget( { - icon: 'previous', + renderArrowButton: function ( direction ) { + const prev = direction < 0; + const button = new OO.ui.ButtonWidget( { + icon: prev ? 'previous' : 'next', width: 20, height: 140, framed: true, - classes: [ 'mw-revslider-arrow', 'mw-revslider-arrow-backwards' ] + classes: [ + 'mw-revslider-arrow', + prev ? 'mw-revslider-arrow-backwards' : 'mw-revslider-arrow-forwards' + ] } ); - const backwardArrowPopup = new OO.ui.PopupWidget( { - $content: $( '

' ).text( mw.msg( 'revisionslider-arrow-tooltip-older' ) ), - $floatableContainer: backwardArrowButton.$element, + const tooltip = mw.msg( prev ? 'revisionslider-arrow-tooltip-older' : 'revisionslider-arrow-tooltip-newer' ); + const popup = new OO.ui.PopupWidget( { + $content: $( '

' ).text( tooltip ), + $floatableContainer: button.$element, width: 200, classes: [ 'mw-revslider-tooltip', 'mw-revslider-arrow-tooltip' ] } ); - backwardArrowButton.connect( this, { - click: [ 'arrowClickHandler', backwardArrowButton ] - } ); + button.connect( this, { click: [ 'arrowClickHandler', button ] } ); - backwardArrowButton.$element - .attr( 'data-dir', -1 ) - .children().attr( 'aria-label', mw.msg( 'revisionslider-arrow-tooltip-older' ) ) - .on( 'mouseover', { button: backwardArrowButton, popup: backwardArrowPopup }, this.showPopup ) - .on( 'mouseout', { popup: backwardArrowPopup }, this.hidePopup ) - .on( 'focusin', { button: backwardArrowButton }, this.arrowFocusHandler ); + button.$element + .attr( 'data-dir', direction ) + .children().attr( 'aria-label', tooltip ) + .on( 'mouseover', { button: button, popup: popup }, this.showPopup ) + .on( 'mouseout', { popup: popup }, this.hidePopup ) + .on( 'focusin', { button: button }, this.arrowFocusHandler ); - $( document.body ).append( backwardArrowPopup.$element ); + $( document.body ).append( popup.$element ); - return backwardArrowButton; - }, - - /** - * Renders the forwards arrow button, returns it - * and renders and adds the popup for it. - * - * @return {OO.ui.ButtonWidget} - */ - renderForwardArrow: function () { - const forwardArrowButton = new OO.ui.ButtonWidget( { - icon: 'next', - width: 20, - height: 140, - framed: true, - classes: [ 'mw-revslider-arrow', 'mw-revslider-arrow-forwards' ] - } ); - - const forwardArrowPopup = new OO.ui.PopupWidget( { - $content: $( '

' ).text( mw.msg( 'revisionslider-arrow-tooltip-newer' ) ), - $floatableContainer: forwardArrowButton.$element, - width: 200, - classes: [ 'mw-revslider-tooltip', 'mw-revslider-arrow-tooltip' ] - } ); - - forwardArrowButton.connect( this, { - click: [ 'arrowClickHandler', forwardArrowButton ] - } ); - - forwardArrowButton.$element - .attr( 'data-dir', 1 ) - .children().attr( 'aria-label', mw.msg( 'revisionslider-arrow-tooltip-newer' ) ) - .on( 'mouseover', { button: forwardArrowButton, popup: forwardArrowPopup }, this.showPopup ) - .on( 'mouseout', { popup: forwardArrowPopup }, this.hidePopup ) - .on( 'focusin', { button: forwardArrowButton }, this.arrowFocusHandler ); - - $( document.body ).append( forwardArrowPopup.$element ); - - return forwardArrowButton; + return button; }, showPopup: function ( e ) { - const button = e.data.button, - popup = e.data.popup; - if ( typeof button !== 'undefined' && button.isDisabled() ) { + if ( e.data.button.isDisabled() ) { return; } + + const $button = $( this ); + const popup = e.data.popup; popup.$element.css( { - left: $( this ).offset().left + $( this ).outerWidth() / 2 + 'px', - top: $( this ).offset().top + $( this ).outerHeight() + 'px' + left: $button.offset().left + $button.outerWidth() / 2 + 'px', + top: $button.offset().top + $button.outerHeight() + 'px' } ); popup.toggle( true ); }, hidePopup: function ( e ) { - const popup = e.data.popup; - popup.toggle( false ); + e.data.popup.toggle( false ); }, /** diff --git a/modules/ext.RevisionSlider.SliderView.js b/modules/ext.RevisionSlider.SliderView.js index d6a946aa..1ef3e082 100644 --- a/modules/ext.RevisionSlider.SliderView.js +++ b/modules/ext.RevisionSlider.SliderView.js @@ -82,8 +82,8 @@ $.extend( SliderView.prototype, { this.pointerOlder = this.pointerOlder || new Pointer( 'mw-revslider-pointer-older' ); this.pointerNewer = this.pointerNewer || new Pointer( 'mw-revslider-pointer-newer' ); - this.backwardArrowButton = sliderArrowView.renderBackwardArrow(); - this.forwardArrowButton = sliderArrowView.renderForwardArrow(); + this.backwardArrowButton = sliderArrowView.renderArrowButton( -1 ); + this.forwardArrowButton = sliderArrowView.renderArrowButton( 1 ); this.$element = $( '

' ) .addClass( 'mw-revslider-revision-slider' )