Merge "Navigate to next file with comment thread"
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 468c8ca..19f43c2 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -594,6 +594,8 @@
export const _testOnly_findCommentById =
ChangeComments.prototype.findCommentById;
+export const _testOnly_getCommentsForPath =
+ ChangeComments.prototype.getCommentsForPath;
@customElement('gr-comment-api')
export class GrCommentApi extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 035c1a3..5ac37dc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -235,7 +235,11 @@
return result;
}
- moveToNextCommentThread(): CursorMoveResult {
+ moveToNextCommentThread(): CursorMoveResult | undefined {
+ if (this.isAtEnd()) {
+ fireEvent(this, 'navigate-to-next-file-with-comments');
+ return;
+ }
const result = this.$.cursorManager.next({
filter: (row: HTMLElement) => this._rowHasThread(row),
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 62b34e9..1550108 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -711,7 +711,12 @@
);
}
- _navToFile(path: string, fileList: string[], direction: -1 | 1) {
+ _navToFile(
+ path: string,
+ fileList: string[],
+ direction: -1 | 1,
+ navigateToFirstComment?: boolean
+ ) {
const newPath = this._getNavLinkPath(path, fileList, direction);
if (!newPath) return;
if (!this._change) return;
@@ -727,11 +732,18 @@
}
if (!newPath.path) return;
+ let lineNum;
+ if (navigateToFirstComment)
+ lineNum = this._changeComments?.getCommentsForPath(
+ newPath.path,
+ this._patchRange
+ )?.[0].line;
GerritNav.navigateToDiff(
this._change,
newPath.path,
this._patchRange.patchNum,
- this._patchRange.basePatchNum
+ this._patchRange.basePatchNum,
+ lineNum
);
}
@@ -1757,6 +1769,23 @@
this._navToFile(this._path, unreviewedFiles, 1);
}
+ _navigateToNextFileWithCommentThread() {
+ if (!this._path) return;
+ if (!this._fileList) return;
+ if (!this._patchRange) return;
+ if (!this._change) return;
+ const hasComment = (path: string) => {
+ return (
+ this._changeComments?.getCommentsForPath(path, this._patchRange!)
+ ?.length ?? 0 > 0
+ );
+ };
+ const filesWithComments = this._fileList.filter(
+ file => file === this._path || hasComment(file)
+ );
+ this._navToFile(this._path, filesWithComments, 1, true);
+ }
+
_handleReloadingDiffPreference() {
this._getDiffPreferences();
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index a00927c..bfb0cd8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -428,6 +428,7 @@
<gr-diff-cursor
id="cursor"
on-navigate-to-next-unreviewed-file="_handleNextUnreviewedFile"
+ on-navigate-to-next-file-with-comments="_navigateToNextFileWithCommentThread"
></gr-diff-cursor>
<gr-comment-api id="commentAPI"></gr-comment-api>
`;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 34e74b6..46a670d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -21,7 +21,7 @@
import {ChangeStatus} from '../../../constants/constants.js';
import {TestKeyboardShortcutBinder} from '../../../test/test-utils.js';
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
-import {_testOnly_findCommentById} from '../gr-comment-api/gr-comment-api.js';
+import {ChangeComments, _testOnly_findCommentById, _testOnly_getCommentsForPath} from '../gr-comment-api/gr-comment-api.js';
import {GerritView} from '../../../services/router/router-model.js';
import {
createChange,
@@ -137,6 +137,7 @@
computeUnresolvedNum: () => {},
getPaths: () => {},
getThreadsBySideForFile: () => [],
+ getCommentsForPath: _testOnly_getCommentsForPath,
findCommentById: _testOnly_findCommentById,
}));
@@ -465,6 +466,48 @@
assert.equal(element._setReviewed.lastCall.args[0], true);
});
+ test('moveToNextCommentThread navigates to next file', () => {
+ const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
+ const diffChangeStub = sinon.stub(element, '_navigateToChange');
+ sinon.stub(element.$.cursor, 'isAtEnd').returns(true);
+ element._changeNum = '42';
+ const comment = {
+ 'wheatley.md': [{
+ ...createComment(),
+ patch_set: 10,
+ line: 21,
+ }],
+ };
+ element._changeComments = new ChangeComments(comment);
+ element._patchRange = {
+ basePatchNum: PARENT,
+ patchNum: 10,
+ };
+ element._change = {
+ _number: 42,
+ revisions: {
+ a: {_number: 10, commit: {parents: []}},
+ },
+ };
+ element._files = getFilesFromFileList(
+ ['chell.go', 'glados.txt', 'wheatley.md']);
+ element._path = 'glados.txt';
+ element.changeViewState.selectedFileIndex = 1;
+ element._loggedIn = true;
+
+ MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
+ flush();
+ assert.isTrue(diffNavStub.calledWithExactly(
+ element._change, 'wheatley.md', 10, PARENT, 21));
+
+ element._path = 'wheatley.md'; // navigated to next file
+
+ MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
+ flush();
+
+ assert.isTrue(diffChangeStub.called);
+ });
+
test('shift+x shortcut expands all diff context', () => {
const expandStub = sinon.stub(element.$.diffHost, 'expandAllContext');
MockInteractions.pressAndReleaseKeyOn(element, 88, 'shift', 'x');
@@ -621,14 +664,14 @@
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'wheatley.md', 10, 5),
+ 'wheatley.md', 10, 5, undefined),
'Should navigate to /c/42/5..10/wheatley.md');
element._path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert.isTrue(element._loading);
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'glados.txt', 10, 5),
+ 'glados.txt', 10, 5, undefined),
'Should navigate to /c/42/5..10/glados.txt');
element._path = 'glados.txt';
@@ -638,7 +681,8 @@
element._change,
'chell.go',
10,
- 5),
+ 5,
+ undefined),
'Should navigate to /c/42/5..10/chell.go');
element._path = 'chell.go';
@@ -692,13 +736,13 @@
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'wheatley.md', 1, PARENT),
+ 'wheatley.md', 1, PARENT, undefined),
'Should navigate to /c/42/1/wheatley.md');
element._path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(diffNavStub.lastCall.calledWithExactly(element._change,
- 'glados.txt', 1, PARENT),
+ 'glados.txt', 1, PARENT, undefined),
'Should navigate to /c/42/1/glados.txt');
element._path = 'glados.txt';
@@ -707,7 +751,8 @@
element._change,
'chell.go',
1,
- PARENT), 'Should navigate to /c/42/1/chell.go');
+ PARENT,
+ undefined), 'Should navigate to /c/42/1/chell.go');
element._path = 'chell.go';
changeNavStub.reset();
