diff --git a/reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js b/reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js index f2582e0150c672a3f8bed23bddeb665f8dc1d0b1..9347af2a7ce99336d6ae9053febc969a425c8f92 100644 --- a/reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js +++ b/reviewboard/static/rb/js/diffviewer/views/diffReviewableView.js @@ -42,8 +42,6 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ initialize: function() { _super(this).initialize.call(this); - _.bindAll(this, '_updateCollapseButtonPos', '_onWindowResize'); - this._selector = new RB.TextCommentRowSelector({ el: this.el, reviewableView: this @@ -51,7 +49,6 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ this._hiddenCommentBlockViews = []; this._visibleCommentBlockViews = []; - this._$collapseButtons = $(); /* State for keeping consistent column widths for diff content. */ this._$filenameRow = null; @@ -79,9 +76,6 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ remove: function() { RB.AbstractReviewableView.prototype.remove.call(this); - this._$window.off('scroll', this._updateCollapseButtonPos); - this._$window.off('resize', this._onWindowResize); - this._selector.remove(); }, @@ -93,6 +87,8 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ _super(this).render.call(this); + this._centered = new RB.CenteredElementManager(); + $thead = this.$('thead'); this._$revisionRow = $thead.find('.revision-row'); @@ -118,9 +114,6 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ this._precalculateContentWidths(); this._updateColumnSizes(); - this._$window.on('scroll', this._updateCollapseButtonPos); - this._$window.on('resize', this._onWindowResize); - return this; }, @@ -254,83 +247,7 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ * chunk. */ _updateCollapseButtonPos: function() { - var windowTop, - windowHeight, - len = this._$collapseButtons.length, - $button, - $tbody, - parentOffset, - parentTop, - parentHeight, - i, - y1, - y2; - - if (len === 0) { - return; - } - - windowTop = this._$window.scrollTop(); - windowHeight = this._$window.height(); - - for (i = 0; i < len; i++) { - $button = $(this._$collapseButtons[i]); - $tbody = $button.parents('tbody'); - parentOffset = $tbody.offset(); - parentTop = parentOffset.top; - parentHeight = $tbody.height(); - - /* - * We're going to first try to limit our processing to expanded - * chunks that are currently on the screen. We'll skip over any - * before those chunks, and stop once we're sure we have no further - * ones we can show. - */ - if (parentTop >= windowTop + windowHeight) { - /* We hit the last one, so we're done. */ - break; - } else if (parentTop + parentHeight <= windowTop) { - /* We're not there yet. */ - } else { - /* Center the button in the view. */ - if ( windowTop >= parentTop - && windowTop + windowHeight <= parentTop + parentHeight) { - if ($button.css('position') !== 'fixed') { - /* - * Position this fixed in the center of the screen. - * It'll be less jumpy. - */ - $button.css({ - position: 'fixed', - left: $button.offset().left, - top: Math.round((windowHeight - - $button.outerHeight()) / 2) - }); - } - - /* - * Since the expanded chunk is taking up the whole screen, - * we have nothing else to process, so break. - */ - break; - } else { - y1 = Math.max(windowTop, parentTop); - y2 = Math.min(windowTop + windowHeight, - parentTop + parentHeight); - - /* - * The area doesn't take up the entire height of the - * view. Switch back to an absolute position. - */ - $button.css({ - position: 'absolute', - left: '', - top: y1 - parentTop + - Math.round((y2 - y1 - $button.outerHeight()) / 2) - }); - } - } - } + this._centered.updatePosition(); }, /* @@ -421,8 +338,12 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ } } - /* Recompute the list of buttons for later use. */ - this._$collapseButtons = this.$('.diff-collapse-btn'); + /* Recompute the set of buttons for later use. */ + this._centered.setElements(new Map( + Array.prototype.map.call( + this.$('.diff-collapse-btn'), + el => [el, $(el).closest('tbody')]) + )); this._updateCollapseButtonPos(); /* @@ -531,17 +452,6 @@ RB.DiffReviewableView = RB.AbstractReviewableView.extend({ } }, - /* - * Handler for when the window resizes. - * - * Updates the sizes of the diff columns, and the location of the - * collapse buttons (if one or more are visible). - */ - _onWindowResize: function() { - this._updateColumnSizes(); - this._updateCollapseButtonPos(); - }, - /* * Handler for when a file download link is clicked. * diff --git a/reviewboard/static/rb/js/ui/views/centeredElementManager.es6.js b/reviewboard/static/rb/js/ui/views/centeredElementManager.es6.js new file mode 100644 index 0000000000000000000000000000000000000000..6e388bf2d5b5ea198d025fccf291796bc4e22fb6 --- /dev/null +++ b/reviewboard/static/rb/js/ui/views/centeredElementManager.es6.js @@ -0,0 +1,99 @@ +/** + * A view which ensures that the specified elements are vertically centered. + */ +RB.CenteredElementManager = Backbone.View.extend({ + /** + * Initialize the view. + */ + initialize() { + this._elements = new Map(); + this._$window = $(window); + + this._updatePositionThrottled = _.throttle(() => this.updatePosition(), + 10); + + this._$window.on('resize', this._updatePositionThrottled); + this._$window.on('scroll', this._updatePositionThrottled); + }, + + /** + * Remove the CenteredElementManager. + * + * This will result in the event handlers being removed. + */ + remove() { + Backbone.View.prototype.remove.call(this); + + this._$window.off('resize', this._updatePositionThrottled); + this._$window.off('scroll', this._updatePositionThrottled); + }, + + /** + * Set the elements and their containers. + * + * Args: + * elements (Map): + * The elements to center within their respective containers. + */ + setElements(elements) { + this._elements = elements; + }, + + /** + * Update the position of the elements. + * + * This should only be done when the set of elements changed, as the view + * will handle updating on window resizing and scrolling. + */ + updatePosition() { + if (this._elements.size === 0) { + return; + } + + const windowTop = this._$window.scrollTop(); + const windowHeight = this._$window.height(); + const windowBottom = windowTop + windowHeight; + + this._elements.forEach(($container, el) => { + const $el = $(el); + const containerTop = $container.offset().top; + const containerHeight = $container.height(); + const containerBottom = containerTop + containerHeight; + + /* + * We don't have to vertically center the element when its + * container is not on screen. + */ + if (containerTop < windowBottom && containerBottom > windowTop) { + /* + * When a container takes up the entire viewport, we can switch + * the CSS to use position: fixed. This way, we do not have to + * re-compute its position. + */ + if (windowTop >= containerTop && + windowBottom <= containerBottom) { + if ($el.css('position') !== 'fixed') { + $el.css({ + position: 'fixed', + left: $el.offset().left, + top: Math.round( + (windowHeight - $el.outerHeight()) / 2), + }); + } + } else { + const top = Math.max(windowTop, containerTop); + const bottom = Math.min(windowBottom, containerBottom); + const elTop = top - containerTop + Math.round( + (bottom - top - $el.outerHeight()) / 2); + + $el.css({ + position: 'absolute', + left: '', + top: elTop, + }); + } + } + + }); + }, +}); diff --git a/reviewboard/static/rb/js/views/diffFragmentQueueView.js b/reviewboard/static/rb/js/views/diffFragmentQueueView.js index 1223739c8a9c96bc1733399858f7d0ca0d97478c..4f34ccb41671fdcb2023f565e3145e0b5f05562a 100644 --- a/reviewboard/static/rb/js/views/diffFragmentQueueView.js +++ b/reviewboard/static/rb/js/views/diffFragmentQueueView.js @@ -15,15 +15,11 @@ RB.DiffFragmentQueueView = Backbone.View.extend({ initialize: function() { this._queue = {}; - this._$window = $(window); - this._$collapseButtons = $(); + this._centered = new RB.CenteredElementManager(); - _.bindAll(this, '_onExpandOrCollapseFinished', '_onScrollOrResize', + _.bindAll(this, '_onExpandOrCollapseFinished', '_updateCollapseButtonPos', '_tryHideControlsDelayed', '_tryShowControlsDelayed'); - - this._$window.on('scroll', this._onScrollOrResize); - this._$window.on('resize', _.debounce(this._onScrollOrResize, 300)); }, /* @@ -32,10 +28,6 @@ RB.DiffFragmentQueueView = Backbone.View.extend({ remove: function() { RB.AbstractReviewableView.prototype.remove.call(this); - this._$collapseButtons = null; - - this._$window.off('scroll', this._updateCollapseButtonPos); - this._$window.off('resize', this._updateCollapseButtonPos); this._$window = null; this._active = null; }, @@ -111,13 +103,6 @@ RB.DiffFragmentQueueView = Backbone.View.extend({ this._expandOrCollapse($(e.target).closest('.diff-collapse-btn'), e); }, - /* - * Handle a scroll or resize by updating the button positions. - */ - _onScrollOrResize: function() { - this._updateCollapseButtonPos(); - }, - /* * Update the positions of the collapse buttons. * @@ -129,95 +114,7 @@ RB.DiffFragmentQueueView = Backbone.View.extend({ * with them. It will not, however, leave the confines of the table. */ _updateCollapseButtonPos: function() { - var windowTop, - windowHeight, - len = this._$collapseButtons.length, - $button, - $chunks, - $firstChunk, - $lastChunk, - chunkTop, - chunkBottom, - chunkHeight, - i, - top, - bottom; - - if (len === 0) { - return; - } - - windowTop = this._$window.scrollTop(); - windowHeight = this._$window.height(); - - for (i = 0; i < len; i++) { - $button = $(this._$collapseButtons[i]); - - /* - * We are only showing one button per table. If we try to use the - * table to position the element, we get jumpy behaviour. Instead - * we use the first and last expanded chunks in the table and - * position relative to them. - */ - $chunks = $button - .closest('.sidebyside') - .children('tbody') - .not('.diff-header'); - - $firstChunk = $($chunks.get(0)); - $lastChunk = $($chunks.get(-1)); - - chunkTop = $firstChunk.offset().top; - chunkBottom = $lastChunk.offset().top + $lastChunk.height(); - - // The effective height of the chunks we are working with. - chunkHeight = chunkBottom - chunkTop; - - if (chunkTop >= windowTop + windowHeight) { - // We've gone past the last visible button. - break; - } else if (chunkTop + chunkHeight <= windowTop) { - // We haven't reached a visible button yet. - } else { - if ( windowTop >= chunkTop - && windowTop + windowHeight <= chunkBottom) { - if ($button.css('position') !== 'fixed') { - /* - * Position in the center of the screen once so it will - * be less jumpy. - */ - $button.css({ - position: 'fixed', - left: $button.offset().left, - top: Math.round((windowHeight - - $button.outerHeight()) / 2) - }); - } - - /* - * The table is taking up the entire screen so we have - * nothing else to process. - */ - break; - } else { - top = Math.max(windowTop, chunkTop); - bottom = Math.min(windowTop + windowHeight, chunkBottom); - - /* - * The area doesn't take up the entire height of the view. - * Switch back to an absolute position. - */ - $button.css({ - position: 'absolute', - left: '', - top: top - chunkTop + - Math.round((bottom - top - - $button.outerHeight()) / 2) - }); - } - } - - } + this._centered.updatePosition(); }, /* @@ -302,8 +199,13 @@ RB.DiffFragmentQueueView = Backbone.View.extend({ */ _onExpandOrCollapseFinished: function(id) { var $expanded = this.$('#' + this.options.containerPrefix + '_' + id); - this._$collapseButtons = this.$('.diff-collapse-btn'); - this._updateCollapseButtonPos('table'); + + this._centered.setElements(new Map( + Array.prototype.map.call( + this.$('.diff-collapse-btn'), + el => [el, $(el).closest('table')]) + )); + this._updateCollapseButtonPos(); RB.setActivityIndicator(false, {}); diff --git a/reviewboard/staticbundles.py b/reviewboard/staticbundles.py index 9a029316fa39f8bb5fc3d32190616568c15ce9aa..6c2b7083d429acf5ce03514594ef5d73ab6c2940 100644 --- a/reviewboard/staticbundles.py +++ b/reviewboard/staticbundles.py @@ -165,6 +165,7 @@ PIPELINE_JAVASCRIPT = dict({ 'rb/js/resources/collections/resourceCollection.es6.js', 'rb/js/resources/collections/repositoryBranchesCollection.es6.js', 'rb/js/resources/collections/repositoryCommitsCollection.es6.js', + 'rb/js/ui/views/centeredElementManager.es6.js', 'rb/js/ui/views/dialogView.es6.js', 'rb/js/ui/views/infoboxView.es6.js', 'rb/js/ui/views/notificationManager.es6.js',