Bartosz Dziewoński has uploaded a new change for review. (
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(-)
git pull ssh://gerrit.wikimedia.org:29418/oojs/ui refs/changes/81/338181/1
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: newchange
Gerrit-Change-Id: I4153635b1d8853ca460a526cbfe63a98bd102235
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
