From 5eb1193670da29fd9a3a8f72a78f1040a822738f Mon Sep 17 00:00:00 2001 From: Thiemo Kreuz Date: Tue, 2 Aug 2022 11:08:23 +0200 Subject: [PATCH] Tweaks to focus/scrolling code relative to sticky header * Rename method because it turns out it is not only about the sticky header, but also relevant when there is no header. * Move some code to more appropriate places. * Use 0 as documented in OOUI, not null. * Set the padding back to 0 when the sticky header is not visible. As of now this is an unreachable state because the filters never go away after they have been made visible. Still this code was always written with this possibility in mind to make it future-proof. * Performance optimization for the boolean "show filters?" check. Bug: T312926 Change-Id: Iaba08ccd8bf00360fd26f9268d5be43df4f4fbd8 --- ...ransclusionOutlineParameterSelectWidget.js | 18 +++++------ ...ui.MWTransclusionOutlineParameterWidget.js | 6 ++-- ....ui.MWTransclusionOutlineTemplateWidget.js | 31 +++++++------------ 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterSelectWidget.js b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterSelectWidget.js index 896e82df6c..d1c84b1997 100644 --- a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterSelectWidget.js +++ b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterSelectWidget.js @@ -15,7 +15,7 @@ * @param {Object} config * @cfg {ve.ui.MWTransclusionOutlineParameterWidget[]} items * @property {string|null} activeParameter Name of the currently selected parameter - * @property {number|null} stickyHeaderHeight + * @property {number} stickyHeaderHeight */ ve.ui.MWTransclusionOutlineParameterSelectWidget = function VeUiMWTransclusionOutlineParameterSelectWidget( config ) { // Parent constructor @@ -37,7 +37,7 @@ ve.ui.MWTransclusionOutlineParameterSelectWidget = function VeUiMWTransclusionOu } ); this.activeParameter = null; - this.stickyHeaderHeight = null; + this.stickyHeaderHeight = 0; }; /* Inheritance */ @@ -92,12 +92,12 @@ ve.ui.MWTransclusionOutlineParameterSelectWidget.prototype.addItems = function ( return this; }; -/** - * @return {ve.ui.MWTransclusionOutlineParameterWidget|null} - */ -ve.ui.MWTransclusionOutlineParameterSelectWidget.prototype.findFirstSelectedItem = function () { - var selected = this.findSelectedItems(); - return Array.isArray( selected ) ? selected[ 0 ] || null : selected; +ve.ui.MWTransclusionOutlineParameterSelectWidget.prototype.ensureVisibilityOfFirstCheckedParameter = function () { + // TODO: Replace with {@see OO.ui.SelectWidget.findFirstSelectedItem} when available + var firstChecked = this.findSelectedItems()[ 0 ]; + if ( firstChecked ) { + firstChecked.ensureVisibility( this.stickyHeaderHeight ); + } }; /** @@ -130,7 +130,7 @@ ve.ui.MWTransclusionOutlineParameterSelectWidget.prototype.setActiveParameter = */ ve.ui.MWTransclusionOutlineParameterSelectWidget.prototype.highlightItem = function ( item ) { if ( item ) { - item.ensureVisibilityBelowStickyHeader( this.stickyHeaderHeight ); + item.ensureVisibility( this.stickyHeaderHeight ); } ve.ui.MWTransclusionOutlineParameterSelectWidget.super.prototype.highlightItem.call( this, item ); }; diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterWidget.js b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterWidget.js index a0c694ec72..c9d8966b25 100644 --- a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterWidget.js +++ b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineParameterWidget.js @@ -88,9 +88,9 @@ ve.ui.MWTransclusionOutlineParameterWidget.prototype.toggleHasValue = function ( /** * Custom method to scroll parameter into view respecting the sticky part that sits above * - * @param {number} stickyHeaderHeight + * @param {number} paddingTop */ -ve.ui.MWTransclusionOutlineParameterWidget.prototype.ensureVisibilityBelowStickyHeader = function ( stickyHeaderHeight ) { +ve.ui.MWTransclusionOutlineParameterWidget.prototype.ensureVisibility = function ( paddingTop ) { // make sure parameter is visible and scrolled underneath the sticky - this.scrollElementIntoView( { animate: false, padding: { top: stickyHeaderHeight } } ); + this.scrollElementIntoView( { animate: false, padding: { top: paddingTop } } ); }; diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineTemplateWidget.js b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineTemplateWidget.js index 6c2bf9b5c3..19e87474bc 100644 --- a/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineTemplateWidget.js +++ b/modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineTemplateWidget.js @@ -343,9 +343,10 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onParameterWidgetListChanged * @return {boolean} */ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.shouldFiltersBeShown = function () { - var numParams = this.getRelevantTemplateParameters().length, - visible = numParams >= this.constructor.static.searchableParameterCount; - return visible; + var min = this.constructor.static.searchableParameterCount, + existingParameterWidgets = this.parameterList && this.parameterList.getItemCount(); + // Avoid expensive calls when there are already enough items in the parameter list + return existingParameterWidgets >= min || this.getRelevantTemplateParameters().length >= min; }; /** @@ -359,10 +360,11 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.toggleFilters = function () } else if ( visible ) { this.initializeFilters(); this.updateUnusedParameterToggleState(); - if ( this.parameterList ) { - // TODO find a dynamic way to get height of the sticky part - this.parameterList.stickyHeaderHeight = 114; - } + } + + if ( this.parameterList ) { + // TODO find a dynamic way to get height of the sticky part + this.parameterList.stickyHeaderHeight = visible ? 114 : 0; } }; @@ -467,18 +469,9 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onToggleUnusedFields = funct } if ( !visibility && fromClick ) { - this.scrollTopIntoView(); - } -}; - -ve.ui.MWTransclusionOutlineTemplateWidget.prototype.scrollTopIntoView = function () { - this.header.scrollElementIntoView(); - - if ( this.parameterList ) { - var firstSelected = this.parameterList.findFirstSelectedItem(); - if ( firstSelected ) { - // FIXME: This code should be in {@see ve.ui.MWTransclusionOutlineParameterSelectWidget} - firstSelected.ensureVisibilityBelowStickyHeader( this.parameterList.stickyHeaderHeight ); + this.header.scrollElementIntoView(); + if ( this.parameterList ) { + this.parameterList.ensureVisibilityOfFirstCheckedParameter(); } } };