jenkins-bot has submitted this change and it was merged.
Change subject: Fallback options for receiving bad information into Scalable
object
......................................................................
Fallback options for receiving bad information into Scalable object
The Scalable object is updated with information from the API request
needed for original dimensions. Most of the code then checks whether
we have original dimensions before proceeding to use it.
However, when an API response returns invalid dimensions, the
dimensions object might be set to { width: undefined } which is then
processed as having dimensions object. Strictly speaking, however,
this is not a valid dimensions object and shouldn't be considered
as such.
This wrecked havoc on the calculations done with the aspect ratio,
and specifically the MediaSizeWidget's 'full size' and 'default'
buttons.
Changes made:
* Added a 'isDimensionsObjectValid' method to ve.dm.Scalable that
tests dimension objects specifically to see if they are valid
before update is done to any of the stored dimension values.
* Added originalSizeChange event to ve.dm.Scalable and a listener
in ve.ui.MediaSizeWidget to make sure that the 'full size'
and the 'default' buttons are only enabled when original
dimensions are acquired, even if the API response is slow or was
completed after the widget is already loaded.
* Fixed small typos in setDefaultDimensions in ve.dm.Scalable
Change-Id: I1242e8dadca4f2cf2737c366034a77edc5ce308e
---
M modules/ve/dm/ve.dm.Scalable.js
M modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
2 files changed, 91 insertions(+), 13 deletions(-)
Approvals:
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/dm/ve.dm.Scalable.js b/modules/ve/dm/ve.dm.Scalable.js
index 6274b4a..f43367b 100644
--- a/modules/ve/dm/ve.dm.Scalable.js
+++ b/modules/ve/dm/ve.dm.Scalable.js
@@ -76,6 +76,13 @@
*/
/**
+ * Original size changed
+ *
+ * @event originalSizeChange
+ * @param {Object} Original dimensions width and height
+ */
+
+/**
* Min size changed
*
* @event minSizeChange
@@ -111,7 +118,7 @@
* @param {Object} dimensions Dimensions object with width & height
*/
ve.dm.Scalable.prototype.setCurrentDimensions = function ( dimensions ) {
- if ( dimensions ) {
+ if ( this.isDimensionsObjectValid( dimensions ) ) {
this.currentDimensions = ve.copy( dimensions );
// Only use current dimensions for ratio if it isn't set
if ( this.fixedRatio && !this.ratio ) {
@@ -127,14 +134,24 @@
* Also resets the aspect ratio if in fixed ratio mode.
*
* @param {Object} dimensions Dimensions object with width & height
+ * @fires originalSizeChange
*/
ve.dm.Scalable.prototype.setOriginalDimensions = function ( dimensions ) {
- this.originalDimensions = ve.copy( dimensions );
- // Always overwrite ratio
- if ( this.fixedRatio ) {
- this.setRatioFromDimensions( this.getOriginalDimensions() );
+ if (
+ !this.originalDimensions ||
+ (
+ this.isDimensionsObjectValid( dimensions ) &&
+ !ve.compare( this.originalDimensions, dimensions )
+ )
+ ) {
+ this.originalDimensions = ve.copy( dimensions );
+ // Always overwrite ratio
+ if ( this.fixedRatio ) {
+ this.setRatioFromDimensions(
this.getOriginalDimensions() );
+ }
+ this.valid = null;
+ this.emit( 'originalSizeChange', this.getOriginalDimensions() );
}
- this.valid = null;
};
/**
@@ -144,7 +161,13 @@
* @fires defaultSizeChange
*/
ve.dm.Scalable.prototype.setDefaultDimensions = function ( dimensions ) {
- if ( !this.defaultDimensions || !ve.compare( this.defaultDimensions,
dimensions ) ) {
+ if (
+ !this.defaultDimensions ||
+ (
+ this.isDimensionsObjectValid( dimensions ) &&
+ !ve.compare( this.defaultDimensions, dimensions )
+ )
+ ) {
this.defaultDimensions = ve.copy( dimensions );
this.valid = null;
this.emit( 'defaultSizeChange', this.isDefault() );
@@ -179,7 +202,13 @@
* @fires minSizeChange
*/
ve.dm.Scalable.prototype.setMinDimensions = function ( dimensions ) {
- if ( !this.minDimensions || !ve.compare( this.minDimensions, dimensions
) ) {
+ if (
+ !this.minDimensions ||
+ (
+ this.isDimensionsObjectValid( dimensions ) &&
+ !ve.compare( this.minDimensions, dimensions )
+ )
+ ) {
this.minDimensions = ve.copy( dimensions );
this.valid = null;
this.emit( 'minSizeChange', dimensions );
@@ -193,7 +222,13 @@
* @fires maxSizeChange
*/
ve.dm.Scalable.prototype.setMaxDimensions = function ( dimensions ) {
- if ( !this.maxDimensions || !ve.compare( this.maxDimensions, dimensions
) ) {
+ if (
+ !this.maxDimensions ||
+ (
+ this.isDimensionsObjectValid( dimensions ) &&
+ !ve.compare( this.maxDimensions, dimensions )
+ )
+ ) {
this.maxDimensions = ve.copy( dimensions );
this.emit( 'maxSizeChange', dimensions );
this.valid = null;
@@ -383,7 +418,7 @@
* calculations.
*
* @param {Object} dimensions Dimensions object with either width or height
- * if both are given, the object will be returned as-is.
+ * if both are given, the object will be returned as-is.
* @param {number} [dimensions.width] The width of the image
* @param {number} [dimensions.height] The height of the image
* @returns {Object} Dimensions object with width and height
@@ -493,3 +528,24 @@
);
return this.valid;
};
+
+/**
+ * Check if an object is a dimensions object.
+ * Make sure that if width or height are set, they are not 'undefined'.
+ *
+ * @param {Object} dimensions A dimensions object to test
+ * @returns {boolean} Valid or invalid dimensions object
+ */
+ve.dm.Scalable.prototype.isDimensionsObjectValid = function ( dimensions ) {
+ if (
+ dimensions &&
+ !$.isEmptyObject( dimensions ) &&
+ (
+ dimensions.width !== undefined ||
+ dimensions.height !== undefined
+ )
+ ) {
+ return true;
+ }
+ return false;
+};
diff --git a/modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
b/modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
index 1fa98b2..258b8af 100644
--- a/modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
+++ b/modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
@@ -44,8 +44,8 @@
} ),
// TODO: when upright is supported by Parsoid
// new OO.ui.ButtonOptionWidget( 'scale', {
- // '$': this.$,
- // 'label': ve.msg(
'visualeditor-mediasizewidget-sizeoptions-scale' )
+ // '$': this.$,
+ // 'label': ve.msg(
'visualeditor-mediasizewidget-sizeoptions-scale' )
// } ),
new OO.ui.ButtonOptionWidget( 'custom', {
'$': this.$,
@@ -146,6 +146,22 @@
/* Methods */
+/**
+ * Respond to change in original dimensions in the scalable object.
+ * Specifically, enable or disable to 'set full size' button and the 'default'
option.
+ *
+ * @param {Object} dimensions Original dimensions
+ */
+ve.ui.MediaSizeWidget.prototype.onScalableOriginalSizeChange = function (
dimensions ) {
+ var disabled = !dimensions || $.isEmptyObject( dimensions );
+ this.fullSizeButton.setDisabled( disabled );
+ this.sizeTypeSelectWidget.getItemFromData( 'default' ).setDisabled(
disabled );
+};
+
+/**
+ * Respond to default size or status change in the scalable object.
+ * @param {Boolean} isDefault Current default state
+ */
ve.ui.MediaSizeWidget.prototype.onScalableDefaultSizeChange = function (
isDefault ) {
// Update the default size into the dimensions widget
this.updateDefaultDimensions();
@@ -276,7 +292,11 @@
}
this.scalable = scalable;
// Events
- this.scalable.connect( this, { 'defaultSizeChange':
'onScalableDefaultSizeChange' } );
+ this.scalable.connect( this, {
+ 'defaultSizeChange': 'onScalableDefaultSizeChange',
+ 'originalSizeChange': 'onScalableOriginalSizeChange'
+ } );
+
// Reset current dimensions to new scalable object
this.setCurrentDimensions( this.scalable.getCurrentDimensions() );
this.updateDefaultDimensions();
@@ -284,8 +304,10 @@
// If we don't have original dimensions, disable the full size button
if ( !this.scalable.getOriginalDimensions() ) {
this.fullSizeButton.setDisabled( true );
+ this.sizeTypeSelectWidget.getItemFromData( 'default'
).setDisabled( true );
} else {
this.fullSizeButton.setDisabled( false );
+ this.sizeTypeSelectWidget.getItemFromData( 'default'
).setDisabled( false );
}
};
--
To view, visit https://gerrit.wikimedia.org/r/125428
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1242e8dadca4f2cf2737c366034a77edc5ce308e
Gerrit-PatchSet: 8
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
