Revisions: Better error handling.

* Shows an error message if the current diff can't be loaded.
* For bulk pre-loading, catches errors, and cuts subsequent requests in half, until eventually giving up.
* Some CSS fixes related to this, and the loading spinner.
* `wp.revisions.loadAll()` now returns a promise representing whether or not all revisions could be loaded.

Fixes #24758.

git-svn-id: http://core.svn.wordpress.org/trunk@24732 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This commit is contained in:
Mark Jaquith 2013-07-18 16:35:19 +00:00
parent 929def2359
commit 93514ba101
3 changed files with 63 additions and 17 deletions

View File

@ -3554,16 +3554,24 @@ body.revision-php {
height: 140px; height: 140px;
} }
.revisions .loading-indicator { .revisions .diff-error {
position: absolute; position: absolute;
text-align: center; text-align: center;
vertical-align: middle;
opacity: 0;
margin: 0 auto; margin: 0 auto;
width: 100%; width: 100%;
height: 32px; display: none;
}
.revisions.diff-error .diff-error {
display: block;
}
.revisions .loading-indicator {
position: absolute;
vertical-align: middle;
opacity: 0;
width: 100%;
top: 3em; top: 3em;
background: #fff url(../images/wpspin_light-2x.gif) no-repeat center top;
-webkit-transition: opacity 0.5s; -webkit-transition: opacity 0.5s;
-moz-transition: opacity 0.5s; -moz-transition: opacity 0.5s;
-ms-transition: opacity 0.5s; -ms-transition: opacity 0.5s;
@ -3571,6 +3579,12 @@ body.revision-php {
transition: opacity 0.5s; transition: opacity 0.5s;
} }
.revisions .loading-indicator span.spinner {
display: block;
margin: 0 auto;
float: none;
}
.revisions.loading .loading-indicator { .revisions.loading .loading-indicator {
opacity: 1; opacity: 1;
} }
@ -3587,6 +3601,10 @@ body.revision-php {
opacity: 0.5; opacity: 0.5;
} }
.revisions.diff-error .diff {
visibility: hidden;
}
.revisions-meta { .revisions-meta {
margin-top: 15px; margin-top: 15px;
} }

View File

