Mooeypoo has uploaded a new change for review.
https://gerrit.wikimedia.org/r/125428
Change subject: Small validation fixes to scalable
......................................................................
Small validation fixes to scalable
There was an issue where scalable dimensions may have been set as
{ width: undefined } which made the tests in most 'set' methods
pass probperly, because this wasn't an empty object. However,
practically speaking this is an empty dimensions object, and it
should be considered as if it isn't valid.
This wrecked havoc on the calculations done with the aspect ratio,
and affected 'default dimensions' most of all.
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'
button is 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, 81 insertions(+), 9 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/28/125428/1
diff --git a/modules/ve/dm/ve.dm.Scalable.js b/modules/ve/dm/ve.dm.Scalable.js
index ca4b145..51035ac 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.dimensions || !ve.compare( this.dimensions, 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;
@@ -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 ||
+ dimensions.height
+ )
+ ) {
+ 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 1075bc8..cbb1352 100644
--- a/modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
+++ b/modules/ve/ui/widgets/ve.ui.MediaSizeWidget.js
@@ -146,6 +146,20 @@
/* Methods */
+/**
+ * Respond to change in original dimensions in the scalable object.
+ * Specifically, enable or disable to 'set full size' button.
+ *
+ * @param {Object} dimensions Original dimensions
+ */
+ve.ui.MediaSizeWidget.prototype.onScalableOriginalSizeChange = function (
dimensions ) {
+ this.fullSizeButton.setDisabled( ( !dimensions || $.isEmptyObject(
dimensions ) ) );
+};
+
+/**
+ * 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();
@@ -277,6 +291,8 @@
this.scalable = scalable;
// Events
this.scalable.connect( this, { 'defaultSizeChange':
'onScalableDefaultSizeChange' } );
+ this.scalable.connect( this, { 'originalSizeChange':
'onScalableOriginalSizeChange' } );
+
// Reset current dimensions to new scalable object
this.setCurrentDimensions( this.scalable.getCurrentDimensions() );
--
To view, visit https://gerrit.wikimedia.org/r/125428
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1242e8dadca4f2cf2737c366034a77edc5ce308e
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <mor...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
