Matthias Mullie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/322122
Change subject: Have clear load/unload methods
......................................................................
Have clear load/unload methods
* moveTo was not the clearest of method names, especially now that
we've replaced moveFrom with moveNext & movePrevious
* Having a single cleanup method (now unload) will be convenient for
when we just want to jump to anywhere else (e.g. resetting the
wizard) regardless of the direction. Plus it reduces duplicate
code, for when (parts of) handling is the same in previous & next.
Change-Id: I9ce7ebded7790749f2dca8440a1fccd139b32b99
---
M resources/controller/uw.controller.Deed.js
M resources/controller/uw.controller.Details.js
M resources/controller/uw.controller.Step.js
M resources/controller/uw.controller.Thanks.js
M resources/controller/uw.controller.Tutorial.js
M resources/controller/uw.controller.Upload.js
M resources/mw.UploadWizard.js
M resources/ui/steps/uw.ui.Deed.js
M resources/ui/steps/uw.ui.Details.js
M resources/ui/steps/uw.ui.Tutorial.js
M resources/ui/steps/uw.ui.Upload.js
M resources/ui/uw.ui.Step.js
M tests/qunit/controller/uw.controller.Deed.test.js
M tests/qunit/controller/uw.controller.Details.test.js
M tests/qunit/controller/uw.controller.Thanks.test.js
15 files changed, 68 insertions(+), 72 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard
refs/changes/22/322122/1
diff --git a/resources/controller/uw.controller.Deed.js
b/resources/controller/uw.controller.Deed.js
index ce348bf..e62773a 100644
--- a/resources/controller/uw.controller.Deed.js
+++ b/resources/controller/uw.controller.Deed.js
@@ -78,7 +78,7 @@
/**
* Move to this step.
*/
- uw.controller.Deed.prototype.moveTo = function ( uploads ) {
+ uw.controller.Deed.prototype.load = function ( uploads ) {
var customDeed, previousDeed, fromStepName,
showDeed = false,
step = this;
@@ -91,7 +91,7 @@
}
} );
- uw.controller.Step.prototype.moveTo.call( this, uploads );
+ uw.controller.Step.prototype.load.call( this, uploads );
// If all of the uploads are from URLs, then we know the
licenses
// already, we don't need this step.
diff --git a/resources/controller/uw.controller.Details.js
b/resources/controller/uw.controller.Details.js
index 2631a37..4fcb8f9 100644
--- a/resources/controller/uw.controller.Details.js
+++ b/resources/controller/uw.controller.Details.js
@@ -50,10 +50,10 @@
*
* @param {mw.UploadWizardUpload[]} uploads List of uploads being
carried forward.
*/
- uw.controller.Details.prototype.moveTo = function ( uploads ) {
+ uw.controller.Details.prototype.load = function ( uploads ) {
var details = this;
- uw.controller.Step.prototype.moveTo.call( this, uploads );
+ uw.controller.Step.prototype.load.call( this, uploads );
$.each( uploads, function ( i, upload ) {
var serialized;
@@ -89,18 +89,7 @@
} );
};
- uw.controller.Details.prototype.moveNext = function () {
- $.each( this.uploads, function ( i, upload ) {
- upload.off( 'remove-upload', upload.removeCallback );
- delete upload.removeCallback;
- } );
-
- this.removeErrorUploads();
-
- uw.controller.Step.prototype.moveNext.call( this );
- };
-
- uw.controller.Details.prototype.movePrevious = function () {
+ uw.controller.Details.prototype.unload = function () {
$.each( this.uploads, function ( i, upload ) {
// get rid of remove handler, this handler only makes
sense in this
// exact step - having it bound while in other steps
could cause
@@ -109,7 +98,13 @@
delete upload.removeCallback;
} );
- uw.controller.Step.prototype.movePrevious.call( this );
+ uw.controller.Step.prototype.unload.call( this );
+ };
+
+ uw.controller.Details.prototype.moveNext = function () {
+ this.removeErrorUploads();
+
+ uw.controller.Step.prototype.moveNext.call( this );
};
uw.controller.Details.prototype.addCopyMetadataFeature = function (
uploads ) {
@@ -435,7 +430,7 @@
this.addCopyMetadataFeature( this.uploads );
} else if ( this.uploads.length === 0 ) {
// If we have no more uploads, go to the "Upload" step.
(This will go to "Thanks" step,
- // which will skip itself in moveTo() because there are
no uploads left.)
+ // which will skip itself in load() because there are
no uploads left.)
this.moveNext();
}
};
diff --git a/resources/controller/uw.controller.Step.js
b/resources/controller/uw.controller.Step.js
index 09b0c00..bee6297 100644
--- a/resources/controller/uw.controller.Step.js
+++ b/resources/controller/uw.controller.Step.js
@@ -95,11 +95,11 @@
};
/**
- * Move to this step.
+ * Initialize this step.
*
* @param {mw.UploadWizardUpload[]} uploads List of uploads being
carried forward.
*/
- uw.controller.Step.prototype.moveTo = function ( uploads ) {
+ uw.controller.Step.prototype.load = function ( uploads ) {
var step = this;
this.movedFrom = false;
@@ -114,22 +114,31 @@
}
} );
- this.ui.moveTo( uploads );
+ this.ui.load( uploads );
uw.eventFlowLogger.logStep( this.stepName );
this.updateFileCounts( this.uploads );
};
/**
+ * Cleanup this step.
+ */
+ uw.controller.Step.prototype.unload = function () {
+ this.movedFrom = true;
+
+ this.ui.unload();
+
+ this.emit( 'unload' );
+ };
+
+ /**
* Move to the next step.
*/
uw.controller.Step.prototype.moveNext = function () {
- this.ui.moveNext( this.uploads );
-
- this.movedFrom = true;
+ this.unload();
if ( this.nextStep ) {
- this.nextStep.moveTo( this.uploads );
+ this.nextStep.load( this.uploads );
}
};
@@ -137,12 +146,10 @@
* Move to the previous step.
*/
uw.controller.Step.prototype.movePrevious = function () {
- this.ui.movePrevious( this.uploads );
-
- this.movedFrom = true;
+ this.unload();
if ( this.previousStep ) {
- this.previousStep.moveTo( this.uploads );
+ this.previousStep.load( this.uploads );
}
};
diff --git a/resources/controller/uw.controller.Thanks.js
b/resources/controller/uw.controller.Thanks.js
index 307a9e0..7ed402f 100644
--- a/resources/controller/uw.controller.Thanks.js
+++ b/resources/controller/uw.controller.Thanks.js
@@ -37,7 +37,7 @@
OO.inheritClass( uw.controller.Thanks, uw.controller.Step );
- uw.controller.Thanks.prototype.moveTo = function ( uploads ) {
+ uw.controller.Thanks.prototype.load = function ( uploads ) {
var thanks = this;
if ( uploads.length === 0 ) {
@@ -46,7 +46,7 @@
return;
}
- uw.controller.Step.prototype.moveTo.call( this, uploads );
+ uw.controller.Step.prototype.load.call( this, uploads );
$.each( uploads, function ( i, upload ) {
thanks.ui.addUpload( upload );
diff --git a/resources/controller/uw.controller.Tutorial.js
b/resources/controller/uw.controller.Tutorial.js
index 41b66e1..217053c 100644
--- a/resources/controller/uw.controller.Tutorial.js
+++ b/resources/controller/uw.controller.Tutorial.js
@@ -80,20 +80,23 @@
} );
};
- uw.controller.Tutorial.prototype.moveTo = function ( uploads ) {
- uw.controller.Step.prototype.moveTo.call( this, uploads );
+ uw.controller.Tutorial.prototype.load = function ( uploads ) {
+ uw.controller.Step.prototype.load.call( this, uploads );
uw.eventFlowLogger.logTutorialAction( 'load' );
};
uw.controller.Tutorial.prototype.moveNext = function () {
uw.eventFlowLogger.logTutorialAction( 'continue' );
+ uw.controller.Step.prototype.moveNext.call( this );
+ };
+ uw.controller.Tutorial.prototype.unload = function () {
// if the skip checkbox is checked, set the skip user preference
if ( this.shouldSkipTutorial !== this.skipPreference ) {
this.setSkipPreference( this.shouldSkipTutorial );
}
- uw.controller.Step.prototype.moveNext.call( this );
+ uw.controller.Step.prototype.unload.call( this );
};
uw.controller.Tutorial.prototype.isComplete = function () {
diff --git a/resources/controller/uw.controller.Upload.js
b/resources/controller/uw.controller.Upload.js
index a705289..f873cd5 100644
--- a/resources/controller/uw.controller.Upload.js
+++ b/resources/controller/uw.controller.Upload.js
@@ -95,10 +95,10 @@
this.ui.empty();
};
- uw.controller.Upload.prototype.moveTo = function ( uploads ) {
+ uw.controller.Upload.prototype.load = function ( uploads ) {
var controller = this;
- uw.controller.Step.prototype.moveTo.call( this, uploads );
+ uw.controller.Step.prototype.load.call( this, uploads );
this.startProgressBar();
if ( uploads.length > 0 ) {
diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js
index a25807f..786d171 100644
--- a/resources/mw.UploadWizard.js
+++ b/resources/mw.UploadWizard.js
@@ -58,7 +58,7 @@
/**
* Resets wizard state and moves to the file step.
*/
- bailAndMoveToFile: function () {
+ bailAndloadFile: function () {
// destroy the flickr interface if it exists
this.flickrInterfaceDestroy();
@@ -77,7 +77,7 @@
this.initialiseSteps();
// "select" the first step - highlight, make it
visible, hide all others
- this.steps.firstStep.moveTo( [] );
+ this.steps.firstStep.load( [] );
},
/**
@@ -109,7 +109,7 @@
$.each( this.steps, function ( name, step ) {
step
.on( 'no-uploads', function () {
- wizard.bailAndMoveToFile();
+ wizard.bailAndloadFile();
} );
} );
diff --git a/resources/ui/steps/uw.ui.Deed.js b/resources/ui/steps/uw.ui.Deed.js
index 2dbf418..9422bde 100644
--- a/resources/ui/steps/uw.ui.Deed.js
+++ b/resources/ui/steps/uw.ui.Deed.js
@@ -35,10 +35,10 @@
OO.inheritClass( uw.ui.Deed, uw.ui.Step );
- uw.ui.Deed.prototype.moveTo = function ( uploads ) {
+ uw.ui.Deed.prototype.load = function ( uploads ) {
var ui = this;
- uw.ui.Step.prototype.moveTo.call( this, uploads );
+ uw.ui.Step.prototype.load.call( this, uploads );
this.$div.prepend(
$( '<div>' )
diff --git a/resources/ui/steps/uw.ui.Details.js
b/resources/ui/steps/uw.ui.Details.js
index dfaad0f..9ca86b4 100644
--- a/resources/ui/steps/uw.ui.Details.js
+++ b/resources/ui/steps/uw.ui.Details.js
@@ -69,8 +69,8 @@
OO.inheritClass( uw.ui.Details, uw.ui.Step );
- uw.ui.Details.prototype.moveTo = function ( uploads ) {
- uw.ui.Step.prototype.moveTo.call( this, uploads );
+ uw.ui.Details.prototype.load = function ( uploads ) {
+ uw.ui.Step.prototype.load.call( this, uploads );
this.$div.prepend(
$( '<div>' )
diff --git a/resources/ui/steps/uw.ui.Tutorial.js
b/resources/ui/steps/uw.ui.Tutorial.js
index 016de82..f04d4aa 100644
--- a/resources/ui/steps/uw.ui.Tutorial.js
+++ b/resources/ui/steps/uw.ui.Tutorial.js
@@ -98,8 +98,8 @@
this.skipCheckbox.setSelected( selected );
};
- uw.ui.Tutorial.prototype.moveTo = function ( uploads ) {
- uw.ui.Step.prototype.moveTo.call( this, uploads );
+ uw.ui.Tutorial.prototype.load = function ( uploads ) {
+ uw.ui.Step.prototype.load.call( this, uploads );
this.$div.prepend(
$( '<div>' )
diff --git a/resources/ui/steps/uw.ui.Upload.js
b/resources/ui/steps/uw.ui.Upload.js
index b876e20..c659aab 100644
--- a/resources/ui/steps/uw.ui.Upload.js
+++ b/resources/ui/steps/uw.ui.Upload.js
@@ -286,8 +286,8 @@
this.$flickrSelect.unbind();
};
- uw.ui.Upload.prototype.moveTo = function ( uploads ) {
- uw.ui.Step.prototype.moveTo.call( this, uploads );
+ uw.ui.Upload.prototype.load = function ( uploads ) {
+ uw.ui.Step.prototype.load.call( this, uploads );
if ( uploads.length === 0 ) {
this.$fileList.removeClass( 'mwe-upwiz-filled-filelist'
);
diff --git a/resources/ui/uw.ui.Step.js b/resources/ui/uw.ui.Step.js
index 035e9b2..fb167ee 100644
--- a/resources/ui/uw.ui.Step.js
+++ b/resources/ui/uw.ui.Step.js
@@ -55,11 +55,11 @@
OO.mixinClass( uw.ui.Step, OO.EventEmitter );
/**
- * Move to the step.
+ * Initialize this step.
*
* @param {mw.UploadWizardUpload[]} uploads
*/
- uw.ui.Step.prototype.moveTo = function ( uploads ) {
+ uw.ui.Step.prototype.load = function ( uploads ) {
var offset = $( 'h1:first' ).offset();
this.movedFrom = false;
@@ -75,18 +75,9 @@
};
/**
- * Move to the next step.
+ * Cleanup this step.
*/
- uw.ui.Step.prototype.moveNext = function () {
- this.movedFrom = true;
-
- this.$div.children().detach();
- };
-
- /**
- * Move to the previous step.
- */
- uw.ui.Step.prototype.movePrevious = function () {
+ uw.ui.Step.prototype.unload = function () {
this.movedFrom = true;
this.$div.children().detach();
diff --git a/tests/qunit/controller/uw.controller.Deed.test.js
b/tests/qunit/controller/uw.controller.Deed.test.js
index 9572baf..5242249 100644
--- a/tests/qunit/controller/uw.controller.Deed.test.js
+++ b/tests/qunit/controller/uw.controller.Deed.test.js
@@ -25,7 +25,7 @@
assert.ok( step.ui );
} );
- QUnit.test( 'moveTo', 1, function ( assert ) {
+ QUnit.test( 'load', 1, function ( assert ) {
var step = new uw.controller.Deed(
new mw.Api(),
{ licensing: { thirdParty: { type: 'test',
licenses: [] } } }
@@ -38,8 +38,8 @@
{ getThumbnail: ststub }
];
- this.sandbox.stub( step.ui, 'moveTo' );
- step.moveTo( uploads );
+ this.sandbox.stub( step.ui, 'load' );
+ step.load( uploads );
assert.strictEqual( ststub.callCount, 2 );
} );
diff --git a/tests/qunit/controller/uw.controller.Details.test.js
b/tests/qunit/controller/uw.controller.Details.test.js
index fb845ec..3a5b607 100644
--- a/tests/qunit/controller/uw.controller.Details.test.js
+++ b/tests/qunit/controller/uw.controller.Details.test.js
@@ -54,35 +54,35 @@
assert.ok( step.ui );
} );
- QUnit.test( 'moveTo', 12, function ( assert ) {
+ QUnit.test( 'load', 12, function ( assert ) {
var step = new uw.controller.Details( new mw.Api(), {
maxSimultaneousConnections: 1
} ),
testUpload = createTestUpload( this.sandbox ),
- stepUiStub = this.sandbox.stub( step.ui, 'moveTo' );
+ stepUiStub = this.sandbox.stub( step.ui, 'load' );
- step.moveTo( [ testUpload ] );
+ step.load( [ testUpload ] );
assert.strictEqual( testUpload.stubs.ucdc.called, false );
assert.ok( testUpload.stubs.cd.called );
assert.ok( stepUiStub.called );
testUpload = createTestUpload( this.sandbox, true );
- step.moveTo( [ testUpload ] );
+ step.load( [ testUpload ] );
assert.ok( testUpload.stubs.ucdc.called );
assert.ok( testUpload.stubs.cd.called );
assert.ok( stepUiStub.called );
testUpload = createTestUpload( this.sandbox );
- step.moveTo( [ testUpload, createTestUpload( this.sandbox ) ] );
+ step.load( [ testUpload, createTestUpload( this.sandbox ) ] );
assert.strictEqual( testUpload.stubs.ucdc.called, false );
assert.ok( testUpload.stubs.cd.called );
assert.ok( stepUiStub.called );
testUpload = createTestUpload( this.sandbox );
- step.moveTo( [ testUpload, createTestUpload( this.sandbox,
false, true ) ] );
+ step.load( [ testUpload, createTestUpload( this.sandbox, false,
true ) ] );
assert.strictEqual( testUpload.stubs.ucdc.called, false );
assert.ok( testUpload.stubs.cd.called );
diff --git a/tests/qunit/controller/uw.controller.Thanks.test.js
b/tests/qunit/controller/uw.controller.Thanks.test.js
index 447b4af..895b75e 100644
--- a/tests/qunit/controller/uw.controller.Thanks.test.js
+++ b/tests/qunit/controller/uw.controller.Thanks.test.js
@@ -25,12 +25,12 @@
assert.ok( step.ui );
} );
- QUnit.test( 'moveTo', 1, function ( assert ) {
+ QUnit.test( 'load', 1, function ( assert ) {
var step = new uw.controller.Thanks( new mw.Api(), {} ),
auStub = this.sandbox.stub( step.ui, 'addUpload' );
- this.sandbox.stub( step.ui, 'moveTo' );
- step.moveTo( [ 1, 2, 3 ] );
+ this.sandbox.stub( step.ui, 'load' );
+ step.load( [ 1, 2, 3 ] );
assert.strictEqual( auStub.callCount, 3 );
} );
--
To view, visit https://gerrit.wikimedia.org/r/322122
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ce7ebded7790749f2dca8440a1fccd139b32b99
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/UploadWizard
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
