opendev/gerrit
Code Issues Proposed changes

Update URL generation in gr-change-list-item and gr-change-list

Browse Source 
The topic in the branch column of the change list is also given a
separate link.

Bug: Issue 6446
Change-Id: I67f2411900ace29483c71910bb469275ad56b8b4
This commit is contained in:
Wyatt Allen
2017-08-15 15:03:51 -07:00
parent 833e15263e
commit db6afdc3e8
7 changed files with 44 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
View File

@@ -19,6 +19,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../styles/gr-change-list-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-change-star/gr-change-star.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
@@ -72,7 +73,8 @@ limitations under the License.
}
a {
color: var(--default-text-color);
display: block;
cursor: pointer;
display: inline-block;
text-decoration: none;
}
a:hover {
@@ -141,8 +143,11 @@ limitations under the License.
<td class="cell branch"
hidden$="[[isColumnHidden('Branch', visibleChangeTableColumns)]]">
<a href$="[[_computeProjectBranchURL(change)]]">
[[_computeBranchText(change)]]
[[change.branch]]
</a>
<template is="dom-if" if="[[change.topic]]">
(<a href$="[[_computeTopicURL(change)]]">[[change.topic]]</a>)
</template>
</td>
<td class="cell updated"
hidden$="[[isColumnHidden('Updated', visibleChangeTableColumns)]]">

25
polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
View File

@@ -27,7 +27,7 @@
change: Object,
changeURL: {
type: String,
computed: '_computeChangeURL(change._number)',
computed: '_computeChangeURL(change)',
},
showStar: {
type: Boolean,
@@ -43,9 +43,8 @@
Gerrit.URLEncodingBehavior,
],
_computeChangeURL(changeNum) {
if (!changeNum) { return ''; }
return this.getBaseUrl() + '/c/' + changeNum + '/';
_computeChangeURL(change) {
return Gerrit.Nav.getUrlForChange(change);
},
_computeLabelTitle(change, labelName) {
@@ -105,24 +104,16 @@
},
_computeProjectURL(project) {
return this.getBaseUrl() + '/q/status:open+project:' +
this.encodeURL(project, false);
return Gerrit.Nav.getUrlForProject(project, true);
},
_computeProjectBranchURL(change) {
// @see Issue 4255, Issue 6195.
let output = this._computeProjectURL(change.project);
output += '+branch:' + this.encodeURL(change.branch, false);
if (change.topic) {
output += '+topic:' + this.encodeURL(change.topic, false);
}
return output;
return Gerrit.Nav.getUrlForBranch(change.branch, change.project);
},
_computeBranchText(change) {
let output = change.branch;
if (change.topic) { output += ` (${change.topic})`; }
return output;
_computeTopicURL(change) {
if (!change.topic) { return ''; }
return Gerrit.Nav.getUrlForTopic(change.topic);
},
});
})();

17
polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
View File

