From fe8f8fa05dca47bc88f701b826a1802b46cfa998 Mon Sep 17 00:00:00 2001 From: thiemowmde Date: Fri, 25 Aug 2023 16:23:18 +0200 Subject: [PATCH] Don't add keypress handler when not needed I'm not sure why it was done this way. Probably because it doesn't make an actual difference from the user's perspective. My motivation is: When we already called the code that auto-expands the RevisionSlider UI then it doesn't make much sense to give the user a keyypress handler that does the same a second time. Possibly even related to T342556? This patch also contains a few small, unrelated code cleanups. Change-Id: I123e89d9d7dc3b1e33cf43831c679330d9dd1cdd --- modules/ext.RevisionSlider.Settings.js | 6 ++---- modules/ext.RevisionSlider.SliderView.js | 11 +++-------- modules/ext.RevisionSlider.lazy.js | 21 ++++++++++----------- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/modules/ext.RevisionSlider.Settings.js b/modules/ext.RevisionSlider.Settings.js index 1b21c246..dcf25066 100644 --- a/modules/ext.RevisionSlider.Settings.js +++ b/modules/ext.RevisionSlider.Settings.js @@ -62,10 +62,8 @@ $.extend( Settings.prototype, { if ( mw.user.isNamed() ) { setting = mw.user.options.get( 'userjs-revslider-' + name ); } else { - setting = mw.storage.get( 'mw-revslider-' + name ); - if ( !setting ) { - setting = mw.cookie.get( '-revslider-' + name ); - } + setting = mw.storage.get( 'mw-revslider-' + name ) || + mw.cookie.get( '-revslider-' + name ); } return setting !== null && setting !== false ? setting : defaultValue; diff --git a/modules/ext.RevisionSlider.SliderView.js b/modules/ext.RevisionSlider.SliderView.js index b22fd526..98530971 100644 --- a/modules/ext.RevisionSlider.SliderView.js +++ b/modules/ext.RevisionSlider.SliderView.js @@ -769,16 +769,11 @@ $.extend( SliderView.prototype, { }, setSliderLineCSS: function ( $lineContainer, widthToSet, marginToSet ) { + $lineContainer.css( 'width', widthToSet ); if ( this.dir === 'ltr' ) { - $lineContainer.css( { - width: widthToSet, - 'margin-left': marginToSet - } ); + $lineContainer.css( 'margin-left', marginToSet ); } else { - $lineContainer.css( { - width: widthToSet, - 'margin-right': marginToSet + this.revisionWidth - } ); + $lineContainer.css( 'margin-right', marginToSet + this.revisionWidth ); } }, diff --git a/modules/ext.RevisionSlider.lazy.js b/modules/ext.RevisionSlider.lazy.js index 27b7592e..cda06b56 100644 --- a/modules/ext.RevisionSlider.lazy.js +++ b/modules/ext.RevisionSlider.lazy.js @@ -1,19 +1,18 @@ const Settings = require( 'ext.RevisionSlider.Settings' ), - settings = new Settings(), - autoExpand = settings.shouldAutoExpand(); + autoExpand = new Settings().shouldAutoExpand(); if ( autoExpand ) { mw.loader.load( 'ext.RevisionSlider.init' ); } else { - $( '.mw-revslider-toggle-button' ).on( 'click', - function () { + $( '.mw-revslider-toggle-button' ).on( { + click: function () { mw.loader.load( 'ext.RevisionSlider.init' ); + }, + keypress: function ( event ) { + if ( event.which === 13 || event.which === 32 ) { + event.preventDefault(); + $( '.mw-revslider-toggle-button' ).trigger( 'click' ); + } } - ); + } ); } -$( '.mw-revslider-toggle-button' ).on( 'keypress', function ( event ) { - if ( event.which === 13 || event.which === 32 ) { - event.preventDefault(); - $( '.mw-revslider-toggle-button' ).trigger( 'click' ); - } -} );