Add next/prev links to the diff view
Bug: Issue 4514
Change-Id: I39927b397fda8d6bb80b7e3a9144e5cfe620fb5a
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 38a6746..76f7aec 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -33,9 +33,18 @@
background-color: var(--view-background-color);
display: block;
}
- h3 {
+ header,
+ .subHeader {
+ align-items: center;
+ display: flex;
+ justify-content: space-between;
+ }
+ header {
padding: .75em var(--default-horizontal-margin);
}
+ .navLink:not([href]) {
+ color: #999;
+ }
.reviewed {
display: inline-block;
margin: 0 .25em;
@@ -97,10 +106,7 @@
padding: 0 var(--default-horizontal-margin) 1em;
color: #666;
}
- .header {
- align-items: center;
- display: flex;
- justify-content: space-between;
+ .subHeader {
margin: 0 var(--default-horizontal-margin) .75em;
}
.prefsButton {
@@ -125,49 +131,56 @@
}
}
</style>
- <h3>
- <a href$="[[_computeChangePath(_changeNum, _patchRange.*, _change.revisions)]]">
- [[_changeNum]]</a><span>:</span>
- <span>[[_change.subject]]</span>
- <span class="dash">—</span>
- <input id="reviewed"
- class="reviewed"
- type="checkbox"
- on-change="_handleReviewedChange"
- hidden$="[[!_loggedIn]]" hidden>
- <div class="jumpToFileContainer">
- <gr-button link class="dropdown-trigger" id="trigger" on-tap="_showDropdownTapHandler">
- <span>[[_computeFileDisplayName(_path)]]</span>
- <span class="downArrow">▼</span>
- </gr-button>
- <iron-dropdown id="dropdown" vertical-align="top" vertical-offset="25">
- <div class="dropdown-content">
+ <header>
+ <h3>
+ <a href$="[[_computeChangePath(_changeNum, _patchRange.*, _change.revisions)]]">
+ [[_changeNum]]</a><span>:</span>
+ <span>[[_change.subject]]</span>
+ <span class="dash">—</span>
+ <input id="reviewed"
+ class="reviewed"
+ type="checkbox"
+ on-change="_handleReviewedChange"
+ hidden$="[[!_loggedIn]]" hidden>
+ <div class="jumpToFileContainer">
+ <gr-button link class="dropdown-trigger" id="trigger" on-tap="_showDropdownTapHandler">
+ <span>[[_computeFileDisplayName(_path)]]</span>
+ <span class="downArrow">▼</span>
+ </gr-button>
+ <iron-dropdown id="dropdown" vertical-align="top" vertical-offset="25">
+ <div class="dropdown-content">
+ <template is="dom-repeat" items="[[_fileList]]" as="path">
+ <a href$="[[_computeDiffURL(_changeNum, _patchRange.*, path)]]"
+ selected$="[[_computeFileSelected(path, _path)]]"
+ data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]"
+ on-tap="_handleFileTap">[[_computeFileDisplayName(path)]]</a>
+ </template>
+ </div>
+ </iron-dropdown>
+ </div>
+ <div class="mobileJumpToFileContainer">
+ <select on-change="_handleMobileSelectChange">
<template is="dom-repeat" items="[[_fileList]]" as="path">
- <a href$="[[_computeDiffURL(_changeNum, _patchRange.*, path)]]"
- selected$="[[_computeFileSelected(path, _path)]]"
- data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]"
- on-tap="_handleFileTap">
- [[_computeFileDisplayName(path)]]
- </a>
+ <option
+ value$="[[path]]"
+ selected$="[[_computeFileSelected(path, _path)]]">
+ [[_computeFileDisplayName(path)]]
+ </option>
</template>
- </div>
- </iron-dropdown>
+ </select>
+ </div>
+ </h3>
+ <div>
+ <a class="navLink"
+ href$="[[_computeNavLinkURL(_path, _fileList, -1, 1)]]">Prev</a>
+ /
+ <a class="navLink"
+ href$="[[_computeNavLinkURL(_path, _fileList, 1, 1)]]">Next</a>
</div>
- <div class="mobileJumpToFileContainer">
- <select on-change="_handleMobileSelectChange">
- <template is="dom-repeat" items="[[_fileList]]" as="path">
- <option
- value$="[[path]]"
- selected$="[[_computeFileSelected(path, _path)]]">
- [[_computeFileDisplayName(path)]]
- </option>
- </template>
- </select>
- </div>
- </h3>
+ </header>
<div class="loading" hidden$="[[!_loading]]">Loading...</div>
<div hidden$="[[_loading]]" hidden>
- <div class="header">
+ <div class="subHeader">
<gr-patch-range-select
path="[[_path]]"
change-num="[[_changeNum]]"
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 7d9f9d8..40b0ee1 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
@@ -211,11 +211,11 @@
break;
case 219: // '['
e.preventDefault();
- this._navToFile(this._fileList, -1);
+ this._navToFile(this._path, this._fileList, -1);
break;
case 221: // ']'
e.preventDefault();
- this._navToFile(this._fileList, 1);
+ this._navToFile(this._path, this._fileList, 1);
break;
case 78: // 'n'
e.preventDefault();
@@ -260,20 +260,41 @@
}
},
- _navToFile: function(fileList, direction) {
- if (fileList.length == 0) { return; }
+ _navToFile: function(path, fileList, direction) {
+ var url = this._computeNavLinkURL(path, fileList, direction);
+ if (!url) { return; }
- var idx = fileList.indexOf(this._path) + direction;
+ page.show(this._computeNavLinkURL(path, fileList, direction));
+ },
+
+ /**
+ * @param {?string} path The path of the current file being shown.
+ * @param {Array.<string>} fileList The list of files in this change and
+ * patch range.
+ * @param {number} direction Either 1 (next file) or -1 (prev file).
+ * @param {(number|boolean)} opt_noUp Whether to return to the change view
+ * when advancing the file goes outside the bounds of fileList.
+ *
+ * @return {?string} The next URL when proceeding in the specified
+ * direction.
+ */
+ _computeNavLinkURL: function(path, fileList, direction, opt_noUp) {
+ if (!path || fileList.length === 0) { return null; }
+
+ var idx = fileList.indexOf(path);
+ if (idx === -1) { return null; }
+
+ idx += direction;
+ // Redirect to the change view if opt_noUp isn’t truthy and idx falls
+ // outside the bounds of [0, fileList.length).
if (idx < 0 || idx > fileList.length - 1) {
- page.show(this._getChangePath(
+ if (opt_noUp) { return null; }
+ return this._getChangePath(
this._changeNum,
this._patchRange,
- this._change && this._change.revisions));
- return;
+ this._change && this._change.revisions);
}
- page.show(this._getDiffURL(this._changeNum,
- this._patchRange,
- fileList[idx]));
+ return this._getDiffURL(this._changeNum, this._patchRange, fileList[idx]);
},
_paramsChanged: function(value) {
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 c9c27c5..979ed55 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
@@ -344,6 +344,52 @@
assert.equal(linkEls[2].getAttribute('href'), '/c/42/5..10/wheatley.md');
});
+ test('prev/next links', function() {
+ element._changeNum = '42';
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '10',
+ };
+ element._fileList = ['chell.go', 'glados.txt', 'wheatley.md'];
+ element._path = 'glados.txt';
+ flushAsynchronousOperations();
+ var linkEls = Polymer.dom(element.root).querySelectorAll('.navLink');
+ assert.equal(linkEls.length, 2);
+ assert.equal(linkEls[0].getAttribute('href'), '/c/42/10/chell.go');
+ assert.equal(linkEls[1].getAttribute('href'), '/c/42/10/wheatley.md');
+ element._path = 'wheatley.md';
+ flushAsynchronousOperations();
+ assert.equal(linkEls[0].getAttribute('href'), '/c/42/10/glados.txt');
+ assert.isFalse(linkEls[1].hasAttribute('href'));
+ element._path = 'chell.go';
+ flushAsynchronousOperations();
+ assert.isFalse(linkEls[0].hasAttribute('href'));
+ assert.equal(linkEls[1].getAttribute('href'), '/c/42/10/glados.txt');
+ });
+
+ test('prev/next links with patch range', function() {
+ element._changeNum = '42';
+ element._patchRange = {
+ basePatchNum: '5',
+ patchNum: '10',
+ };
+ element._fileList = ['chell.go', 'glados.txt', 'wheatley.md'];
+ element._path = 'glados.txt';
+ flushAsynchronousOperations();
+ var linkEls = Polymer.dom(element.root).querySelectorAll('.navLink');
+ assert.equal(linkEls.length, 2);
+ assert.equal(linkEls[0].getAttribute('href'), '/c/42/5..10/chell.go');
+ assert.equal(linkEls[1].getAttribute('href'), '/c/42/5..10/wheatley.md');
+ element._path = 'wheatley.md';
+ flushAsynchronousOperations();
+ assert.equal(linkEls[0].getAttribute('href'), '/c/42/5..10/glados.txt');
+ assert.isFalse(linkEls[1].hasAttribute('href'));
+ element._path = 'chell.go';
+ flushAsynchronousOperations();
+ assert.isFalse(linkEls[0].hasAttribute('href'));
+ assert.equal(linkEls[1].getAttribute('href'), '/c/42/5..10/glados.txt');
+ });
+
test('file review status', function(done) {
element._loggedIn = true;
element._changeNum = '42';
