Update diff line hash via Gerrit.Nav
Formerly, the gr-diff-view generated and updated the URL hash that
indicates the location of the diff cursor, regardless of the hash
generation scheme implemented in _generateUrl. With this change, the
diff view uses the _generateUrl for the cursor-location-specific URL.
Change-Id: Ia70a298f114c9eae9cdc5b0a8f73d7ecab55896e
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index b14ea7f..014b590 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -226,13 +226,14 @@
},
/**
- * Get a short address for the location of the cursor. Such as '123' for
- * line 123 of the revision, or 'b321' for line 321 of the base patch.
- * Returns an empty string if an address is not available.
- * @return {string}
+ * Get an object describing the location of the cursor. Such as
+ * {leftSide: false, number: 123} for line 123 of the revision, or
+ * {leftSide: true, number: 321} for line 321 of the base patch.
+ * Returns null if an address is not available.
+ * @return {?Object}
*/
getAddress() {
- if (!this.diffRow) { return ''; }
+ if (!this.diffRow) { return null; }
// Get the line-number cell targeted by the cursor. If the mode is unified
// then prefer the revision cell if available.
@@ -245,12 +246,15 @@
} else {
cell = this.diffRow.querySelector('.lineNum.' + this.side);
}
- if (!cell) { return ''; }
+ if (!cell) { return null; }
const number = cell.getAttribute('data-value');
- if (!number || number === 'FILE') { return ''; }
+ if (!number || number === 'FILE') { return null; }
- return (cell.matches('.left') ? 'b' : '') + number;
+ return {
+ leftSide: cell.matches('.left'),
+ number: parseInt(number, 10),
+ };
},
_getViewMode() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index 15a405a..54f7d6f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -240,27 +240,31 @@
test('getAddress', () => {
// It should initialize to the first chunk: line 5 of the revision.
- assert.equal(cursorElement.getAddress(), '5');
+ assert.deepEqual(cursorElement.getAddress(),
+ {leftSide: false, number: 5});
// Revision line 4 is up.
cursorElement.moveUp();
- assert.equal(cursorElement.getAddress(), '4');
+ assert.deepEqual(cursorElement.getAddress(),
+ {leftSide: false, number: 4});
// Base line 4 is left.
cursorElement.moveLeft();
- assert.equal(cursorElement.getAddress(), 'b4');
+ assert.deepEqual(cursorElement.getAddress(), {leftSide: true, number: 4});
// Moving to the next chunk takes it back to the start.
cursorElement.moveToNextChunk();
- assert.equal(cursorElement.getAddress(), '5');
+ assert.deepEqual(cursorElement.getAddress(),
+ {leftSide: false, number: 5});
// The following chunk is a removal starting on line 10 of the base.
cursorElement.moveToNextChunk();
- assert.equal(cursorElement.getAddress(), 'b10');
+ assert.deepEqual(cursorElement.getAddress(),
+ {leftSide: true, number: 10});
- // Should be an empty string if there is no selection.
+ // Should be null if there is no selection.
cursorElement.$.cursorManager.unsetCursor();
- assert.equal(cursorElement.getAddress(), '');
+ assert.isNotOk(cursorElement.getAddress());
});
test('_findRowByNumberAndFile', () => {
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 fbd5b1b..409fd09 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
@@ -727,7 +727,13 @@
_onLineSelected(e, detail) {
this.$.cursor.moveToLineNumber(detail.number, detail.side);
- history.replaceState(null, '', '#' + this.$.cursor.getAddress());
+ if (!this._change) { return; }
+ const cursorAddress = this.$.cursor.getAddress();
+ const url = Gerrit.Nav.getUrlForDiffById(this._changeNum,
+ this._change.project, this._path, this._patchRange.patchNum,
+ this._patchRange.basePatchNum, cursorAddress.number,
+ cursorAddress.leftSide);
+ history.replaceState(null, '', url);
},
_computeDownloadLink(changeNum, patchRange, path) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index e9c5de8..250e0f5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -620,9 +620,18 @@
});
test('_onLineSelected', () => {
+ const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiffById');
const replaceStateStub = sandbox.stub(history, 'replaceState');
const moveStub = sandbox.stub(element.$.cursor, 'moveToLineNumber');
+ sandbox.stub(element.$.cursor, 'getAddress')
+ .returns({number: 123, isLeftSide: false});
+ element._changeNum = 321;
+ element._change = {_number: 321, project: 'foo/bar'};
+ element._patchRange = {
+ basePatchNum: '3',
+ patchNum: '5',
+ };
const e = {};
const detail = {number: 123, side: 'right'};
@@ -633,6 +642,7 @@
assert.equal(moveStub.lastCall.args[1], detail.side);
assert.isTrue(replaceStateStub.called);
+ assert.isTrue(getUrlStub.called);
});
test('_getDiffViewMode', () => {
