Support jumping to next/previous file with comments via shortcut
Adds keyboard shortcuts to the diff view to navigate to the next or
previous file in the change's file list that has comments in the current
patch range.
Feature: Issue 5235
Change-Id: I1ad39089c1ac227e335093f25b72311f7e98b3f7
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 600415a..63e3574 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -80,6 +80,21 @@
},
_isImageDiff: Boolean,
_filesWeblinks: Object,
+
+ /**
+ * Map of paths in the current chnage and patch range that have comments
+ * or drafts or robot comments.
+ */
+ _commentMap: Object,
+
+ /**
+ * Object to contain the path of the next and previous file in the current
+ * change and patch range that has comments.
+ */
+ _commentSkips: {
+ type: Object,
+ computed: '_computeCommentSkips(_commentMap, _fileList, _path)',
+ },
},
behaviors: [
@@ -210,8 +225,13 @@
},
_handleUpKey: function(e) {
- if (this.shouldSuppressKeyboardShortcut(e) ||
- this.modifierPressed(e)) { return; }
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (e.detail.keyboardEvent.shiftKey &&
+ e.detail.keyboardEvent.keyCode === 75) { // 'K'
+ this._moveToPreviousFileWithComment();
+ return;
+ }
+ if (this.modifierPressed(e)) { return; }
e.preventDefault();
this.$.diff.displayLine = true;
@@ -219,14 +239,33 @@
},
_handleDownKey: function(e) {
- if (this.shouldSuppressKeyboardShortcut(e) ||
- this.modifierPressed(e)) { return; }
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (e.detail.keyboardEvent.shiftKey &&
+ e.detail.keyboardEvent.keyCode === 74) { // 'J'
+ this._moveToNextFileWithComment();
+ return;
+ }
+ if (this.modifierPressed(e)) { return; }
e.preventDefault();
this.$.diff.displayLine = true;
this.$.cursor.moveDown();
},
+ _moveToPreviousFileWithComment: function() {
+ if (this._commentSkips && this._commentSkips.previous) {
+ page.show(this._getDiffURL(this._changeNum, this._patchRange,
+ this._commentSkips.previous));
+ }
+ },
+
+ _moveToNextFileWithComment: function() {
+ if (this._commentSkips && this._commentSkips.next) {
+ page.show(this._getDiffURL(this._changeNum, this._patchRange,
+ this._commentSkips.next));
+ }
+ },
+
_handleCKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (this.$.diff.isRangeSelected()) { return; }
@@ -409,6 +448,10 @@
Promise.all(promises)
.then(function() { return this.$.diff.reload(); }.bind(this))
.then(function() { this._loading = false; }.bind(this));
+
+ this._loadCommentMap().then(function(commentMap) {
+ this._commentMap = commentMap;
+ }.bind(this));
},
/**
@@ -596,5 +639,67 @@
url += '/patch?zip&path=' + encodeURIComponent(path);
return url;
},
+
+ /**
+ * Request all comments (and drafts and robot comments) for the current
+ * change and construct the map of file paths that have comments for the
+ * current patch range.
+ * @return {Promise} A promise that yields a comment map object.
+ */
+ _loadCommentMap: function() {
+ function filterByRange(comment) {
+ var patchNum = comment.patch_set + '';
+ return patchNum === this._patchRange.patchNum ||
+ patchNum === this._patchRange.basePatchNum;
+ };
+
+ return Promise.all([
+ this.$.restAPI.getDiffComments(this._changeNum),
+ this._getDiffDrafts(),
+ this.$.restAPI.getDiffRobotComments(this._changeNum),
+ ]).then(function(results) {
+ var commentMap = {};
+ results.forEach(function(response) {
+ for (var path in response) {
+ if (response.hasOwnProperty(path) &&
+ response[path].filter(filterByRange.bind(this)).length) {
+ commentMap[path] = true;
+ }
+ }
+ }.bind(this));
+ return commentMap;
+ }.bind(this));
+ },
+
+ _getDiffDrafts: function() {
+ return this._getLoggedIn().then(function(loggedIn) {
+ if (!loggedIn) { return Promise.resolve({}); }
+ return this.$.restAPI.getDiffDrafts(this._changeNum);
+ }.bind(this));
+ },
+
+ _computeCommentSkips: function(commentMap, fileList, path) {
+ var skips = {previous: null, next: null};
+ if (!fileList.length) { return skips; }
+ var pathIndex = fileList.indexOf(path);
+
+ // Scan backward for the previous file.
+ for (var i = pathIndex - 1; i >= 0; i--) {
+ if (commentMap[fileList[i]]) {
+ skips.previous = fileList[i];
+ break;
+ }
+ }
+
+ // Scan forward for the next file.
+ for (i = pathIndex + 1; i < fileList.length; i++) {
+ if (commentMap[fileList[i]]) {
+ skips.next = fileList[i];
+ break;
+ }
+ }
+
+ return skips;
+ },
});
})();
