Support “diff against” in the file list Bug: Issue 3936 Change-Id: I07f242a8cb7be063a2d56dbe3f4b6a28143588c0
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html index 8c6c216..1f5d9e1 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -273,7 +273,7 @@ </section> <gr-file-list id="fileList" change-num="[[_changeNum]]" - patch-num="{{_patchRange.patchNum}}" + patch-range="[[_patchRange]]" comments="[[_comments]]" drafts="[[_diffDrafts]]" revisions="[[_change.revisions]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index a8012a0..a3d2146 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -179,7 +179,14 @@ basePatchNum: value.basePatchNum || 'PARENT', }; - this._resetStateIfNecessary(); + // If the change number or patch range is different, then reset the + // selected file index. + var patchRangeState = this.viewState.patchRange; + if (this.viewState.changeNum !== this._changeNum || + patchRangeState.basePatchNum !== this._patchRange.basePatchNum || + patchRangeState.patchNum !== this._patchRange.patchNum) { + this._resetFileListViewState(); + } if (!this._changeNum) { return; @@ -212,17 +219,10 @@ }.bind(this)); }, - _resetStateIfNecessary: function() { - // If the change number or patch range is different, then reset the - // selected file index. - if (this.viewState.changeNum !== this._changeNum || - this.viewState.patchRange.basePatchNum !== - this._patchRange.basePatchNum || - this.viewState.patchRange.patchNum !== this._patchRange.patchNum) { - this.set('viewState.selectedFileIndex', 0); - this.set('viewState.changeNum', this._changeNum); - this.set('viewState.patchRange', this._patchRange); - } + _resetFileListViewState: function() { + this.set('viewState.selectedFileIndex', 0); + this.set('viewState.changeNum', this._changeNum); + this.set('viewState.patchRange', this._patchRange); }, _changeChanged: function(change) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html index 104b27f2..7ce687c 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -34,7 +34,7 @@ justify-content: space-between; margin-bottom: .5em; } - header label { + .diffAgainst { font-weight: normal; } .positionIndicator, @@ -125,12 +125,22 @@ </style> <header> <div>Files</div> - <label hidden> - Diff against - <select></select> - </label> + <div class="diffAgainst"> + <label> + Diff against + <select on-change="_handlePatchChange"> + <option value="PARENT">Base</option> + <template is="dom-repeat" items="[[_computePatchSets(revisions, patchRange.*)]]" as="patchNum"> + <option + value$="[[patchNum]]" + selected$="[[_computePatchSetSelected(patchNum, patchRange.basePatchNum)]]" + disabled$="[[_computePatchSetDisabled(patchNum, patchRange.patchNum)]]">[[patchNum]]</option> + </template> + </select> + </label> + </div> </header> - <template is="dom-repeat" items="{{_files}}" as="file"> + <template is="dom-repeat" items="[[_files]]" as="file"> <div class="row" selected$="[[_computeFileSelected(index, selectedIndex)]]"> <div class="positionIndicator">▶</div> <div class="reviewed" hidden$="[[!_loggedIn]]" hidden> @@ -140,7 +150,7 @@ <div class$="[[_computeClass('status', file.__path)]]"> [[_computeFileStatus(file.status)]] </div> - <a class="path" href$="[[_computeDiffURL(changeNum, patchNum, file.__path)]]"> + <a class="path" href$="[[_computeDiffURL(changeNum, patchRange, file.__path)]]"> <div title$="[[_computeFileDisplayName(file.__path)]]"> [[_computeFileDisplayName(file.__path)]] </div> @@ -150,8 +160,8 @@ </div> </a> <div class="comments"> - <span class="drafts">[[_computeDraftsString(drafts, patchNum, file.__path)]]</span> - [[_computeCommentsString(comments, patchNum, file.__path)]] + <span class="drafts">[[_computeDraftsString(drafts, patchRange.patchNum, file.__path)]]</span> + [[_computeCommentsString(comments, patchRange.patchNum, file.__path)]] </div> <div class$="[[_computeClass('stats', file.__path)]]"> <span class="added">+[[file.lines_inserted]]</span>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js index 47b6abd..a9ce44e 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -20,6 +20,7 @@ is: 'gr-file-list', properties: { + patchRange: Object, patchNum: String, changeNum: String, comments: Object, @@ -50,7 +51,7 @@ ], reload: function() { - if (!this.changeNum || !this.patchNum) { + if (!this.changeNum || !this.patchRange.patchNum) { return Promise.resolve(); } @@ -73,6 +74,28 @@ return Promise.all(promises); }, + _computePatchSets: function(revisions) { + var patchNums = []; + for (var commit in revisions) { + patchNums.push(revisions[commit]._number); + } + return patchNums.sort(function(a, b) { return a - b; }); + }, + + _computePatchSetDisabled: function(patchNum, currentPatchNum) { + return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10); + }, + + _computePatchSetSelected: function(patchNum, basePatchNum) { + return parseInt(patchNum, 10) === parseInt(basePatchNum, 10); + }, + + _handlePatchChange: function(e) { + this.set('patchRange.basePatchNum', Polymer.dom(e).rootTarget.value); + page.show('/c/' + encodeURIComponent(this.changeNum) + '/' + + encodeURIComponent(this._patchRangeStr(this.patchRange))); + }, + _computeCommentsString: function(comments, patchNum, path) { return this._computeCountString(comments, patchNum, path, 'comment'); }, @@ -114,8 +137,8 @@ }, _saveReviewedState: function(path, reviewed) { - return this.$.restAPI.saveFileReviewed(this.changeNum, this.patchNum, - path, reviewed); + return this.$.restAPI.saveFileReviewed(this.changeNum, + this.patchRange.patchNum, path, reviewed); }, _getLoggedIn: function() { @@ -123,12 +146,13 @@ }, _getReviewedFiles: function() { - return this.$.restAPI.getReviewedFiles(this.changeNum, this.patchNum); + return this.$.restAPI.getReviewedFiles(this.changeNum, + this.patchRange.patchNum); }, _getFiles: function() { return this.$.restAPI.getChangeFilesAsSpeciallySortedArray( - this.changeNum, this.patchNum); + this.changeNum, this.patchRange); }, _handleKey: function(e) { @@ -164,7 +188,7 @@ if (opt_index != null) { this.selectedIndex = opt_index; } - page.show(this._computeDiffURL(this.changeNum, this.patchNum, + page.show(this._computeDiffURL(this.changeNum, this.patchRange, this._files[this.selectedIndex].__path)); }, @@ -176,8 +200,19 @@ return status || 'M'; }, - _computeDiffURL: function(changeNum, patchNum, path) { - return '/c/' + changeNum + '/' + patchNum + '/' + path; + _computeDiffURL: function(changeNum, patchRange, path) { + return '/c/' + + encodeURIComponent(changeNum) + + '/' + + encodeURIComponent(this._patchRangeStr(patchRange)) + + '/' + + path; + }, + + _patchRangeStr: function(patchRange) { + return patchRange.basePatchNum !== 'PARENT' ? + patchRange.basePatchNum + '..' + patchRange.patchNum : + patchRange.patchNum + ''; }, _computeFileDisplayName: function(path) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html index 64ef91f..f80d211 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -89,7 +89,10 @@ {__path: 'myfile.txt'}, ]; element.changeNum = '42'; - element.patchNum = '2'; + element.patchRange = { + basePatchNum: 'PARENT', + patchNum: '2', + }; element.selectedIndex = 0; flushAsynchronousOperations(); @@ -181,7 +184,10 @@ ]; element._reviewed = ['/COMMIT_MSG', 'myfile.txt']; element.changeNum = '42'; - element.patchNum = '2'; + element.patchRange = { + basePatchNum: 'PARENT', + patchNum: '2', + }; element.selectedIndex = 0; flushAsynchronousOperations(); @@ -205,5 +211,52 @@ saveStub.restore(); }); + + test('patch set from revisions', function() { + var patchNums = element._computePatchSets({ + rev3: {_number: 3}, + rev1: {_number: 1}, + rev4: {_number: 4}, + rev2: {_number: 2}, + }); + assert.deepEqual(patchNums, [1, 2, 3, 4]); + }); + + test('patch range string', function() { + assert.equal( + element._patchRangeStr({basePatchNum: 'PARENT', patchNum: '1'}), + '1'); + assert.equal( + element._patchRangeStr({basePatchNum: '1', patchNum: '3'}), + '1..3'); + }); + + test('diff against dropdown', function(done) { + var showStub = sinon.stub(page, 'show'); + element.changeNum = '42'; + element.patchRange = { + basePatchNum: 'PARENT', + patchNum: '3', + }; + element.revisions = { + rev1: {_number: 1}, + rev2: {_number: 2}, + rev3: {_number: 3}, + }; + flush(function() { + var selectEl = element.$$('select'); + assert.equal(selectEl.value, 'PARENT'); + assert.isTrue(element.$$('option[value="3"]').hasAttribute('disabled')); + selectEl.addEventListener('change', function() { + assert.equal(selectEl.value, '2'); + assert(showStub.lastCall.calledWithExactly('/c/42/2..3'), + 'Should navigate to /c/42/2..3'); + showStub.restore(); + done(); + }); + selectEl.value = '2'; + element.fire('change', {}, {node: selectEl}); + }); + }); }); </script>