@ -233,7 +233,9 @@ window.wp = window.wp || {};
request.done( _.bind( function() { request.done( _.bind( function() {
deferred.resolveWith( context, [ this.get( id ) ] ); deferred.resolveWith( context, [ this.get( id ) ] );
}, this ) ); }, this ) ).fail( _.bind( function() {
deferred.reject();
}) );
} }
return deferred.promise(); return deferred.promise();
@ -256,13 +258,22 @@ window.wp = window.wp || {};
diffs = _.first( this.getClosestUnloaded( allRevisionIds, centerId ), num ); diffs = _.first( this.getClosestUnloaded( allRevisionIds, centerId ), num );
if ( _.size( diffs ) > 0 ) { if ( _.size( diffs ) > 0 ) {
this.load( diffs ).done( function() { this.load( diffs ).done( function() {
deferred.resolve(); self._loadAll( allRevisionIds, centerId, num ).done( function() {
self._loadAll( allRevisionIds, centerId, num ); deferred.resolve();
});
}).fail( function() {
if ( 1 === num ) { // Already tried 1. This just isn't working. Give up.
deferred.reject();
} else { // Request fewer diffs this time
self._loadAll( allRevisionIds, centerId, Math.ceil( num / 2 ) ).done( function() {
deferred.resolve();
});
}
}); });
return deferred.promise();
} else { } else {
return deferred.reject().promise(); deferred.resolve();
} }
return deferred;
}, },
load: function( comparisons ) { load: function( comparisons ) {
@ -314,12 +325,14 @@ window.wp = window.wp || {};
revisions.model.FrameState = Backbone.Model.extend({ revisions.model.FrameState = Backbone.Model.extend({
defaults: { defaults: {
loading: false, loading: false,
error: false,
compareTwoMode: false compareTwoMode: false
}, },
initialize: function( attributes, options ) { initialize: function( attributes, options ) {
var properties = {}; var properties = {};
_.bindAll( this, 'receiveDiff' );
this._debouncedEnsureDiff = _.debounce( this._ensureDiff, 200 ); this._debouncedEnsureDiff = _.debounce( this._ensureDiff, 200 );
this.revisions = options.revisions; this.revisions = options.revisions;
@ -351,6 +364,7 @@ window.wp = window.wp || {};
}, },
updateLoadingStatus: function() { updateLoadingStatus: function() {
this.set( 'error', false );
this.set( 'loading', ! this.diff() ); this.set( 'loading', ! this.diff() );
}, },
@ -399,7 +413,7 @@ window.wp = window.wp || {};
// If we already have the diff, then immediately trigger the update. // If we already have the diff, then immediately trigger the update.
if ( diff ) { if ( diff ) {
this.trigger( 'update:diff', diff ); this.receiveDiff( diff );
return $.Deferred().resolve().promise(); return $.Deferred().resolve().promise();
// Otherwise, fetch the diff. // Otherwise, fetch the diff.
} else { } else {
@ -418,12 +432,20 @@ window.wp = window.wp || {};
this.updateDiff(); this.updateDiff();
}, },
receiveDiff: function( diff ) {
// Did we actually get a diff?
if ( _.isUndefined( diff ) || _.isUndefined( diff.id ) ) {
this.set({
loading: false,
error: true
});
} else if ( this._diffId === diff.id ) { // Make sure the current diff didn't change
this.trigger( 'update:diff', diff );
}
},
_ensureDiff: function() { _ensureDiff: function() {
return this.diffs.ensure( this._diffId, this ).done( function( diff ) { return this.diffs.ensure( this._diffId, this ).always( this.receiveDiff );
// Make sure the current diff didn't change while the request was in flight.
if ( this._diffId === diff.id )
this.trigger( 'update:diff', diff );
});
} }
}); });
@ -443,6 +465,7 @@ window.wp = window.wp || {};
this.listenTo( this.model, 'update:diff', this.renderDiff ); this.listenTo( this.model, 'update:diff', this.renderDiff );
this.listenTo( this.model, 'change:compareTwoMode', this.updateCompareTwoMode ); this.listenTo( this.model, 'change:compareTwoMode', this.updateCompareTwoMode );
this.listenTo( this.model, 'change:loading', this.updateLoadingStatus ); this.listenTo( this.model, 'change:loading', this.updateLoadingStatus );
this.listenTo( this.model, 'change:error', this.updateErrorStatus );
this.views.set( '.revisions-control-frame', new revisions.view.Controls({ this.views.set( '.revisions-control-frame', new revisions.view.Controls({
model: this.model model: this.model
@ -470,6 +493,10 @@ window.wp = window.wp || {};
this.$el.toggleClass( 'loading', this.model.get('loading') ); this.$el.toggleClass( 'loading', this.model.get('loading') );
}, },
updateErrorStatus: function() {
this.$el.toggleClass( 'diff-error', this.model.get('error') );
},
updateCompareTwoMode: function() { updateCompareTwoMode: function() {
this.$el.toggleClass( 'comparing-two-revisions', this.model.get('compareTwoMode') ); this.$el.toggleClass( 'comparing-two-revisions', this.model.get('compareTwoMode') );
} }

View File

@ -186,7 +186,8 @@ require_once( './admin-header.php' );
</script> </script>
<script id="tmpl-revisions-diff" type="text/html"> <script id="tmpl-revisions-diff" type="text/html">
<div class="loading-indicator"></div> <div class="loading-indicator"><span class="spinner"></span></div>
<div class="diff-error"><?php _e( 'Sorry, something went wrong. The requested comparison could not be loaded.' ); ?></div>
<div class="diff"> <div class="diff">
<# _.each( data.fields, function( field ) { #> <# _.each( data.fields, function( field ) { #>
<h3>{{ field.name }}</h3> <h3>{{ field.name }}</h3>