From 759c081addbb71ff8bfbe93760532362fb73b67e Mon Sep 17 00:00:00 2001 From: thiemowmde Date: Wed, 21 Feb 2024 18:40:07 +0100 Subject: [PATCH] Drop separate .render/.initialize logic from View classes Most callers use it as if it's a `getElement` call anyway. There is one .initialize method left in the HelpDialog class. That's part of the upstream OOUI Window interfaces. Change-Id: I5727c59ad0ad05d712d51d255906ddc44e898668 --- modules/ext.RevisionSlider.PointerLine.js | 15 +++++------ modules/ext.RevisionSlider.PointerView.js | 27 +++++-------------- modules/ext.RevisionSlider.SliderView.js | 6 ++--- tests/qunit/RevisionSlider.Pointer.test.js | 2 +- .../qunit/RevisionSlider.PointerView.test.js | 4 +-- 5 files changed, 18 insertions(+), 36 deletions(-) diff --git a/modules/ext.RevisionSlider.PointerLine.js b/modules/ext.RevisionSlider.PointerLine.js index b370d55a..0d22b12c 100644 --- a/modules/ext.RevisionSlider.PointerLine.js +++ b/modules/ext.RevisionSlider.PointerLine.js @@ -132,6 +132,8 @@ $.extend( PointerLine.prototype, { /** * Initializes the DOM element with the line-box for drawing the lines + * + * @private */ initialize: function () { // eslint-disable-next-line mediawiki/class-doc @@ -179,19 +181,14 @@ $.extend( PointerLine.prototype, { mw.hook( 'revslider.collapse' ).add( this.removeColoredColumnBorders.bind( this ) ); }, - /** - * @return {jQuery} - */ - render: function () { - this.initialize(); - this.setColumnBorderHooks(); - return this.getElement(); - }, - /** * @return {jQuery} */ getElement: function () { + if ( !this.$html ) { + this.initialize(); + this.setColumnBorderHooks(); + } return this.$html; } diff --git a/modules/ext.RevisionSlider.PointerView.js b/modules/ext.RevisionSlider.PointerView.js index f6b43af5..643e7b64 100644 --- a/modules/ext.RevisionSlider.PointerView.js +++ b/modules/ext.RevisionSlider.PointerView.js @@ -27,27 +27,15 @@ $.extend( PointerView.prototype, { */ $html: null, - /** - * Initializes the DOM element - */ - initialize: function () { - // eslint-disable-next-line mediawiki/class-doc - this.$html = $( '
' ) - .addClass( 'mw-revslider-pointer mw-revslider-pointer-cursor ' + this.name ); - }, - /** * @return {jQuery} */ - render: function () { - this.initialize(); - return this.$html; - }, - - /** - * @return {jQuery|null} Null if not initialized - */ getElement: function () { + if ( !this.$html ) { + // eslint-disable-next-line mediawiki/class-doc + this.$html = $( '
' ) + .addClass( 'mw-revslider-pointer mw-revslider-pointer-cursor ' + this.name ); + } return this.$html; }, @@ -72,10 +60,7 @@ $.extend( PointerView.prototype, { * @param {number} pos */ setDataPositionAttribute: function ( pos ) { - if ( !this.$html ) { - this.initialize(); - } - this.$html.attr( 'data-pos', pos ); + this.getElement().attr( 'data-pos', pos ); }, /** diff --git a/modules/ext.RevisionSlider.SliderView.js b/modules/ext.RevisionSlider.SliderView.js index 4466e164..504b09aa 100644 --- a/modules/ext.RevisionSlider.SliderView.js +++ b/modules/ext.RevisionSlider.SliderView.js @@ -97,7 +97,7 @@ $.extend( SliderView.prototype, { this.renderPointerContainer( containerWidth ), this.forwardArrowButton.$element, $( '
' ).css( { clear: 'both' } ), - this.pointerOlder.getLine().render(), this.pointerNewer.getLine().render(), + this.pointerOlder.getLine().getElement(), this.pointerNewer.getLine().getElement(), HelpButtonView.render() ); @@ -193,13 +193,13 @@ $.extend( SliderView.prototype, { .addClass( 'mw-revslider-pointer-container-older' ) .append( $( '
' ).addClass( 'mw-revslider-slider-line' ), - this.pointerOlder.getView().render() + this.pointerOlder.getView().getElement() ), $( '
' ) .addClass( 'mw-revslider-pointer-container-newer' ) .append( $( '
' ).addClass( 'mw-revslider-slider-line' ), - this.pointerNewer.getView().render() + this.pointerNewer.getView().getElement() ) ]; }, diff --git a/tests/qunit/RevisionSlider.Pointer.test.js b/tests/qunit/RevisionSlider.Pointer.test.js index 964fcc02..abb2c4ea 100644 --- a/tests/qunit/RevisionSlider.Pointer.test.js +++ b/tests/qunit/RevisionSlider.Pointer.test.js @@ -4,7 +4,7 @@ QUnit.module( 'ext.RevisionSlider.Pointer' ); QUnit.test( 'Initialize Pointer', function ( assert ) { - assert.true( ( new Pointer( 'mw-revslider-pointer' ) ).getView().render().hasClass( 'mw-revslider-pointer' ) ); + assert.true( ( new Pointer( 'mw-revslider-pointer' ) ).getView().getElement().hasClass( 'mw-revslider-pointer' ) ); } ); QUnit.test( 'Set and get position', function ( assert ) { diff --git a/tests/qunit/RevisionSlider.PointerView.test.js b/tests/qunit/RevisionSlider.PointerView.test.js index 80a66f3b..2d1e8ded 100644 --- a/tests/qunit/RevisionSlider.PointerView.test.js +++ b/tests/qunit/RevisionSlider.PointerView.test.js @@ -4,12 +4,12 @@ QUnit.module( 'ext.RevisionSlider.PointerView' ); QUnit.test( 'Initialize PointerView', function ( assert ) { - assert.true( ( new PointerView( null, 'mw-revslider-pointer' ) ).render().hasClass( 'mw-revslider-pointer' ) ); + assert.true( ( new PointerView( null, 'mw-revslider-pointer' ) ).getElement().hasClass( 'mw-revslider-pointer' ) ); } ); QUnit.test( 'Is newer pointer', function ( assert ) { const pv = new PointerView( null, 'mw-revslider-pointer' ); - pv.render(); + pv.getElement(); assert.false( pv.isNewerPointer() ); pv.getElement().addClass( 'mw-revslider-pointer-newer' );