jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/338181 )
Change subject: PopupWidget (and similar): Document why it is unwise to show unattached widgets, and emit warnings ...................................................................... PopupWidget (and similar): Document why it is unwise to show unattached widgets, and emit warnings This should help prevent future problems similar to T158141. A deprecation warning is emitted once per widget. The ones in ClippableElement and FloatableElement would be sufficient, but PopupWidget and MenuSelectWidget have extra ones to avoid confusing theirs users with the clippable/floatable internals. In the future, the deprecation warnings should be changed to explicitly throwing an exception. But since this may be happening in uncommon code paths, we should probably keep them for a few releases. Change-Id: I4153635b1d8853ca460a526cbfe63a98bd102235 --- M src/mixins/ClippableElement.js M src/mixins/FloatableElement.js M src/widgets/MenuSelectWidget.js M src/widgets/PopupWidget.js 4 files changed, 40 insertions(+), 0 deletions(-) Approvals: jenkins-bot: Verified Jforrester: Looks good to me, approved diff --git a/src/mixins/ClippableElement.js b/src/mixins/ClippableElement.js index 2a716fe..ae07d1d 100644 --- a/src/mixins/ClippableElement.js +++ b/src/mixins/ClippableElement.js @@ -94,6 +94,11 @@ OO.ui.mixin.ClippableElement.prototype.toggleClipping = function ( clipping ) { clipping = clipping === undefined ? !this.clipping : !!clipping; + if ( clipping && !this.warnedUnattached && !this.isElementAttached() ) { + OO.ui.warnDeprecation( 'ClippableElement#toggleClipping: Before calling this method, the element must be attached to the DOM.' ); + this.warnedUnattached = true; + } + if ( this.clipping !== clipping ) { this.clipping = clipping; if ( clipping ) { diff --git a/src/mixins/FloatableElement.js b/src/mixins/FloatableElement.js index f5e0c71..360bdab 100644 --- a/src/mixins/FloatableElement.js +++ b/src/mixins/FloatableElement.js @@ -85,6 +85,11 @@ positioning = positioning === undefined ? !this.positioning : !!positioning; + if ( positioning && !this.warnedUnattached && !this.isElementAttached() ) { + OO.ui.warnDeprecation( 'FloatableElement#togglePositioning: Before calling this method, the element must be attached to the DOM.' ); + this.warnedUnattached = true; + } + if ( this.positioning !== positioning ) { this.positioning = positioning; diff --git a/src/widgets/MenuSelectWidget.js b/src/widgets/MenuSelectWidget.js index b3d3a9c..615f29a 100644 --- a/src/widgets/MenuSelectWidget.js +++ b/src/widgets/MenuSelectWidget.js @@ -16,6 +16,8 @@ * - Down-arrow key: highlight the next menu option * - Esc key: hide the menu * + * Unlike most widgets, MenuSelectWidget is initially hidden and must be shown by calling #toggle. + * * Please see the [OOjs UI documentation on MediaWiki][1] for more information. * [1]: https://www.mediawiki.org/wiki/OOjs_UI/Widgets/Selects_and_Options * @@ -256,6 +258,14 @@ }; /** + * Toggle visibility of the menu. The menu is initially hidden and must be shown by calling + * `.toggle( true )` after its #$element is attached to the DOM. + * + * Do not show the menu while it is not attached to the DOM. The calculations required to display + * it in the right place and with the right dimensions only work correctly while it is attached. + * Side-effects may include broken interface and exceptions being thrown. This wasn't always + * strictly enforced, so currently it only generates a warning in the browser console. + * * @inheritdoc */ OO.ui.MenuSelectWidget.prototype.toggle = function ( visible ) { @@ -264,6 +274,11 @@ visible = ( visible === undefined ? !this.visible : !!visible ) && !!this.items.length; change = visible !== this.isVisible(); + if ( visible && !this.warnedUnattached && !this.isElementAttached() ) { + OO.ui.warnDeprecation( 'MenuSelectWidget#toggle: Before calling this method, the menu must be attached to the DOM.' ); + this.warnedUnattached = true; + } + // Parent method OO.ui.MenuSelectWidget.parent.prototype.toggle.call( this, visible ); diff --git a/src/widgets/PopupWidget.js b/src/widgets/PopupWidget.js index 00ca1a2..d11961d 100644 --- a/src/widgets/PopupWidget.js +++ b/src/widgets/PopupWidget.js @@ -3,6 +3,8 @@ * By default, each popup has an anchor that points toward its origin. * Please see the [OOjs UI documentation on Mediawiki] [1] for more information and examples. * + * Unlike most widgets, PopupWidget is initially hidden and must be shown by calling #toggle. + * * @example * // A popup widget. * var popup = new OO.ui.PopupWidget( { @@ -244,6 +246,14 @@ }; /** + * Toggle visibility of the popup. The popup is initially hidden and must be shown by calling + * `.toggle( true )` after its #$element is attached to the DOM. + * + * Do not show the popup while it is not attached to the DOM. The calculations required to display + * it in the right place and with the right dimensions only work correctly while it is attached. + * Side-effects may include broken interface and exceptions being thrown. This wasn't always + * strictly enforced, so currently it only generates a warning in the browser console. + * * @inheritdoc */ OO.ui.PopupWidget.prototype.toggle = function ( show ) { @@ -252,6 +262,11 @@ change = show !== this.isVisible(); + if ( show && !this.warnedUnattached && !this.isElementAttached() ) { + OO.ui.warnDeprecation( 'PopupWidget#toggle: Before calling this method, the popup must be attached to the DOM.' ); + this.warnedUnattached = true; + } + // Parent method OO.ui.PopupWidget.parent.prototype.toggle.call( this, show ); -- To view, visit https://gerrit.wikimedia.org/r/338181 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4153635b1d8853ca460a526cbfe63a98bd102235 Gerrit-PatchSet: 1 Gerrit-Project: oojs/ui Gerrit-Branch: master Gerrit-Owner: Bartosz DziewoĆski <matma....@gmail.com> Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits