From 8c835588f24a35fa8ca4f4e08b6f207d02db4254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20Laxstr=C3=B6m?= Date: Mon, 22 Mar 2021 10:23:51 +0100 Subject: [PATCH] Fix positioning of input/display settings for new language selector Unrelated behavior change: ULS language selection dialog is hidden while display or input settings is open. Also simplified dead code in addDisplaySettings. Bug: T276248 Change-Id: Ia91a2b83e7ad4072016649230e2376b0793cbbff --- resources/js/ext.uls.displaysettings.js | 14 +----- resources/js/ext.uls.interface.js | 54 ++++++++---------------- resources/js/ext.uls.languagesettings.js | 17 +++++--- resources/js/ext.uls.launch.js | 37 +++++++--------- 4 files changed, 46 insertions(+), 76 deletions(-) diff --git a/resources/js/ext.uls.displaysettings.js b/resources/js/ext.uls.displaysettings.js index 04dffc23..3e532bac 100644 --- a/resources/js/ext.uls.displaysettings.js +++ b/resources/js/ext.uls.displaysettings.js @@ -302,8 +302,7 @@ $languages.append( $moreLanguagesButton ); // Show the long language list to select a language for display settings $moreLanguagesButton.uls( { - left: displaySettings.$parent.left, - top: displaySettings.$parent.top, + onPosition: this.$parent.position.bind( this.$parent ), onReady: function () { var $wrap, uls = this, @@ -328,8 +327,6 @@ uls.$menu.toggleClass( 'selector-right', displaySettings.$parent.$window.hasClass( 'selector-right' ) ); }, onVisible: function () { - var $parent; - this.$menu.find( '.uls-languagefilter' ) .prop( 'placeholder', $.i18n( 'ext-uls-display-settings-ui-language' ) ); @@ -340,15 +337,6 @@ return; } - $parent = $( '#language-settings-dialog' ); - - // Re-position the element according to the window that called it - if ( parseInt( $parent.css( 'left' ), 10 ) ) { - this.$menu.css( 'left', $parent.css( 'left' ) ); - } - if ( parseInt( $parent.css( 'top' ), 10 ) ) { - this.$menu.css( 'top', $parent.css( 'top' ) ); - } // If the ULS is shown in the sidebar, // add a caret pointing to the icon // eslint-disable-next-line no-jquery/no-class-state diff --git a/resources/js/ext.uls.interface.js b/resources/js/ext.uls.interface.js index 6a0951d8..cc46f484 100644 --- a/resources/js/ext.uls.interface.js +++ b/resources/js/ext.uls.interface.js @@ -74,27 +74,14 @@ var $displaySettings = displaySettings(); uls.$menu.find( '#uls-settings-block' ).append( $displaySettings ); - // Initialize the trigger $displaySettings.one( 'click', function () { - var displaySettingsOptions = { - defaultModule: 'display' - }, - ulsPosition = mw.config.get( 'wgULSPosition' ), - anonMode = ( mw.user.isAnon() && - !mw.config.get( 'wgULSAnonCanChangeLanguage' ) ); - - // If the ULS trigger is shown in the top personal menu, - // closing the display settings must show the main ULS - // languages list, unless we are in anon mode and thus - // cannot show the language list - if ( ulsPosition === 'personal' && !anonMode ) { - displaySettingsOptions.onClose = function () { - uls.show(); - }; - } - $.extend( displaySettingsOptions, uls.position() ); - $displaySettings.languagesettings( displaySettingsOptions ).trigger( 'click' ); + $displaySettings.languagesettings( { + defaultModule: 'display', + onClose: uls.show.bind( uls ), + onPosition: uls.position.bind( uls ), + onVisible: uls.hide.bind( uls ) + } ).trigger( 'click' ); } ); } @@ -107,20 +94,14 @@ var $inputSettings = inputSettings(); uls.$menu.find( '#uls-settings-block' ).append( $inputSettings ); - // Initialize the trigger $inputSettings.one( 'click', function () { - var position = uls.position(); - $inputSettings.languagesettings( { defaultModule: 'input', - onClose: function () { - uls.show(); - }, - top: position.top, - left: position.left + onClose: uls.show.bind( uls ), + onPosition: uls.position.bind( uls ), + onVisible: uls.hide.bind( uls ) } ).trigger( 'click' ); - } ); } @@ -305,33 +286,34 @@ // Initialize the Language settings window languageSettingsOptions = { defaultModule: 'display', - onVisible: function () { - var caretRadius, + onPosition: function () { + var caretRadius, top, left, ulsTriggerHeight = this.$element.height(), ulsTriggerWidth = this.$element[ 0 ].offsetWidth, ulsTriggerOffset = this.$element.offset(); - this.$window.addClass( 'callout' ); - // Same as border width in mixins.less, or near enough caretRadius = 12; if ( ulsTriggerOffset.left > $( window ).width() / 2 ) { - this.left = ulsTriggerOffset.left - this.$window.width() - caretRadius; + left = ulsTriggerOffset.left - this.$window.width() - caretRadius; this.$window.removeClass( 'selector-left' ).addClass( 'selector-right' ); } else { - this.left = ulsTriggerOffset.left + ulsTriggerWidth + caretRadius; + left = ulsTriggerOffset.left + ulsTriggerWidth + caretRadius; this.$window.removeClass( 'selector-right' ).addClass( 'selector-left' ); } // The top of the dialog is aligned in relation to // the middle of the trigger, so that middle of the // caret aligns with it. 16 is trigger icon height in pixels - this.top = ulsTriggerOffset.top + + top = ulsTriggerOffset.top + ( ulsTriggerHeight / 2 ) - ( caretRadius + 16 ); - this.position(); + return { top: top, left: left }; + }, + onVisible: function () { + this.$window.addClass( 'callout' ); } }; diff --git a/resources/js/ext.uls.languagesettings.js b/resources/js/ext.uls.languagesettings.js index f5748e1b..064dbff9 100644 --- a/resources/js/ext.uls.languagesettings.js +++ b/resources/js/ext.uls.languagesettings.js @@ -174,12 +174,16 @@ }, position: function () { + if ( this.options.onPosition ) { + return this.options.onPosition.call( this ); + } + this.top = this.top || this.$element.offset().top + this.$element.outerHeight(); this.left = this.left || '25%'; - this.$window.css( { + return { top: this.top, left: this.left - } ); + }; }, i18n: function () { @@ -187,7 +191,7 @@ }, show: function () { - this.position(); + this.$window.css( this.position() ); if ( !this.initialized ) { this.render(); @@ -311,9 +315,10 @@ template: windowTemplate, defaultModule: false, // Name of the default module onClose: null, // An onClose event handler. - top: null, // Top position of this window - left: null, // Left position of this window - onVisible: null // A callback that runs after the ULS panel becomes visible + top: null, // DEPRECATED: Top position of this window + left: null, // DEPRECATED: Left position of this window + onVisible: null, // A callback that runs after the ULS panel becomes visible + onPosition: null // A callback that allows positioning the dialog }; $.fn.languagesettings.Constructor = LanguageSettings; diff --git a/resources/js/ext.uls.launch.js b/resources/js/ext.uls.launch.js index 98610eef..ce95e14d 100644 --- a/resources/js/ext.uls.launch.js +++ b/resources/js/ext.uls.launch.js @@ -62,14 +62,11 @@ function launchULS( $trigger, languagesObject, forCLS ) { } location.href = languagesObject[ language ].href; }, - onVisible: function () { + onPosition: function () { // Override the default positioning. See https://phabricator.wikimedia.org/T276248 // Default positioning of jquery.uls is middle of the screen under the trigger. // This code aligns it under the trigger and to the trigger edge depending on which // side of the page the trigger is - should work automatically for both LTR and RTL. - // - // FIXME: make position() overrideable in ULS to avoid doing this after the dialog - // has been made visible var offset, height, width; // These are for the trigger. offset = $trigger.offset(); @@ -78,22 +75,21 @@ function launchULS( $trigger, languagesObject, forCLS ) { if ( offset.left + ( width / 2 ) > $( window ).width() / 2 ) { // Midpoint of the trigger is on the right side of the viewport. - this.$menu.css( { - left: 'auto', + return { // Right edge of the dialog aligns with the right edge of the trigger. right: $( window ).width() - ( offset.left + width ), top: offset.top + height - } ); + }; } else { // Midpoint of the trigger is on the left side of the viewport. - this.$menu.css( { + return { // Left edge of the dialog aligns with the left edge of the trigger. left: offset.left, - right: 'auto', top: offset.top + height - } ); + }; } - + }, + onVisible: function () { $trigger.addClass( 'selector-open' ); }, languageDecorator: function ( $languageLink, language ) { @@ -138,9 +134,9 @@ function launchULS( $trigger, languagesObject, forCLS ) { // This class enables the caret this.$menu.addClass( 'interlanguage-uls-menu' ); }; - ulsConfig.onVisible = function () { + ulsConfig.onPosition = function () { // Compact language links specific positioning with a caret - var offset, height, width, triangleWidth; + var top, left, offset, height, width, triangleWidth; // The panel is positioned carefully so that our pointy triangle, // which is implemented as a square box rotated 45 degrees with // rotation origin in the middle. See the corresponding style file. @@ -156,20 +152,19 @@ function launchULS( $trigger, languagesObject, forCLS ) { // selector-{left,right} control which side the caret appears. // It needs to match the positioning of the dialog. if ( offset.left > $( window ).width() / 2 ) { - this.left = offset.left - this.$menu.outerWidth() - triangleWidth; + left = offset.left - this.$menu.outerWidth() - triangleWidth; this.$menu.removeClass( 'selector-left' ).addClass( 'selector-right' ); } else { - this.left = offset.left + width + triangleWidth; + left = offset.left + width + triangleWidth; this.$menu.removeClass( 'selector-right' ).addClass( 'selector-left' ); } // Offset from the middle of the trigger - this.top = offset.top + ( height / 2 ) - 27; + top = offset.top + ( height / 2 ) - 27; - this.$menu.css( { - left: this.left, - top: this.top - } ); - $trigger.addClass( 'selector-open' ); + return { + left: left, + top: top + }; }; }