@@ -112,23 +112,6 @@ limitations under the License.
{labels: {Verified: {approved: true}}}, 'Verified'), '✓');
assert.equal(element._computeLabelValue(
{labels: {Verified: {rejected: true}}}, 'Verified'), '✕');
assert.equal(element._computeProjectURL('combustible/stuff'),
'/q/status:open+project:combustible%252Fstuff');
const change = {project: 'combustible-stuff', branch: 'le/mons'};
assert.equal(element._computeProjectBranchURL(change),
'/q/status:open+project:combustible-stuff+branch:le%252Fmons');
change.topic = 'test/test';
assert.equal(element._computeProjectBranchURL(change),
'/q/status:open+project:combustible-stuff+branch:le%252Fmons' +
'+topic:test%252Ftest');
element.change = {_number: 42};
assert.equal(element.changeURL, '/c/42/');
element.change = {_number: 43};
assert.equal(element.changeURL, '/c/43/');
});
test('no hidden columns', () => {

1
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
View File

@@ -21,6 +21,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../styles/gr-change-list-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-change-list-item/gr-change-list-item.html">
<link rel="import" href="../../../styles/shared-styles.html">

8
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
View File

@@ -238,7 +238,7 @@
this.modifierPressed(e)) { return; }
e.preventDefault();
page.show(this._changeURLForIndex(this.selectedIndex));
Gerrit.Nav.navigateToChange(this._changeForIndex(this.selectedIndex));
},
_handleNKey(e) {
@@ -285,12 +285,12 @@
this.$.restAPI.saveChangeStarred(change._number, newVal);
},
_changeURLForIndex(index) {
_changeForIndex(index) {
const changeEls = this._getListItems();
if (index < changeEls.length && changeEls[index]) {
return changeEls[index].changeURL;
return changeEls[index].change;
}
return '';
return null;
},
_getListItems() {

25
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
View File

@@ -42,11 +42,15 @@ limitations under the License.
<script>
suite('gr-change-list basic tests', () => {
let element;
let sandbox;
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
});
teardown(() => { sandbox.restore(); });
function stubRestAPI(preferences) {
const loggedInPromise = Promise.resolve(preferences !== null);
const preferencesPromise = Promise.resolve(preferences);
@@ -149,16 +153,16 @@ limitations under the License.
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
const showStub = sinon.stub(page, 'show');
const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
assert.equal(element.selectedIndex, 2);
MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
assert(showStub.lastCall.calledWithExactly('/c/2/'),
assert.deepEqual(navStub.lastCall.args[0], {_number: 2},
'Should navigate to /c/2/');
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
assert(showStub.lastCall.calledWithExactly('/c/1/'),
assert.deepEqual(navStub.lastCall.args[0], {_number: 1},
'Should navigate to /c/1/');
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
@@ -166,7 +170,6 @@ limitations under the License.
MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
assert.equal(element.selectedIndex, 0);
showStub.restore();
done();
});
});
@@ -367,11 +370,15 @@ limitations under the License.
suite('gr-change-list sections', () => {
let element;
let sandbox;
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
});
teardown(() => { sandbox.restore(); });
test('keyboard shortcuts', () => {
element.selectedIndex = 0;
element.sections = [
@@ -411,16 +418,17 @@ limitations under the License.
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
const showStub = sinon.stub(page, 'show');
const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
assert.equal(element.selectedIndex, 2);
MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
assert(showStub.lastCall.calledWithExactly('/c/2/'),
assert.deepEqual(navStub.lastCall.args[0], {_number: 2},
'Should navigate to /c/2/');
MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
assert(showStub.lastCall.calledWithExactly('/c/1/'),
assert.deepEqual(navStub.lastCall.args[0], {_number: 1},
'Should navigate to /c/1/');
MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
@@ -428,9 +436,8 @@ limitations under the License.
MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
assert.equal(element.selectedIndex, 4);
MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
assert(showStub.lastCall.calledWithExactly('/c/4/'),
assert.deepEqual(navStub.lastCall.args[0], {_number: 4},
'Should navigate to /c/4/');
showStub.restore();
});
test('assigned attribute set in each item', () => {

13
polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
View File

@@ -123,28 +123,31 @@ limitations under the License.
},
/**
* @param {string} project The name of the project.
* @param {!string} project The name of the project.
* @param {boolean=} opt_openOnly When true, only search open changes in
* the project.
* @return {string}
*/
getUrlForProject(project) {
getUrlForProject(project, opt_openOnly) {
return this._getUrlFor({
view: Gerrit.Nav.View.SEARCH,
project,
statuses: opt_openOnly ? ['open'] : [],
});
},
/**
* @param {string} branch The name of the branch.
* @param {string} project The name of the project.
* @param {string} status The status to search.
* @param {string=} opt_status The status to search.
* @return {string}
*/
getUrlForBranch(branch, project, status) {
getUrlForBranch(branch, project, opt_status) {
return this._getUrlFor({
view: Gerrit.Nav.View.SEARCH,
branch,
project,
statuses: [status],
statuses: opt_status ? [opt_status] : undefined,
});
},