Esanders has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/347359 )
Change subject: Refactor savedialog/target interaction
......................................................................
Refactor savedialog/target interaction
* Pass a wikitext promise, instead of wikitext
* Handle writing of content to the dialog from within
the dialog.
* Handle diff errors within the dialog.
* Never disable the 'Return to save form' (approve) button
Change-Id: Ibd76e8951998f751abfb4f407682202c2f73ac7e
---
M modules/ve-mw/i18n/en.json
M modules/ve-mw/i18n/qqq.json
M modules/ve-mw/init/ve.init.mw.ArticleTarget.js
M modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js
M modules/ve-mw/ui/styles/dialogs/ve.ui.MWSaveDialog.css
5 files changed, 89 insertions(+), 140 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/59/347359/1
diff --git a/modules/ve-mw/i18n/en.json b/modules/ve-mw/i18n/en.json
index e4ddb5e..4e47650 100644
--- a/modules/ve-mw/i18n/en.json
+++ b/modules/ve-mw/i18n/en.json
@@ -364,7 +364,6 @@
"visualeditor-savedialog-review-visual": "Visual",
"visualeditor-savedialog-review-wikitext": "Wikitext",
"visualeditor-savedialog-title-conflict": "Conflict",
- "visualeditor-savedialog-title-nochanges": "No changes to review",
"visualeditor-savedialog-title-preview": "Preview your changes",
"visualeditor-savedialog-title-review": "Review your changes",
"visualeditor-savedialog-title-save": "Save your changes",
diff --git a/modules/ve-mw/i18n/qqq.json b/modules/ve-mw/i18n/qqq.json
index 0ed043c..b48d832 100644
--- a/modules/ve-mw/i18n/qqq.json
+++ b/modules/ve-mw/i18n/qqq.json
@@ -377,7 +377,6 @@
"visualeditor-savedialog-review-visual": "Label for button to select
visual diff mode.\n{{Identical|Visual}}",
"visualeditor-savedialog-review-wikitext": "Label for button to select
wikitext diff mode.\n{{Identical|Wikitext}}",
"visualeditor-savedialog-title-conflict": "Title for save dialog slide
if there is an edit conflict\n{{Identical|Conflict}}",
- "visualeditor-savedialog-title-nochanges": "Title for save dialog slide
for the wikitext diff if there are no changes",
"visualeditor-savedialog-title-preview": "Title for save dialog slide
for the HTML preview from wikitext mode",
"visualeditor-savedialog-title-review": "Title for save dialog slide
for the wikitext diff",
"visualeditor-savedialog-title-save": "Title for save dialog slide for
the final save step. Appears between the buttons
{{msg-mw|visualeditor-toolbar-savedialog}} and
{{msg-mw|visualeditor-dialog-action-cancel}}.",
diff --git a/modules/ve-mw/init/ve.init.mw.ArticleTarget.js
b/modules/ve-mw/init/ve.init.mw.ArticleTarget.js
index af5c319..52204f4 100644
--- a/modules/ve-mw/init/ve.init.mw.ArticleTarget.js
+++ b/modules/ve-mw/init/ve.init.mw.ArticleTarget.js
@@ -703,74 +703,6 @@
};
/**
- * Handle a successful show changes request.
- *
- * @method
- * @param {Object} response API response data
- * @param {string} status Text status message
- */
-ve.init.mw.ArticleTarget.prototype.showChangesSuccess = function ( response ) {
- var data = response.visualeditoredit;
- this.diffing = false;
- if ( !data && !response.error ) {
- this.showChangesFail( null, 'Invalid response from server',
null );
- } else if ( response.error ) {
- this.showChangesFail(
- null, 'Unsuccessful request: ' + response.error.info,
null
- );
- } else if ( data.result === 'nochanges' ) {
- this.noChanges();
- } else if ( data.result !== 'success' ) {
- this.showChangesFail( null, 'Failed request: ' + data.result,
null );
- } else if ( typeof data.diff !== 'string' ) {
- this.showChangesFail(
- null, 'Invalid HTML content in response from server',
null
- );
- } else {
- this.showChangesDiff( data.diff );
- }
-};
-
-/**
- * Show changes diff HTML
- *
- * @param {string} diffHtml Diff HTML
- * @fires showChanges
- */
-ve.init.mw.ArticleTarget.prototype.showChangesDiff = function ( diffHtml ) {
- this.emit( 'showChanges' );
-
- // Invalidate the viewer diff on next change
- this.getSurface().getModel().getDocument().once( 'transact',
- this.saveDialog.clearDiff.bind( this.saveDialog )
- );
- this.saveDialog.setDiffAndReview(
- diffHtml,
- this.getVisualDiffPromise(),
- this.getSurface().getModel().getDocument().getHtmlDocument()
- );
-};
-
-/**
- * Handle errors during showChanges action.
- *
- * @method
- * @this ve.init.mw.ArticleTarget
- * @param {Object} jqXHR
- * @param {string} status Text status message
- * @param {Mixed} error HTTP status text
- * @fires showChangesError
- */
-ve.init.mw.ArticleTarget.prototype.showChangesFail = function ( jqXHR, status
) {
- this.diffing = false;
- this.emit( 'showChangesError' );
-
- OO.ui.alert( ve.msg( 'visualeditor-differror', status ) );
-
- this.saveDialog.popPending();
-};
-
-/**
* Show an save process error message
*
* @method
@@ -1001,19 +933,6 @@
};
/**
- * Handle no changes in diff
- *
- * @method
- * @fires noChanges
- */
-ve.init.mw.ArticleTarget.prototype.noChanges = function () {
- this.emit( 'noChanges' );
- this.saveDialog.popPending();
- this.saveDialog.swapPanel( 'nochanges' );
- this.saveDialog.getActions().setAbilities( { approve: true } );
-};
-
-/**
* Handle a successful serialize request.
*
* This method is called within the context of a target instance.
@@ -1073,7 +992,6 @@
ve.init.mw.ArticleTarget.prototype.onSaveDialogReview = function () {
if ( !this.saveDialog.hasDiff ) {
this.emit( 'saveReview' );
- this.saveDialog.getActions().setAbilities( { approve: false } );
this.saveDialog.pushPending();
if ( this.pageExists ) {
// Has no callback, handled via target.showChangesDiff
@@ -1098,7 +1016,6 @@
if ( !this.saveDialog.$previewViewer.children().length ) {
this.emit( 'savePreview' );
- this.saveDialog.getActions().setAbilities( { approve: false } );
this.saveDialog.pushPending();
wikitext = this.getDocToSave();
@@ -1164,7 +1081,7 @@
ve.init.mw.ArticleTarget.prototype.onSaveDialogReviewComplete = function (
wikitext ) {
this.bindSaveDialogClearDiff();
this.saveDialog.setDiffAndReview(
- $( '<pre>' ).text( wikitext ),
+ $.Deferred().resolve( $( '<pre>' ).text( wikitext ) ).promise(),
this.getVisualDiffPromise(),
this.getSurface().getModel().getDocument().getHtmlDocument()
);
@@ -1301,7 +1218,7 @@
this.clearPreparedCacheKey();
this.loading = false;
this.saving = false;
- this.diffing = false;
+ this.wikitextDiffPromise = null;
this.serializing = false;
this.submitting = false;
this.baseTimeStamp = null;
@@ -1745,27 +1662,64 @@
};
/**
+ * Show changes in the save dialog
+ *
+ * @param {Object} doc Document
+ */
+ve.init.mw.ArticleTarget.prototype.showChanges = function ( doc ) {
+ var target = this;
+ // Invalidate the viewer diff on next change
+ this.getSurface().getModel().getDocument().once( 'transact', function
() {
+ target.saveDialog.clearDiff();
+ target.wikitextDiffPromise = null;
+ } );
+ this.saveDialog.setDiffAndReview(
+ this.getWikitextDiffPromise( doc ),
+ this.getVisualDiffPromise(),
+ this.getSurface().getModel().getDocument().getHtmlDocument()
+ );
+};
+
+/**
* Post DOM data to the Parsoid API to retrieve wikitext diff.
*
* @method
* @param {HTMLDocument} doc Document to compare against (via wikitext)
- * @return {boolean} Diffing has been started
+ * @return {jQuery.Promise} Promise which resolves with the wikitext diff, or
rejects with an error
+ * @fires showChanges
+ * @fires showChangesError
*/
-ve.init.mw.ArticleTarget.prototype.showChanges = function ( doc ) {
- if ( this.diffing ) {
- return false;
+ve.init.mw.ArticleTarget.prototype.getWikitextDiffPromise = function ( doc ) {
+ var target = this;
+ if ( !this.wikitextDiffPromise ) {
+ this.wikitextDiffPromise = this.tryWithPreparedCacheKey( doc, {
+ action: 'visualeditoredit',
+ paction: 'diff',
+ page: this.pageName,
+ oldid: this.revid,
+ etag: this.etag
+ }, 'diff' ).then( function ( response ) {
+ var data = response.visualeditoredit;
+ if ( !data && !response.error ) {
+ return $.Deferred().reject( 'Invalid response
from server' ).promise();
+ } else if ( response.error ) {
+ return $.Deferred().reject( response.error.info
).promise();
+ } else if ( data.result === 'nochanges' ) {
+ target.emit( 'noChanges' );
+ return null;
+ } else if ( data.result !== 'success' ) {
+ return $.Deferred().reject( 'Failed request: '
+ data.result ).promise();
+ } else if ( typeof data.diff !== 'string' ) {
+ return $.Deferred().reject( 'Invalid HTML
content in response from server' ).promise();
+ } else {
+ return data.diff;
+ }
+ } );
+ this.wikitextDiffPromise
+ .done( this.emit.bind( this, 'showChanges' ) )
+ .fail( this.emit.bind( this, 'showChangesError' ) );
}
- this.diffing = this.tryWithPreparedCacheKey( doc, {
- action: 'visualeditoredit',
- paction: 'diff',
- page: this.pageName,
- oldid: this.revid,
- etag: this.etag
- }, 'diff' )
- .done( this.showChangesSuccess.bind( this ) )
- .fail( this.showChangesFail.bind( this ) );
-
- return true;
+ return this.wikitextDiffPromise;
};
/**
diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js
b/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js
index c2a669d..16900e3 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWSaveDialog.js
@@ -125,10 +125,14 @@
* @param {jQuery.Promise} [visualDiffPromise] Visual diff promise
* @param {HTMLDocument} [baseDoc] Base document against which to normalise
links when rendering visualDiff
*/
-ve.ui.MWSaveDialog.prototype.setDiffAndReview = function ( wikitextDiff,
visualDiffPromise, baseDoc ) {
- this.$reviewVisualDiff.empty();
+ve.ui.MWSaveDialog.prototype.setDiffAndReview = function (
wikitextDiffPromise, visualDiffPromise, baseDoc ) {
+ var dialog = this;
+
+ this.clearDiff();
+
+ // Visual diff
+ this.$reviewVisualDiff.append( new OO.ui.ProgressBarWidget().$element );
if ( visualDiffPromise ) {
- this.clearVisualDiff();
// Don't generate the DiffElement until the tab is switched to
this.getDiffElementPromise = function () {
return visualDiffPromise.then( function ( visualDiff ) {
@@ -147,8 +151,22 @@
this.reviewModeButtonSelect.selectItemByData( 'source' );
}
- this.$reviewWikitextDiff.empty().append( wikitextDiff );
- this.actions.setAbilities( { approve: true } );
+ // Wikitext diff
+ this.$reviewWikitextDiff.append( new OO.ui.ProgressBarWidget().$element
);
+ wikitextDiffPromise.then( function ( wikitextDiff ) {
+ if ( wikitextDiff ) {
+ dialog.$reviewWikitextDiff.empty().append( wikitextDiff
);
+ } else {
+ dialog.$reviewWikitextDiff.empty().append(
+ $( '<div>' ).addClass(
've-ui-mwSaveDialog-no-changes' ).text( ve.msg( 'visualeditor-diff-no-changes'
) )
+ );
+ }
+ }, function ( error ) {
+ dialog.$reviewWikitextDiff.empty().append( error );
+ } ).always( function () {
+ dialog.updateSize();
+ } );
+
this.hasDiff = true;
this.popPending();
this.swapPanel( 'review' );
@@ -166,7 +184,6 @@
ve.init.platform.linkCache.styleParsoidElements( this.$previewViewer,
baseDoc );
// Run hooks so other things can alter the document
mw.hook( 'wikipage.content' ).fire( this.$previewViewer );
- this.actions.setAbilities( { approve: true } );
this.popPending();
this.swapPanel( 'preview' );
};
@@ -195,26 +212,18 @@
*/
ve.ui.MWSaveDialog.prototype.clearDiff = function () {
this.$reviewWikitextDiff.empty();
+ this.$reviewVisualDiff.empty();
this.$previewViewer.empty();
this.hasDiff = false;
- this.clearVisualDiff();
-};
-
-/**
- * Clear the visual diff
- */
-ve.ui.MWSaveDialog.prototype.clearVisualDiff = function () {
- if ( this.diffElement ) {
- this.diffElement = null;
- this.diffElementPromise = null;
- this.getDiffElementPromise = null;
- }
+ this.diffElement = null;
+ this.diffElementPromise = null;
+ this.getDiffElementPromise = null;
};
/**
* Swap state in the save dialog.
*
- * @param {string} panel One of 'save', 'review', 'conflict' or 'nochanges'
+ * @param {string} panel One of 'save', 'review' or 'conflict'
* @param {boolean} [noFocus] Don't attempt to focus anything (e.g. while
setting up)
* @throws {Error} Unknown saveDialog panel
*/
@@ -225,14 +234,13 @@
dialog = this,
panelObj = dialog[ panel + 'Panel' ];
- if ( ( [ 'save', 'review', 'preview', 'conflict', 'nochanges'
].indexOf( panel ) ) === -1 ) {
+ if ( ( [ 'save', 'review', 'preview', 'conflict' ].indexOf( panel ) )
=== -1 ) {
throw new Error( 'Unknown saveDialog panel: ' + panel );
}
// Update the window title
// The following messages can be used here:
// visualeditor-savedialog-title-conflict
- // visualeditor-savedialog-title-nochanges
// visualeditor-savedialog-title-preview
// visualeditor-savedialog-title-review
// visualeditor-savedialog-title-save
@@ -301,9 +309,6 @@
setTimeout( function () {
dialog.updateReviewMode();
} );
- break;
- case 'nochanges':
- mode = 'review';
break;
}
@@ -577,24 +582,12 @@
.find( 'a' ).attr( 'target', '_blank' ).end();
this.conflictPanel.$element.append( this.$conflict );
- // No changes panel
- this.nochangesPanel = new OO.ui.PanelLayout( {
- expanded: false,
- scrollable: true,
- padded: true
- } );
- this.$noChanges = $( '<div>' ).addClass( 've-ui-mwSaveDialog-nochanges'
)
- .html( ve.init.platform.getParsedMessage(
'visualeditor-diff-nochanges' ) )
- .find( 'a' ).attr( 'target', '_blank' ).end();
- this.nochangesPanel.$element.append( this.$noChanges );
-
// Panel stack
this.panels.addItems( [
this.savePanel,
this.reviewPanel,
this.previewPanel,
- this.conflictPanel,
- this.nochangesPanel
+ this.conflictPanel
] );
// Save button for "save" panel
@@ -725,7 +718,6 @@
ve.ui.MWSaveDialog.prototype.getTeardownProcess = function ( data ) {
return ve.ui.MWSaveDialog.super.prototype.getTeardownProcess.call(
this, data )
.next( function () {
- this.clearVisualDiff();
this.emit( 'close' );
}, this );
};
diff --git a/modules/ve-mw/ui/styles/dialogs/ve.ui.MWSaveDialog.css
b/modules/ve-mw/ui/styles/dialogs/ve.ui.MWSaveDialog.css
index fb9626f..701f656 100644
--- a/modules/ve-mw/ui/styles/dialogs/ve.ui.MWSaveDialog.css
+++ b/modules/ve-mw/ui/styles/dialogs/ve.ui.MWSaveDialog.css
@@ -84,3 +84,8 @@
margin-top: 1em;
clear: both;
}
+
+.ve-ui-mwSaveDialog-no-changes {
+ color: #72777d;
+ font-style: italic;
+}
--
To view, visit https://gerrit.wikimedia.org/r/347359
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd76e8951998f751abfb4f407682202c2f73ac7e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
