Scroll to comment in diff when file is unchanged
When clicking on a comment link if file is unchanged, we navigate to
Base vs X. Scroll to the line of the comment in the diff view.
Change-Id: Iee0db16e88d4d23b6fdc3dc1bf733045a3a56cea
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index b45782f..5016c40 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -758,10 +758,11 @@
change: ChangeInfo,
filePath: string,
patchNum?: PatchSetNum,
- basePatchNum?: PatchSetNum
+ basePatchNum?: PatchSetNum,
+ lineNum?: number
) {
this._navigate(
- this.getUrlForDiff(change, filePath, patchNum, basePatchNum)
+ this.getUrlForDiff(change, filePath, patchNum, basePatchNum, lineNum)
);
},
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 ab706e9..90acad3 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
@@ -231,6 +231,8 @@
type: Object,
value: () => new Set(),
},
+ // line number on the diff which should be scrolled to upon loading
+ _focusLineNum: Number,
};
}
@@ -744,14 +746,12 @@
.then(files => files.has(path));
}
- _initLineOfInterestAndCursor(lineNum, leftSide) {
+ _initLineOfInterestAndCursor(leftSide) {
this.$.diffHost.lineOfInterest =
this._getLineOfInterest({
- lineNum,
leftSide,
});
this._initCursor({
- lineNum,
leftSide,
});
}
@@ -818,7 +818,7 @@
}
_initPatchRange() {
- let lineNum; let leftSide;
+ let leftSide;
if (this.params.commentId) {
const comment = this._changeComments.findCommentById(
this.params.commentId);
@@ -849,7 +849,7 @@
// comment.patch_set vs latest
leftSide = true;
}
- lineNum = comment.line;
+ this._focusLineNum = comment.line;
} else {
if (this.params.path) {
this._path = this.params.path;
@@ -861,11 +861,11 @@
};
}
if (this.params.lineNum) {
- lineNum = this.params.lineNum;
+ this._focusLineNum = this.params.lineNum;
leftSide = this.params.leftSide;
}
}
- this._initLineOfInterestAndCursor(lineNum, leftSide);
+ this._initLineOfInterestAndCursor(leftSide);
this._commentMap = this._getPaths(this._patchRange);
this._commentsForDiff = this._getCommentsForPath(this._path,
@@ -889,6 +889,7 @@
this._patchRange = undefined;
this._commitRange = undefined;
this._changeComments = undefined;
+ this._focusLineNum = undefined;
if (value.changeNum && value.project) {
this.$.restAPI.setInProjectLookup(value.changeNum, value.project);
@@ -956,7 +957,8 @@
composed: true, bubbles: true,
}));
GerritNav.navigateToDiff(
- this._change, this._path, this._patchRange.basePatchNum);
+ this._change, this._path, this._patchRange.basePatchNum,
+ 'PARENT', this._focusLineNum);
return;
}
if (value.commentLink) {
@@ -1010,20 +1012,20 @@
* If the params specify a diff address then configure the diff cursor.
*/
_initCursor(params) {
- if (params.lineNum === undefined) { return; }
+ if (this._focusLineNum === undefined) { return; }
if (params.leftSide) {
this.$.cursor.side = DiffSides.LEFT;
} else {
this.$.cursor.side = DiffSides.RIGHT;
}
- this.$.cursor.initialLineNumber = params.lineNum;
+ this.$.cursor.initialLineNumber = this._focusLineNum;
}
_getLineOfInterest(params) {
// If there is a line number specified, pass it along to the diff so that
// it will not get collapsed.
- if (!params.lineNum) { return null; }
- return {number: params.lineNum, leftSide: params.leftSide};
+ if (!this._focusLineNum) { return null; }
+ return {number: this._focusLineNum, leftSide: params.leftSide};
}
_pathChanged(path) {
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 e4dd52e..8b5a972 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
@@ -169,7 +169,8 @@
element._change = generateChange({revisionsCount: 11});
return element._paramsChanged.returnValues[0].then(() => {
assert.isTrue(initLineOfInterestAndCursorStub.
- calledWithExactly(10, true));
+ calledWithExactly(true));
+ assert.equal(element._focusLineNum, 10);
assert.equal(element._patchRange.patchNum, 11);
assert.equal(element._patchRange.basePatchNum, 2);
});
@@ -226,7 +227,7 @@
element._change = generateChange({revisionsCount: 11});
return element._paramsChanged.returnValues[0].then(() => {
assert.isTrue(diffNavStub.lastCall.calledWithExactly(
- element._change, '/COMMIT_MSG', 2));
+ element._change, '/COMMIT_MSG', 2, 'PARENT', 10));
});
});
@@ -1218,17 +1219,21 @@
assert.isNotOk(element.$.cursor.initialLineNumber);
// Revision hash: specifies lineNum but not side.
- element._initCursor({lineNum: 234});
+
+ element._focusLineNum = 234;
+ element._initCursor({});
assert.equal(element.$.cursor.initialLineNumber, 234);
assert.equal(element.$.cursor.side, 'right');
// Base hash: specifies lineNum and side.
- element._initCursor({leftSide: true, lineNum: 345});
+ element._focusLineNum = 345;
+ element._initCursor({leftSide: true});
assert.equal(element.$.cursor.initialLineNumber, 345);
assert.equal(element.$.cursor.side, 'left');
// Specifies right side:
- element._initCursor({leftSide: false, lineNum: 123});
+ element._focusLineNum = 123;
+ element._initCursor({leftSide: false});
assert.equal(element.$.cursor.initialLineNumber, 123);
assert.equal(element.$.cursor.side, 'right');
});
@@ -1236,11 +1241,12 @@
test('_getLineOfInterest', () => {
assert.isNull(element._getLineOfInterest({}));
- let result = element._getLineOfInterest({lineNum: 12});
+ element._focusLineNum = 12;
+ let result = element._getLineOfInterest({});
assert.equal(result.number, 12);
assert.isNotOk(result.leftSide);
- result = element._getLineOfInterest({lineNum: 12, leftSide: true});
+ result = element._getLineOfInterest({leftSide: true});
assert.equal(result.number, 12);
assert.isOk(result.leftSide);
});
