diff --git a/reviewboard/static/rb/css/pages/diffviewer.less b/reviewboard/static/rb/css/pages/diffviewer.less index fdd8b1699640c0e314d6c82111db2566109121a8..006c18ef48eb0a07d15af99a42e075ecf7b951b1 100644 --- a/reviewboard/static/rb/css/pages/diffviewer.less +++ b/reviewboard/static/rb/css/pages/diffviewer.less @@ -586,6 +586,31 @@ } } +#diff-banner { + .banner { + background: @review-request-bg; + border: 0 @review-request-border-color; + border-bottom: 1px solid @review-request-border-color; + box-shadow: @box-shadow; + left: 0; + margin-bottom: 0px; + position: fixed; + width: 100%; + z-index: @z-index-banner; + } +} + +#dropdown_button { + cursor: pointer; + display: inline-block; + background-color: @review-request-bg; + border: 1px solid @review-request-bg; + font-size: 18px; + line-height: normal; + padding: 1px 8px; + transition: .1s linear all; +} + #diffs { list-style: none; margin: 0; @@ -626,6 +651,7 @@ } #diff_index, +#diff_banner_index, .diff-index { @icon-size: 20px; @icon-offset: 2px; @@ -730,7 +756,8 @@ /**************************************************************************** * Diff information ****************************************************************************/ -#diff-details { +#diff-details, +#diff-banner-details { border-spacing: 8px; &.loading { diff --git a/reviewboard/static/rb/js/diffviewer/models/diffFileIndexModel.js b/reviewboard/static/rb/js/diffviewer/models/diffFileIndexModel.js new file mode 100644 index 0000000000000000000000000000000000000000..b898d4b3a46a6be4412a2b3b5871de78de8d7572 --- /dev/null +++ b/reviewboard/static/rb/js/diffviewer/models/diffFileIndexModel.js @@ -0,0 +1,19 @@ +/** + * A model with all the data for the DiffFileIndexView + */ +RB.DiffFileIndex = Backbone.Model.extend({ + defaults: { + diffReviewableViews: [], + index: null + }, + + /** + * Adds a reviewableDiffView to the list of reviewableDiffViews + * and updates the new index. + */ + addDiff: function(index, diffReviewableView) { + this.set('index', index); + this.get('diffReviewableViews').push(diffReviewableView); + this.trigger('diffAdded'); + } +}); \ No newline at end of file diff --git a/reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js b/reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js index b7034694ecf8b68d87960a3fe790ba3e9cd1fa7d..39bdb4e507cef06226415df6fed285434fc1f9ea 100644 --- a/reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js +++ b/reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js @@ -1,4 +1,4 @@ -/* +/** * Displays the file index for the diffs on a page. * * The file page lists the names of the files, as well as a little graph @@ -14,7 +14,7 @@ RB.DiffFileIndexView = Backbone.View.extend({ 'click a': '_onAnchorClicked' }, - /* + /** * Initializes the view. */ initialize: function() { @@ -22,10 +22,18 @@ RB.DiffFileIndexView = Backbone.View.extend({ this._$itemsTable = null; this.collection = this.options.collection; + this.viewType = this.options.viewType; + + this._$toggleButton = null; + this._toggle = null; + + _.bindAll(this, '_updateBannerDiffFile', '_toggleBannerDiffFileList'); + this.listenTo(this.collection, 'update', this.update); + this.listenTo(this.model, 'diffAdded', this._diffAdded); }, - /* + /** * Renders the view to the page. */ render: function() { @@ -33,9 +41,24 @@ RB.DiffFileIndexView = Backbone.View.extend({ this._$itemsTable = $('').appendTo(this.$el); this._$items = this.$('tr'); - // Add the files from the collection + /* Add the files from the collection */ this.update(); + /* Add a click event and a scroll event to the banner's index view */ + if (this.viewType === "banner") { + this._toggle = false; + this._$toggleButton = $('#dropdown_button'); + this._$toggleButton.on('click', _.bind( + this._toggleBannerDiffFileList, this)); + + $(window).on('scroll', _.debounce(_.bind(function() { + requestAnimationFrame(this._updateBannerDiffFile); + }, this), 20) + ); + + this._$items.hide(); + } + return this; }, @@ -67,7 +90,7 @@ RB.DiffFileIndexView = Backbone.View.extend({ '' ].join('')), - /* + /** * Update the list of files in the index view. */ update: function() { @@ -83,27 +106,93 @@ RB.DiffFileIndexView = Backbone.View.extend({ }, file.attributes) )); }, this); + this._$items = this.$('tr'); }, - /* + /** + * Updates the banner's default diff file with the file that + * is currently intersecting the windowTop and windowBottom. + */ + _updateBannerDiffFile: function() { + var windowTop, + windowBottom, + diffFileId, + diffFileContainer, + diffFileContainerMargin, + containerTop, + containerBottom, + $bannerTableRow, + i; + + windowTop = $(window).scrollTop(); + windowBottom = windowTop + $(window).height(); + + if (!this._toggle) { + this._$items.hide(); + } + this._$items.removeAttr('class'); + + for (i = 0; i < this.collection.size(); i++) { + diffFileId = '#file' + this.collection.models[i].attributes.id; + diffFileContainer = $(diffFileId).parent().parent(); + diffFileContainerMargin = parseInt(diffFileContainer.css('margin-bottom')); + containerTop = diffFileContainer.offset().top; + if (RB.FloatingBannerIndexView.instance) { + containerTop -= diffFileContainerMargin; + containerTop -= RB.FloatingBannerIndexView.instance.getHeight(); + } + containerBottom = containerTop + diffFileContainer.outerHeight(true); + $bannerTableRow = this._$items.eq(i); + + /* Show the table row only if it intersects with the top of the window */ + if (containerTop <= windowTop && containerBottom >= windowTop || + containerTop >= windowTop && containerTop <= windowBottom) { + if (!this._toggle) { + $bannerTableRow.show(); + } + $bannerTableRow.attr('class', 'banner-active-file'); + break; + } + } + }, + + /** + * Expands or collapses all diff files in the banner. + */ + _toggleBannerDiffFileList: function() { + if (this._toggle) { + /* Collapse the list */ + this._$items.hide(); + $('.banner-active-file').show(); + this._toggle = false; + } else { + /* Expand the list */ + this._$items.show(); + this._toggle = true; + } + }, + + /** * Adds a loaded diff to the index. * * The reserved entry for the diff will be populated with a link to the * diff, and information about the diff. */ - addDiff: function(index, diffReviewableView) { - var $item = $(this._$items[index]) + _diffAdded: function() { + var index = this.model.get('index'), + diffReviewableViews = this.model.get('diffReviewableViews'), + $item = $(this._$items[index]) .removeClass('loading'); - if (diffReviewableView.$el.hasClass('diff-error')) { + if (diffReviewableViews[index].$el.hasClass('diff-error')){ this._renderDiffError($item); } else { - this._renderDiffEntry($item, diffReviewableView); + this._renderDiffEntry($item, diffReviewableViews[index]); } }, - /* + /** * Renders a diff loading error. * * An error icon will be displayed in place of the typical complexity @@ -118,7 +207,7 @@ RB.DiffFileIndexView = Backbone.View.extend({ gettext('There was an error loading this diff. See the details below.')); }, - /* + /** * Renders the display of a loaded diff. */ _renderDiffEntry: function($item, diffReviewableView) { @@ -213,14 +302,23 @@ RB.DiffFileIndexView = Backbone.View.extend({ }); }, - /* + /** * Handler for when an anchor is clicked. * + * Collapses the diff file list in the banner if the + * anchor click belongs to the banner. + * * Gets the name of the target and emits anchorClicked. */ _onAnchorClicked: function(e) { e.preventDefault(); + if (this.viewType === "banner") { + if (this._toggle) { + this._toggleBannerDiffFileList(); + } + } + this.trigger('anchorClicked', e.target.href.split('#')[1]); } }); diff --git a/reviewboard/static/rb/js/diffviewer/views/floatingBannerIndexView.js b/reviewboard/static/rb/js/diffviewer/views/floatingBannerIndexView.js new file mode 100644 index 0000000000000000000000000000000000000000..480cd7f34dde08a18e9f69812a0679b7cbe63d83 --- /dev/null +++ b/reviewboard/static/rb/js/diffviewer/views/floatingBannerIndexView.js @@ -0,0 +1,69 @@ +/** + * This view extends the FloatingBannerView. + * + * This floating banner contains a diffFileIndexView so the user + * can navigate to diff files through the floating banner rather + * than having to scroll all the way to the top of the page. + */ +RB.FloatingBannerIndexView = RB.FloatingBannerView.extend({ + initialize: function() { + _super(this).initialize.call(this); + + this._diffFileIndexView = null; + + this.collection = this.options.collection; + }, + + events: { + 'click a': '_onAnchorClicked' + }, + + /** + * Renders the floating banner containing a diffFileIndexView. + */ + render: function() { + _super(this).render.call(this); + + this._diffFileIndexView = new RB.DiffFileIndexView({ + el: $('#diff_banner_index'), + model: this.model, + collection: this.collection, + viewType: "banner" + }); + this._diffFileIndexView.render(); + + return this; + }, + + /** + * Handler for when an anchor is clicked. + * + * Gets the name of the target and emits bannerAnchorClicked. + */ + _onAnchorClicked: function _onAnchorClicked(e) { + e.preventDefault(); + + this.trigger('bannerAnchorClicked', e.target.href.split('#')[1]); + }, + + /** + * Returns the height of the banner. + */ + getHeight: function() { + return this.$el.outerHeight(); + } +}, { + instance: null, + + /** + * Creates the floating banner index view singleton. + */ + create: function (options) { + if (!this.instance) { + this.instance = new RB.FloatingBannerIndexView(options); + this.instance.render(); + } + + return this.instance; + } +}); \ No newline at end of file diff --git a/reviewboard/static/rb/js/diffviewer/views/tests/floatingBannerIndexViewTests.js b/reviewboard/static/rb/js/diffviewer/views/tests/floatingBannerIndexViewTests.js new file mode 100644 index 0000000000000000000000000000000000000000..d654a81f5ea24c6ac0f5b9c0a88e263bf6a58d06 --- /dev/null +++ b/reviewboard/static/rb/js/diffviewer/views/tests/floatingBannerIndexViewTests.js @@ -0,0 +1,218 @@ +suite('rb/diffviewer/views/floatingBannerIndexView', function() { + var diffTableTemplate = _.template([ + '
', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' <% _.each(chunks, function(chunk, index) { %>', + ' <% if (chunk.type === "collapsed") { %>', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' <% } else { %>', + ' "', + ' id="chunk0.<%= index %>">', + ' <% for (var i = 0; i < chunk.numRows; i++) { %>', + ' ', + ' ', + ' ', + ' ', + ' ', + ' ', + ' <% } %>', + ' ', + ' <% } %>', + ' <% }); %>', + '
', + ' my-file.txt', + '
Revision 1Revision 2
', + ' ', + ' ', + ' Expand', + '
', + ' ', + ' ', + ' ', + ' Some Function', + ' ', + '
', + ' <% if (chunk.expanded && i === 0) { %>', + '
', + ' ', + '
', + ' <% } %>', + '
' + ].join('')), + pageModel, + diffFileCollection, + model, + view, + reviewRequest = new RB.ReviewRequest(); + + beforeEach(function() { + pageModel = new RB.DiffViewerPageModel(); + model = new RB.DiffFileIndex(); + view = new RB.FloatingBannerIndexView({ + el: $('#diff-banner').find('.banner'), + model: model, + collection: pageModel.get('files'), + $floatContainer: $('#diffs') + }); + }); + + afterEach(function() { + view.remove(); + }); + + describe('Initialization', function() { + it('Defined', function() { + expect(view).toBeDefined(); + expect(view.$el).toBeDefined(); + expect(view.collection).toBeDefined(); + expect(view.model).toBeDefined(); + }); + }); + + describe('Diff Files', function() { + var numFiles = 5; + + beforeEach(function() { + var diffFileIndexModel = new RB.DiffFileIndex(); + diffFileCollection = new RB.DiffFileCollection(); + + for (var i = 0; i < numFiles; i++) { + var diffFile = new RB.DiffFile({ + newfile: true, + filediff: 'foo', + index: i + }), + diffReviewable = new RB.DiffReviewable({ + fileIndex: diffFile.get('index'), + fileDiffID: diffFile.get('filediff'), + revision: 1, + reviewRequest: reviewRequest + }), + diffReviewableView = new RB.DiffReviewableView({ + model: diffReviewable, + el: $(diffTableTemplate({ + chunks: [ + { + type: 'equal', + startRow: 1, + numRows: 5 + }, + { + type: 'collapsed', + expandHeaderLines: 7 + }, + { + type: 'delete', + startRow: 10, + numRows: 5 + } + ] + })) + }); + + diffFileCollection.add(diffFile); + diffFileIndexModel.addDiff(diffFile.get('index'), diffReviewableView); + } + + pageModel.set({ + numDiffs: numFiles, + files: diffFileCollection, + diffFileIndex: diffFileIndexModel + }); + + view = new RB.FloatingBannerIndexView({ + el: $('#diff-banner').find('.banner'), + model: model, + collection: pageModel.get('files'), + $floatContainer: $('#diffs') + }); + }); + + it('Added', function() { + expect(diffFileCollection.length).toBe(numFiles); + expect(pageModel.get('numDiffs')).toBe(numFiles); + expect(pageModel.get('files').length).toBe(numFiles); + }); + + describe("Rendered Banner", function() { + beforeEach(function(){ + view.render(); + }); + + it('Render', function() { + var tr = view.$el.find(''); + + expect(view.$el.find('')).toBeDefined(); + expect(tr.find('td.diff-file-icon')).toBeDefined(); + expect(tr.find('td.diff-file-info')).toBeDefined(); + expect(tr.find('td.diff-chunks-cell').find('.diff-chunks')).toBeDefined(); + expect(view.$el.is(':visible')).toBe(false); + }); + + it('Banner Visibility', function() { + window.scroll(0, 2000); + var bannerRows = view.$el.find(''); + + expect(view.$el.is(':hidden')).toBe(false); + expect(bannerRows.length).toBe(5); + expect(bannerRows.find('.banner-active-file').length).toBe(1); + }); + + it('Expand List', function() { + var button = view.$el.find('#dropdown-button'), + bannerRows = view.$el.find(''), + counter = 0; + + button.click(); + bannerRows.each(function(row) { + expect(row.is(':visible')).toBe(true); + }); + + button.click(); + bannerRows.each(function(row) { + if (row.is(':visible')) { + counter++; + } + }); + expect(counter).toBe(1); + }); + + // it('Navigation', function() { + // var anchor = view.$el.find('.banner-active-file').find(''), + // windowPosition = window.location; + // + // + // }); + + + }); + }); +}); \ No newline at end of file diff --git a/reviewboard/static/rb/js/pages/models/diffViewerPageModel.js b/reviewboard/static/rb/js/pages/models/diffViewerPageModel.js index 7b5aed1a5b1eff93870bdbd4c30d8bde4c901d89..c2b2cb171ebed6edc27eb31c1167d70eb8a18de2 100644 --- a/reviewboard/static/rb/js/pages/models/diffViewerPageModel.js +++ b/reviewboard/static/rb/js/pages/models/diffViewerPageModel.js @@ -1,4 +1,4 @@ -/* +/** * A model with all the data for the diff viewer page. */ RB.DiffViewerPageModel = Backbone.Model.extend({ @@ -7,10 +7,11 @@ RB.DiffViewerPageModel = Backbone.Model.extend({ files: null, numDiffs: 1, pagination: null, - revision: null + revision: null, + diffFileIndex: null }, - /* + /** * Parse the data given to us by the server. */ parse: function(rsp) { @@ -20,11 +21,12 @@ RB.DiffViewerPageModel = Backbone.Model.extend({ files: new RB.DiffFileCollection(rsp.files, {parse: true}), numDiffs: rsp.num_diffs, pagination: new RB.Pagination(rsp.pagination, {parse: true}), - revision: new RB.DiffRevision(rsp.revision, {parse: true}) + revision: new RB.DiffRevision(rsp.revision, {parse: true}), + diffFileIndex: new RB.DiffFileIndex() }; }, - /* + /** * Override for Model.set that properly updates the child objects. * * Because several of this model's properties are other backbone models, we @@ -64,6 +66,12 @@ RB.DiffViewerPageModel = Backbone.Model.extend({ toSet.revision = attrs.revision; } + if (this.attributes.diffFileIndex){ + this.attributes.diffFileIndex.set(attrs.diffFileIndex.attributes); + } else { + toSet.diffFileIndex = attrs.diffFileIndex; + } + Backbone.Model.prototype.set.call(this, toSet, options); } }); diff --git a/reviewboard/static/rb/js/pages/views/diffViewerPageView.js b/reviewboard/static/rb/js/pages/views/diffViewerPageView.js index f919a38618e7bec1c78f936fb832b1c3bceb8d8c..bb7071a3ad3b67013d3fbf8d3683a33d36e51a3a 100644 --- a/reviewboard/static/rb/js/pages/views/diffViewerPageView.js +++ b/reviewboard/static/rb/js/pages/views/diffViewerPageView.js @@ -1,4 +1,4 @@ -/* +/** * Manages the diff viewer page. * * This provides functionality for the diff viewer page for managing the @@ -12,8 +12,6 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ ANCHOR_FILE: 2, ANCHOR_CHUNK: 4, - DIFF_SCROLLDOWN_AMOUNT: 15, - keyBindings: { 'aAKP': '_selectNextFile', @@ -30,7 +28,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ 'click .toggle-show-whitespace': '_toggleShowExtraWhitespace' }, RB.ReviewablePageView.prototype.events), - /* + /** * Initializes the diff viewer page. */ initialize: function(options) { @@ -50,6 +48,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._diffReviewableViews = []; this._diffFileIndexView = null; this._highlightedChunk = null; + this._floatingBannerIndexView = null; this.listenTo(this.model.get('files'), 'update', this._setFiles); @@ -81,7 +80,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ } }); - /* + /** * Begin managing the URL history for the page, so that we can * switch revisions and handle pagination while keeping the history * clean and the URLs representative of the current state. @@ -102,7 +101,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ silent: true }); - /* + /** * Navigating here accomplishes two things: * * 1. The user may have viewed diff/, and not diff//, but @@ -128,7 +127,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ }); }, - /* + /** * Removes the view from the page. */ remove: function() { @@ -137,13 +136,18 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._diffFileIndexView.remove(); }, - /* + /** * Renders the page and begins loading all diffs. */ render: function() { var numDiffs = this.model.get('numDiffs'), revisionModel = this.model.get('revision'), - $diffs = $('#diffs'); + diffFileIndex = this.model.get('diffFileIndex'), + filesCollection = this.model.get('files'), + pagination = this.model.get('pagination'), + commentsHint = this.model.get('commentsHint'), + $diffs = $('#diffs'), + $diffBanner = $('#diff-banner'); _super(this).render.call(this); @@ -151,7 +155,9 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._diffFileIndexView = new RB.DiffFileIndexView({ el: $('#diff_index'), - collection: this.model.get('files') + model: diffFileIndex, + collection: filesCollection, + viewType: "page" }); this._diffFileIndexView.render(); @@ -182,7 +188,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._commentsHintModel = this.options.commentsHint; this._commentsHintView = new RB.DiffCommentsHintView({ el: $('#diff_comments_hint'), - model: this.model.get('commentsHint') + model: commentsHint }); this._commentsHintView.render(); this.listenTo(this._commentsHintView, 'revisionSelected', @@ -190,7 +196,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._paginationView1 = new RB.PaginationView({ el: $('#pagination1'), - model: this.model.get('pagination') + model: pagination }); this._paginationView1.render(); this.listenTo(this._paginationView1, 'pageSelected', @@ -198,7 +204,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._paginationView2 = new RB.PaginationView({ el: $('#pagination2'), - model: this.model.get('pagination') + model: pagination }); this._paginationView2.render(); this.listenTo(this._paginationView2, 'pageSelected', @@ -212,7 +218,19 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._chunkHighlighter = new RB.ChunkHighlighterView(); this._chunkHighlighter.render().$el.prependTo($diffs); + this._floatingBannerIndexView = RB.FloatingBannerIndexView.create({ + el: $diffBanner.find('.banner'), + model: diffFileIndex, + collection: this.model.get('files'), + $floatContainer: $diffs + }); + this._floatingBannerIndexView.render().$el.prependTo($diffBanner); + + this.listenTo(this._floatingBannerIndexView, 'bannerAnchorClicked', + this.selectAnchorByName); + $('#diff-details').removeClass('loading'); + $('#diff-banner-details').removeClass('loading'); return this; }, @@ -239,7 +257,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ anchorTemplate: _.template( ''), - /* + /** * Set the displayed files. * * This will replace the displayed files with a set of pending entries, @@ -305,13 +323,13 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ $.funcQueue('diff_files').add(function() { if (!options.showDeleted && $('#file' + fileDiffID).length === 1) { - /* + /** * We already have this diff (probably pre-loaded), and we * don't want to requeue it to show its deleted content. */ this._renderFileDiff(diffReviewable); } else { - /* + /** * We either want to queue this diff for the first time, or we * want to requeue it to show its deleted content. */ @@ -326,7 +344,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ }, this); }, - /* + /** * Sets up a diff as DiffReviewableView and renders it. * * This will set up a DiffReviewableView for the given diffReviewable. @@ -339,11 +357,12 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ var elementName = 'file' + diffReviewable.get('fileDiffID'), $el = $('#' + elementName), diffReviewableView, + diffFileIndex = this.model.get('diffFileIndex'), $anchor, urlSplit; if ($el.length === 0) { - /* + /** * The user changed revisions before the file finished loading, and * the target element no longer exists. Just return. */ @@ -356,7 +375,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ model: diffReviewable }); - this._diffFileIndexView.addDiff(this._diffReviewableViews.length, + diffFileIndex.addDiff(this._diffReviewableViews.length, diffReviewableView); this._diffReviewableViews.push(diffReviewableView); @@ -386,7 +405,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ /* See if we've loaded the anchor the user wants to start at. */ $anchor = $('a[name="' + this._startAtAnchorName + '"]'); - /* + /** * Some anchors are added by the template (such as those at * comment locations), but not all are. If the anchor isn't found, * but the URL hash is indicating that we want to start at a @@ -420,14 +439,14 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ $.funcQueue('diff_files').next(); }, - /* + /** * Selects the anchor at a specified location. * * By default, this will scroll the page to position the anchor near * the top of the view. */ selectAnchor: function($anchor, scroll) { - var scrollAmount, + var scrollAmount = 0, i, url; @@ -449,11 +468,12 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ trigger: false }); - scrollAmount = this.DIFF_SCROLLDOWN_AMOUNT; - if (RB.DraftReviewBannerView.instance) { scrollAmount += RB.DraftReviewBannerView.instance.getHeight(); } + if (RB.FloatingBannerIndexView.instance) { + scrollAmount += RB.FloatingBannerIndexView.instance.getHeight(); + } $(window).scrollTop($anchor.offset().top - scrollAmount); } @@ -470,14 +490,14 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ return true; }, - /* + /** * Selects an anchor by name. */ selectAnchorByName: function(name, scroll) { return this.selectAnchor($('a[name="' + name + '"]'), scroll); }, - /* + /** * Highlights a chunk bound to an anchor element. */ _highlightAnchor: function($anchor) { @@ -486,7 +506,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ $anchor.parents('tbody:first, thead:first')); }, - /* + /** * Updates the list of known anchors based on named anchors in the * specified table. This is called after every part of the diff that we * loaded. @@ -503,7 +523,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ } }, - /* + /** * Returns the next navigatable anchor in the specified direction of * the given types. * @@ -538,7 +558,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ return null; }, - /* + /** * Selects the previous file's header on the page. */ _selectPreviousFile: function() { @@ -546,7 +566,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this.ANCHOR_FILE)); }, - /* + /** * Selects the next file's header on the page. */ _selectNextFile: function() { @@ -554,7 +574,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this.ANCHOR_FILE)); }, - /* + /** * Selects the previous diff chunk on the page. */ _selectPreviousDiff: function() { @@ -563,7 +583,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this.ANCHOR_CHUNK | this.ANCHOR_FILE)); }, - /* + /** * Selects the next diff chunk on the page. */ _selectNextDiff: function() { @@ -572,7 +592,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this.ANCHOR_CHUNK | this.ANCHOR_FILE)); }, - /* + /** * Selects the previous comment on the page. */ _selectPreviousComment: function() { @@ -580,7 +600,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._getNextAnchor(this.SCROLL_BACKWARD, this.ANCHOR_COMMENT)); }, - /* + /** * Selects the next comment on the page. */ _selectNextComment: function() { @@ -588,14 +608,14 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this._getNextAnchor(this.SCROLL_FORWARD, this.ANCHOR_COMMENT)); }, - /* + /** * Re-centers the currently selected area on the page. */ _recenterSelected: function() { this.selectAnchor($(this._$anchors[this._selectedAnchorIndex])); }, - /* + /** * Creates a comment for a chunk of a diff */ _createComment: function() { @@ -624,7 +644,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ } }, - /* + /** * Toggles the display of diff chunks that only contain whitespace changes. */ _toggleWhitespaceOnlyChunks: function() { @@ -637,7 +657,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ return false; }, - /* + /** * Toggles the display of extra whitespace highlights on diffs. * * A cookie will be set to the new whitespace display setting, so that @@ -650,7 +670,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ return false; }, - /* + /** * Callback for when a new revision is selected. * * This supports both single revisions and interdiffs. If `base` is 0, a @@ -670,7 +690,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ } }, - /* + /** * Get the current URL. * * This will compute and return the current page's URL (relative to the @@ -687,7 +707,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ return url; }, - /* + /** * Callback for when a new page is selected. * * Navigates to the same revision with a different page number. @@ -703,7 +723,7 @@ RB.DiffViewerPageView = RB.ReviewablePageView.extend({ this.router.navigate(url, {trigger: true}); }, - /* + /** * Load a given revision. * * This supports both single revisions and interdiffs. If `base` is 0, a diff --git a/reviewboard/static/rb/js/views/draftReviewBannerView.js b/reviewboard/static/rb/js/views/draftReviewBannerView.js index 44e8d099bccad69ee718e080c0d6463f6bc9bdb7..b640f259655d55dccf303345d8e418d79de903da 100644 --- a/reviewboard/static/rb/js/views/draftReviewBannerView.js +++ b/reviewboard/static/rb/js/views/draftReviewBannerView.js @@ -1,4 +1,4 @@ -/* +/** * A banner that represents a pending draft review. * * The banner displays at the top of the page and provides buttons for @@ -12,7 +12,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ 'click #review-banner-discard': '_onDiscardClicked' }, - /* + /** * Returns the height of the banner. */ getHeight: function() { @@ -66,7 +66,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ return this; }, - /* + /** * Shows the banner. * * The banner will appear to slide down from the top of the page. @@ -80,7 +80,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ } }, - /* + /** * Hides the banner. * * The banner will slide up to the top of the page. @@ -96,14 +96,14 @@ RB.DraftReviewBannerView = Backbone.View.extend({ } }, - /* + /** * Hides the banner and reloads the page. * * XXX Remove this function when we make the pages more dynamic. */ hideAndReload: function() { this.hide(function() { - /* + /** * hideAndReload might have been called from within a $.funcQueue. * With Firefox, later async functions that are queued in the * $.funcQueue will not run when we change window.location, which @@ -117,7 +117,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ }, this); }, - /* + /** * Handler for the Edit Review button. * * Displays the review editor dialog. @@ -131,7 +131,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ return false; }, - /* + /** * Handler for the Publish button. * * Publishes the review. @@ -143,7 +143,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ return false; }, - /* + /** * Handler for the Discard button. * * Prompts the user to confirm that they want the review discarded. @@ -172,7 +172,7 @@ RB.DraftReviewBannerView = Backbone.View.extend({ }, { instance: null, - /* + /** * Creates the draft review banner singleton. */ create: function(options) { diff --git a/reviewboard/static/rb/js/views/floatingBannerView.js b/reviewboard/static/rb/js/views/floatingBannerView.js index e96f2f880a4a94beefe8614ae0837f9c60e0decb..e60a4d8ed465396e238dd22a09dfaa2031c0dff8 100644 --- a/reviewboard/static/rb/js/views/floatingBannerView.js +++ b/reviewboard/static/rb/js/views/floatingBannerView.js @@ -1,4 +1,4 @@ -/* +/** * Floats a banner on screen within a container. * * The banner will appear at the top of the container, or the screen, @@ -15,7 +15,7 @@ RB.FloatingBannerView = Backbone.View.extend({ _.bindAll(this, '_updateFloatPosition', '_updateSize'); }, - /* + /** * Renders the banner and listens for scroll and resize updates. */ render: function() { @@ -27,7 +27,7 @@ RB.FloatingBannerView = Backbone.View.extend({ return this; }, - /* + /** * Updates the size of the banner to match the spacer. */ _updateSize: function() { @@ -35,7 +35,8 @@ RB.FloatingBannerView = Backbone.View.extend({ if (this._$floatSpacer !== null) { if (this.$el.hasClass('floating')) { - rect = this._$floatSpacer.parent()[0].getBoundingClientRect(); + //get the width of the page-container div + rect = this._$floatSpacer.parents()[3].getBoundingClientRect(); this.$el.width( Math.ceil(rect.width) - @@ -46,7 +47,7 @@ RB.FloatingBannerView = Backbone.View.extend({ } }, - /* + /** * Updates the position of the banner. * * This will factor in how much of the container is visible, based on @@ -88,7 +89,7 @@ RB.FloatingBannerView = Backbone.View.extend({ topOffset < 0 && containerTop < windowTop && windowTop < containerBottom) { - /* + /** * We're floating! If we just entered this state, set the * appropriate styles on the element. * @@ -97,18 +98,19 @@ RB.FloatingBannerView = Backbone.View.extend({ * much to show, and set the appropriate offset. */ if (!wasFloating) { - /* + /** * Set the spacer to be the dimensions of the docked banner, * so that the container doesn't change sizes when we go into * float mode. */ this._$floatSpacer - .height(this.$el.outerHeight()) .css({ 'margin-top': this.$el.css('margin-top'), 'margin-bottom': this.$el.css('margin-bottom') }); + this.$el.show(); + this.$el .addClass('floating') .css('position', 'fixed'); @@ -121,7 +123,7 @@ RB.FloatingBannerView = Backbone.View.extend({ this._updateSize(); } else if (wasFloating) { - /* + /** * We're now longer floating. Unset the styles on the banner and * on the spacer (in order to prevent the spacer from taking up * any additional room. @@ -132,6 +134,7 @@ RB.FloatingBannerView = Backbone.View.extend({ top: '', position: '' }); + this.$el.hide(); this._$floatSpacer .height('auto') .css('margin', 0); diff --git a/reviewboard/staticbundles.py b/reviewboard/staticbundles.py index bb42045379c745182364fcca4801064509531fba..56f8effd2fd4adcc6f973a228042193a16e02967 100644 --- a/reviewboard/staticbundles.py +++ b/reviewboard/staticbundles.py @@ -55,6 +55,7 @@ PIPELINE_JAVASCRIPT = dict({ 'rb/js/diffviewer/models/tests/diffRevisionModelTests.js', 'rb/js/diffviewer/models/tests/paginationModelTests.js', 'rb/js/diffviewer/views/tests/diffReviewableViewTests.js', + 'rb/js/diffviewer/views/tests/floatingBannerIndexViewTests.js', 'rb/js/models/tests/commentEditorModelTests.js', 'rb/js/models/tests/extraDataTests.js', 'rb/js/models/tests/reviewReplyEditorModelTests.js', @@ -264,6 +265,7 @@ PIPELINE_JAVASCRIPT = dict({ 'rb/js/views/updateDiffView.js', 'rb/js/diffviewer/models/diffCommentBlockModel.es6.js', 'rb/js/diffviewer/models/diffCommentsHintModel.js', + 'rb/js/diffviewer/models/diffFileIndexModel.js', 'rb/js/diffviewer/models/diffFileModel.js', 'rb/js/diffviewer/models/diffReviewableModel.es6.js', 'rb/js/diffviewer/models/diffRevisionModel.js', @@ -277,6 +279,7 @@ PIPELINE_JAVASCRIPT = dict({ 'rb/js/diffviewer/views/diffReviewableView.es6.js', 'rb/js/diffviewer/views/diffRevisionLabelView.js', 'rb/js/diffviewer/views/diffRevisionSelectorView.js', + 'rb/js/diffviewer/views/floatingBannerIndexView.js', 'rb/js/diffviewer/views/paginationView.js', ), 'output_filename': 'rb/js/reviews.min.js', diff --git a/reviewboard/templates/diffviewer/view_diff.html b/reviewboard/templates/diffviewer/view_diff.html index 4dfb5dbe2afe4b70ad3013786c43bbb63c25bfe3..b00a9edda3948f3da1c2b168c7d13f5c7c409e7e 100644 --- a/reviewboard/templates/diffviewer/view_diff.html +++ b/reviewboard/templates/diffviewer/view_diff.html @@ -61,6 +61,15 @@ +
+ +