WordPress.org

Make WordPress Core

Changeset 24671


Ignore:
Timestamp:
07/12/2013 02:01:39 PM (9 years ago)
Author:
markjaquith
Message:

Revisions: Cleanup, bug fixes, refactoring, polish.

  • Hide the tooltip initially.
  • Fix a bug with routing.
  • Further separate the Slider model and view, refactoring its code.
  • More reliance on events than direct calls between areas.
  • Smarter background diff loading (single mode). Loads the diffs closest to your position first.
  • Removed a bunch of manual templating and render() methods. Now relies more on the WP Backbone Views functionality.
  • include the requested id in ensure:load.
  • new trigger: ensure, for ensure() attempts, regardless of whether they are already loaded.
  • pass along a promise in both ensure and ensure:load.
  • in ensure, remove requests for diffs we aready have

See #24425.

Location:
trunk/wp-admin
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/wp-admin/css/wp-admin.css

    r24667 r24671  
    36983698    min-width: 130px;
    36993699    padding: 4px;
     3700    display: none;
    37003701}
    37013702
  • trunk/wp-admin/includes/revision.php

    r24670 r24671  
    100100
    101101    // Now, grab the initial diff
    102     $compare_two_mode = (bool) $from;
    103     if ( ! $from ) // Single mode
     102    $compare_two_mode = is_numeric( $from );
     103    if ( ! $compare_two_mode ) {
    104104        $from = array_keys( array_slice( $revisions, array_search( $selected_revision_id, array_keys( $revisions ) ) - 1, 1, true ) );
    105105        $from = $from[0];
     106    }
     107
     108    $from = absint( $from );
    106109
    107110    $diffs = array( array(
  • trunk/wp-admin/js/revisions.js

    r24667 r24671  
    3434    revisions.model.Slider = Backbone.Model.extend({
    3535        defaults: {
    36             value: 0,
     36            value: null,
     37            values: null,
    3738            min: 0,
    3839            max: 1,
    3940            step: 1,
     41            range: false,
    4042            compareTwoMode: false
    4143        },
     
    4446            this.frame = options.frame;
    4547            this.revisions = options.revisions;
    46             this.set({
    47                 max:   this.revisions.length - 1,
    48                 value: this.revisions.indexOf( this.revisions.get( revisions.settings.to ) ),
    49                 compareTwoMode: this.frame.get('compareTwoMode')
    50             });
    5148
    5249            // Listen for changes to the revisions or mode from outside
     
    5754            this.listenTo( this, 'change:from', this.handleLocalChanges );
    5855            this.listenTo( this, 'change:to', this.handleLocalChanges );
     56            this.listenTo( this, 'change:compareTwoMode', this.updateSliderSettings );
     57            this.listenTo( this, 'update:revisions', this.updateSliderSettings );
    5958
    6059            // Listen for changes to the hovered revision
    6160            this.listenTo( this, 'change:hoveredRevision', this.hoverRevision );
     61
     62            this.set({
     63                max:   this.revisions.length - 1,
     64                compareTwoMode: this.frame.get('compareTwoMode'),
     65                from: this.frame.get('from'),
     66                to: this.frame.get('to')
     67            });
     68            this.updateSliderSettings();
     69        },
     70
     71        getSliderValue: function( a, b ) {
     72            return isRtl ? this.revisions.length - this.revisions.indexOf( this.get(a) ) - 1 : this.revisions.indexOf( this.get(b) );
     73        },
     74
     75        updateSliderSettings: function() {
     76            if ( this.get('compareTwoMode') ) {
     77                this.set({
     78                    values: [
     79                        this.getSliderValue( 'to', 'from' ),
     80                        this.getSliderValue( 'from', 'to' )
     81                    ],
     82                    value: null,
     83                    range: true // ensures handles cannot cross
     84                });
     85            } else {
     86                this.set({
     87                    value: this.getSliderValue( 'to', 'to' ),
     88                    values: null,
     89                    range: false
     90                });
     91            }
     92            this.trigger( 'update:slider' );
    6293        },
    6394
     
    162193    revisions.model.Diffs = Backbone.Collection.extend({
    163194        initialize: function( models, options ) {
     195            _.bindAll( this, 'getClosestUnloaded' );
     196            this.loadAll = _.once( this._loadAll );
    164197            this.revisions = options.revisions;
    165198            this.requests  = {};
     
    173206            var deferred = $.Deferred();
    174207            var ids      = {};
     208            var from     = id.split(':')[0];
     209            var to       = id.split(':')[1];
     210            ids[id] = true;
     211
     212            wp.revisions.log( 'ensure', id );
     213
     214            this.trigger( 'ensure', ids, from, to, deferred.promise() );
    175215
    176216            if ( diff ) {
    177217                deferred.resolveWith( context, [ diff ] );
    178218            } else {
    179                 this.trigger( 'ensure:load', ids );
    180                 _.each( ids, _.bind( function(id) {
     219                this.trigger( 'ensure:load', ids, from, to, deferred.promise() );
     220                _.each( ids, _.bind( function( id ) {
    181221                    // Remove anything that has an ongoing request
    182222                    if ( this.requests[ id ] )
     223                        delete ids[ id ];
     224                    // Remove anything we already have
     225                    if ( this.get( id ) )
    183226                        delete ids[ id ];
    184227                }, this ) );
     
    197240        },
    198241
    199         loadNew: function( comparisons ) {
     242        // Returns an array of proximal diffs
     243        getClosestUnloaded: function( ids, centerId ) {
    200244            var self = this;
    201             _.each( comparisons, function( id, index ) {
    202                 // Already exists in collection. Don't request it again.
    203                 if ( self.get( id ) )
    204                     delete comparisons[ index ];
    205             });
    206             wp.revisions.log( 'loadNew', comparisons );
    207 
    208             if ( comparisons.length )
    209                 return this.load( comparisons );
    210             else
    211                 return $.Deferred().resolve().promise();
     245            return _.chain([0].concat( ids )).initial().zip( ids ).sortBy( function( pair ) {
     246                return Math.abs( centerId - pair[1] );
     247            }).map( function( pair ) {
     248                return pair.join(':');
     249            }).filter( function( diffId ) {
     250                return _.isUndefined( self.get( diffId ) ) && ! self.requests[ diffId ];
     251            }).value();
     252        },
     253
     254        _loadAll: function( allRevisionIds, centerId, num ) {
     255            var self = this, deferred = $.Deferred();
     256            diffs = _.first( this.getClosestUnloaded( allRevisionIds, centerId ), num );
     257            if ( _.size( diffs ) > 0 ) {
     258                this.load( diffs ).done( function() {
     259                    deferred.resolve();
     260                    self._loadAll( allRevisionIds, centerId, num );
     261                });
     262                return deferred.promise();
     263            } else {
     264                return deferred.reject().promise();
     265            }
    212266        },
    213267
     
    215269            wp.revisions.log( 'load', comparisons );
    216270            // Our collection should only ever grow, never shrink, so remove: false
    217             return this.fetch({ data: { compare: comparisons }, remove: false });
    218         },
    219 
    220         loadLast: function( num ) {
    221             var ids;
    222 
    223             num = num || 1;
    224             ids = _.last( this.getProximalDiffIds(), num );
    225 
    226             if ( ids.length )
    227                 return this.loadNew( ids );
    228             else
    229                 return $.Deferred().resolve().promise();
    230         },
    231 
    232         loadLastUnloaded: function( num ) {
    233             var ids;
    234 
    235             num = num || 1;
    236             ids = _.last( this.getUnloadedProximalDiffIds(), num );
    237 
    238             if ( ids.length )
    239                 return this.loadNew( ids );
    240             else
    241                 return $.Deferred().resolve().promise();
    242         },
    243 
    244         getProximalDiffIds: function() {
    245             var previous = 0, ids = [];
    246             this.revisions.each( _.bind( function( revision ) {
    247                 ids.push( previous + ':' + revision.id );
    248                 previous = revision.id;
    249             }, this ) );
    250             return ids;
    251         },
    252 
    253         getUnloadedProximalDiffIds: function() {
    254             var comparisons = this.getProximalDiffIds();
    255             comparisons     = _.object( comparisons, comparisons );
    256             _.each( comparisons, _.bind( function( id ) {
    257                 // Exists
    258                 if ( this.get( id ) )
    259                     delete comparisons[ id ];
    260             }, this ) );
    261             return _.toArray( comparisons );
    262         },
    263 
    264         loadAllBy: function( chunkSize ) {
    265             chunkSize    = chunkSize || 20;
    266             var unloaded = this.getUnloadedProximalDiffIds();
    267             if ( unloaded.length ) {
    268                 return this.loadLastUnloaded( chunkSize ).always( _.bind( function() {
    269                     this.loadAllBy( chunkSize );
    270                 }, this ) );
    271             }
     271            return this.fetch({ data: { compare: comparisons }, remove: false }).done( function(){
     272                wp.revisions.log( 'load:complete', comparisons );
     273            });
    272274        },
    273275
     
    330332            this.listenTo( this, 'change:from', this.changeRevisionHandler );
    331333            this.listenTo( this, 'change:to', this.changeRevisionHandler );
    332             this.listenTo( this, 'update:revisions', this.loadSurrounding );
    333             this.listenTo( this, 'change:compareTwoMode', this.changedMode );
     334            this.listenTo( this, 'update:revisions', this.updatedRevisions );
    334335            this.listenTo( this.diffs, 'ensure:load', this.updateLoadingStatus );
    335336            this.listenTo( this, 'update:diff', this.updateLoadingStatus );
     
    340341            properties.compareTwoMode = revisions.settings.compareTwoMode;
    341342            properties.baseUrl = revisions.settings.baseUrl;
    342             this.set( properties, { silent: true } );
     343            this.set( properties );
    343344
    344345            // Start the router
     
    347348        },
    348349
    349         changedMode: function() {
    350             // This isn't passed from/to so we grab them from the model
    351             this.loadSurrounding( this.get( 'from' ), this.get( 'to' ) );
    352         },
    353 
    354350        updateLoadingStatus: function() {
    355351            this.set( 'loading', ! this.diff() );
    356352        },
    357353
    358         loadSurrounding: function( from, to ) {
    359             // Different strategies for single and compare-two models
     354        updatedRevisions: function( from, to ) {
    360355            if ( this.get( 'compareTwoMode' ) ) {
    361356                // TODO: compare-two loading strategy
    362357            } else {
    363                 // TODO: clean this up to hook in to the ensure process
    364                 if ( this.revisions.length ) {
    365                     // Load the rest: first 10, then the rest by 50
    366                     this.diffs.loadLastUnloaded( 10 ).always( _.bind( function() {
    367                         this.diffs.loadAllBy( 50 );
    368                     }, this ) );
    369                 }
     358                this.diffs.loadAll( this.revisions.pluck('id'), to.id, 40 );
    370359            }
    371360        },
     
    394383            this.trigger( 'update:revisions', from, to );
    395384
     385            diff = this.diffs.get( diffId );
     386
    396387            // If we already have the diff, then immediately trigger the update.
    397             diff = this.diffs.get( diffId );
    398388            if ( diff ) {
    399389                this.trigger( 'update:diff', diff );
     
    448438
    449439        render: function() {
    450             console.log( 'diff', this.model.diff() );
    451             this.model.updateDiff({ immediate: true }).done( _.bind( function() {
    452                 wp.Backbone.View.prototype.render.apply( this, arguments );
    453 
    454                 $('#wpbody-content .wrap').append( this.el );
    455                 this.updateCompareTwoMode();
    456                 this.views.ready();
    457             }, this ) );
     440            wp.Backbone.View.prototype.render.apply( this, arguments );
     441
     442            $('#wpbody-content .wrap').append( this.el );
     443            this.updateCompareTwoMode();
     444            this.renderDiff( this.model.diff() );
     445            this.views.ready();
    458446
    459447            return this;
     
    524512
    525513    // The tickmarks view
    526     // This contains the slider tickmarks.
    527514    revisions.view.Tickmarks = wp.Backbone.View.extend({
    528515        className: 'revisions-tickmarks',
    529516
    530         render: function() {
     517        ready: function() {
    531518            var tickCount, tickWidth;
    532 
    533519            tickCount = this.model.revisions.length - 1;
    534520            tickWidth = 1 / tickCount;
    535521
    536             this.$el.html('');
    537522            _(tickCount).times( function(){ this.$el.append( '<div></div>' ); }, this );
    538 
    539523            this.$('div').css( 'width', ( 100 * tickWidth ) + '%' );
    540         },
    541 
    542         ready: function() {
    543             this.render();
    544         }
    545     });
    546 
    547     // The meta view.
    548     // This contains the revision meta, and the restore button.
     524        }
     525    });
     526
     527    // The meta view
    549528    revisions.view.Meta = wp.Backbone.View.extend({
    550529        className: 'revisions-meta',
     
    556535
    557536        initialize: function() {
    558             this.listenTo( this.model, 'update:revisions', this.updateMeta );
     537            this.listenTo( this.model, 'update:revisions', this.ready );
     538        },
     539
     540        prepare: function() {
     541            return this.model.toJSON();
     542        },
     543
     544        ready: function() {
     545            this.$('.restore-revision').prop( 'disabled', this.model.get('to').get('current') );
    559546        },
    560547
    561548        restoreRevision: function() {
    562             var restoreUrl    = this.model.get('to').attributes.restoreUrl.replace(/&amp;/g, '&');
     549            var restoreUrl = this.model.get('to').attributes.restoreUrl.replace(/&amp;/g, '&');
    563550            document.location = restoreUrl;
    564         },
    565 
    566         updateMeta: function( from, to ) {
    567             this.$el.html( this.template( this.model.toJSON() ) );
    568             this.$('.restore-revision').prop( 'disabled', to.attributes.current );
    569551        }
    570552    });
    571553
    572554    // The checkbox view.
    573     // Encapsulates all of the configuration for the compare checkbox.
    574555    revisions.view.Checkbox = wp.Backbone.View.extend({
    575556        className: 'revisions-checkbox',
     
    581562
    582563        initialize: function() {
    583             this.$el.html( this.template() );
     564            this.listenTo( this.model, 'change:compareTwoMode', this.updateCompareTwoMode );
     565        },
     566
     567        ready: function() {
     568            if ( this.model.revisions.length < 3 )
     569                $('.revision-toggle-compare-mode').hide();
    584570        },
    585571
     
    592578            // Activate compare two mode?
    593579            this.model.set({ compareTwoMode: $('.compare-two-revisions').prop('checked') });
    594 
    595             // Update route
    596             this.model.router.updateUrl();
    597         },
    598 
    599         ready: function() {
    600             // Hide compare two mode toggle when fewer than three revisions.
    601             if ( this.model.revisions.length < 3 )
    602                 $('.revision-toggle-compare-mode').hide();
    603 
    604             this.listenTo( this.model, 'change:compareTwoMode', this.updateCompareTwoMode );
    605 
    606             // Update the mode in case route has set it
    607             this.updateCompareTwoMode();
    608         }
    609 
     580        }
    610581    });
    611582
     
    620591            this.listenTo( this.model, 'change:hovering', this.toggleVisibility );
    621592            this.listenTo( this.model, 'change:scrubbing', this.toggleVisibility );
    622         },
    623 
    624         ready: function() {
    625             this.toggleVisibility({ immediate: true });
    626593        },
    627594
     
    651618                return;
    652619
    653             // Insert revision data.
    654620            this.$el.html( this.template( this.model.get('revision').toJSON() ) );
    655621
     
    676642
    677643        initialize: function() {
    678             this.$el.html( this.template() );
    679644            this.listenTo( this.model, 'update:revisions', this.disabledButtonCheck );
    680645        },
     
    696661
    697662            this.model.set( attributes );
    698 
    699             // Update route
    700             this.model.router.updateUrl();
    701663        },
    702664
     
    744706        initialize: function() {
    745707            _.bindAll( this, 'start', 'slide', 'stop', 'mouseMove' );
    746             this.listenTo( this.model, 'change:compareTwoMode', this.updateSliderSettings );
    747             this.listenTo( this.model, 'update:revisions', this.updateSliderSettings );
     708            this.listenTo( this.model, 'update:slider', this.applySliderSettings );
    748709        },
    749710
     
    755716            }) );
    756717
    757             this.listenTo( this, 'slide:stop', this.updateSliderSettings );
     718            this.applySliderSettings();
    758719        },
    759720
     
    790751        },
    791752
    792         updateSliderSettings: function() {
    793             var handles, leftValue, rightValue;
     753        applySliderSettings: function() {
     754            this.$el.slider( _.pick( this.model.toJSON(), 'value', 'values', 'range' ) );
     755            var handles = this.$('a.ui-slider-handle');
    794756
    795757            if ( this.model.get('compareTwoMode') ) {
    796                 leftValue = isRtl ? this.model.revisions.length - this.model.revisions.indexOf( this.model.get('to') ) - 1 :
    797                                             this.model.revisions.indexOf( this.model.get('from') ),
    798                 rightValue = isRtl ?    this.model.revisions.length - this.model.revisions.indexOf( this.model.get('from') ) - 1 :
    799                                             this.model.revisions.indexOf( this.model.get('to') );
    800 
    801                 // Set handles to current from / to models.
    802                 // Reverse order for RTL
    803                 this.$el.slider( {
    804                     values: [
    805                         leftValue,
    806                         rightValue
    807                     ],
    808                     value: null,
    809                     range: true // Range mode ensures handles can't cross
    810                 } );
    811 
    812                 handles = this.$('a.ui-slider-handle');
    813758                // in RTL mode the 'left handle' is the second in the slider, 'right' is first
    814759                handles.first()
     
    818763                    .toggleClass( 'left-handle', !! isRtl )
    819764                    .toggleClass( 'right-handle', ! isRtl );
    820 
    821765            } else {
    822                 this.$el.slider( { // Set handle to current to model
    823                     // Reverse order for RTL.
    824                     value: isRtl ? this.model.revisions.length - this.model.revisions.indexOf( this.model.get('to') ) - 1 :
    825                                     this.model.revisions.indexOf( this.model.get('to') ),
    826                     values: null, // Clear existing two handled values
    827                     range: false
    828                 } );
    829                 this.$('a.ui-slider-handle').removeClass('left-handle right-handle');
     766                handles.removeClass('left-handle right-handle');
    830767            }
    831768        },
     
    851788                // In two handle mode, ensure handles can't be dragged past each other.
    852789                // Adjust left/right boundaries and reset points.
    853                 if ( view.model.frame.get('compareTwoMode') ) {
     790                if ( view.model.get('compareTwoMode') ) {
    854791                    var rightHandle = $( ui.handle ).parent().find('.right-handle'),
    855792                        leftHandle  = $( ui.handle ).parent().find('.left-handle');
     
    893830            var attributes;
    894831            // Compare two revisions mode
    895             if ( ! _.isUndefined( ui.values ) && this.model.frame.get('compareTwoMode') ) {
     832            if ( ! _.isUndefined( ui.values ) && this.model.get('compareTwoMode') ) {
    896833                // Prevent sliders from occupying same spot
    897834                if ( ui.values[1] === ui.values[0] )
     
    928865        stop: function( event, ui ) {
    929866            $( window ).off('mousemove.wp.revisions');
    930             this.updateSliderSettings();
     867            this.model.updateSliderSettings(); // To snap us back to a tick mark
    931868            this.model.set({ scrubbing: false });
    932869        }
     
    952889            this.routes = this.getRoutes();
    953890
    954             // Maintain state history when dragging
     891            // Maintain state history when dragging/clicking
    955892            this.listenTo( this.model, 'update:diff', _.debounce( this.updateUrl, 250 ) );
     893            this.listenTo( this.model, 'change:compareTwoMode', this.updateUrl );
    956894        },
    957895
  • trunk/wp-admin/revision.php

    r24667 r24671  
    1515
    1616$revision_id = absint( $revision );
    17 $from = absint( $from );
     17
     18$from = is_numeric( $from ) ? absint( $from ) : null;
    1819if ( ! $revision_id )
    1920    $revision_id = absint( $to );
Note: See TracChangeset for help on using the changeset viewer.