Changeset 24611

Timestamp:

07/09/2013 09:57:58 AM (13 years ago)

Author:

koopersmith

Message:

Revisions: Debounce fetching Diffs when revisions are updated.

Fixes navigation and casting bugs.

See #24425.

Location:

trunk/wp-admin

Files:

• includes/revision.php (modified) (1 diff)
• js/revisions.js (modified) (9 diffs)

trunk/wp-admin/includes/revision.php

@@ -102 +102 @@
                 'nonce'            => wp_create_nonce( 'revisions-ajax-nonce' ),
                 'revisionData'     => array_values( $revisions ),
-                'selectedRevision' => (int) $selected_revision_id,
+                'selectedRevision' => $selected_revision_id,
         );
 }

trunk/wp-admin/js/revisions.js

@@ -8 +8 @@
         // Link settings.
         revisions.settings = typeof _wpRevisionsSettings === 'undefined' ? {} : _wpRevisionsSettings;
+
+        // wp_localize_script transforms top-level numbers into strings. Undo that.
+        if ( revisions.settings.selectedRevision )
+                revisions.settings.selectedRevision = parseInt( revisions.settings.selectedRevision, 10 );
 
 
@@ -38 +42 @@
                 comparator: function( revision ) {
                         return revision.id;
+                },
+
+                next: function( revision ) {
+                        var index = this.indexOf( revision );
+
+                        if ( index !== -1 && index !== this.length - 1 )
+                                return this.at( index + 1 );
+                },
+
+                prev: function( revision ) {
+                        var index = this.indexOf( revision );
+
+                        if ( index !== -1 && index !== 0 )
+                                return this.at( index - 1 );
                 }
         });
@@ -199 +217 @@
         revisions.model.FrameState = Backbone.Model.extend({
                 initialize: function( attributes, options ) {
+                        var properties = {};
+
+                        this._debouncedEnsureDiff = _.debounce( this._ensureDiff, 200 );
+
                         this.revisions = options.revisions;
                         this.diffs = new revisions.model.Diffs( [], { revisions: this.revisions });
-                        this.listenTo( this, 'change:from', this.updateDiff );
-                        this.listenTo( this, 'change:to', this.updateDiff );
+
+                        // Set the initial revision provided through the settings.
+                        properties.to = this.revisions.get( revisions.settings.selectedRevision );
+                        properties.from = this.revisions.prev( properties.to );
+                        this.set( properties );
+
+                        // Start the router. This will trigger a navigate event and ensure that
+                        // the `from` and `to` revisions accurately reflect the hash.
                         this.router = new revisions.Router({ model: this });
+                        Backbone.history.start();
+
+                        this.listenTo( this, 'change:from', this.changeRevisionHandler );
+                        this.listenTo( this, 'change:to', this.changeRevisionHandler );
+                        this.updateDiff({ immediate: true });
+                },
+
+                // Fetch the currently loaded diff.
+                diff: function() {
+                        return this.diffs.get( this._diffId );
                 },
 
@@ -209 +247 @@
                 // will only be updated once. This is because Backbone updates all of
                 // the changed attributes in `set`, and then fires the `change` events.
-                updateDiff: function() {
-                        var from, to, diffId;
-
+                updateDiff: function( options ) {
+                        var from, to, diffId, diff;
+
+                        options = options || {};
                         from = this.get('from');
                         to = this.get('to');
@@ -223 +262 @@
                         this.trigger( 'update:revisions', from, to );
 
-                        this.diffs.ensure( diffId, this ).done( function( diff ) {
-                                // Check if the current diff changed while the request was in flight.
-                                if ( this._diffId !== diff.id )
-                                        return;
-
+                        // If we already have the diff, then immediately trigger the update.
+                        diff = this.diffs.get( diffId );
+                        if ( diff ) {
                                 this.trigger( 'update:diff', diff );
+
+                        // Otherwise, fetch the diff.
+                        } else {
+                                if ( options.immediate )
+                                        this._ensureDiff();
+                                else
+                                        this._debouncedEnsureDiff();
+                        }
+                },
+
+                // A simple wrapper around `updateDiff` to prevent the change event's
+                // parameters from being passed through.
+                changeRevisionHandler: function( model, value, options ) {
+                        this.updateDiff();
+                },
+
+                _ensureDiff: function() {
+                        this.diffs.ensure( this._diffId, this ).done( function( diff ) {
+                                // Make sure the current diff didn't change while the request was in flight.
+                                if ( this._diffId === diff.id )
+                                        this.trigger( 'update:diff', diff );
                         });
                 }
@@ -246 +304 @@
 
                 initialize: function() {
+                        // Generate the frame model.
                         this.model = new revisions.model.FrameState({}, {
                                 revisions: this.collection
@@ -257 +316 @@
                         }) );
 
+                        // TODO: The rest of this method should be rewritten and moved into the FrameState.
                         if ( this.model.revisions.length ) {
-                                var last = this.model.revisions.last(2);
-                                var attributes = { to: last.pop() };
-
-                                if ( last.length )
-                                        attributes.from = last.pop();
-
-                                this.model.set( attributes );
-
                                 // Load the rest: first 10, then the rest by 50
                                 this.model.diffs.loadLastUnloaded( 10 ).always( _.bind( function() {
@@ -271 +323 @@
                                 }, this ) );
                         }
-                        Backbone.history.start();
                 },
 
@@ -808 +859 @@
 
                         // Maintain state history when dragging
-                        this.listenTo( this.model, 'renderDiff', _.debounce( this.updateUrl, 250 ) );
+                        this.listenTo( this.model, 'update:diff', _.debounce( this.updateUrl, 250 ) );
                 },