opendev/gerrit
Code Issues Proposed changes

Implement gr-dropdown-list in patch range select

Browse Source 
This also moves the change reloading logic back to the change view,
where it gets updated patch ranges via a two-way data binding.

Change-Id: Ib09ad1a176ba96bac77a513d344226df029aef7b
This commit is contained in:
Becky Siegel
2017-09-26 09:59:33 -07:00
parent ce67e2a921
commit fd7c71f1db
14 changed files with 401 additions and 257 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
View File

@@ -438,7 +438,8 @@ limitations under the License.
shown-file-count="[[_shownFileCount]]"
diff-prefs="[[_diffPrefs]]"
diff-view-mode="{{viewState.diffMode}}"
patch-range="{{_patchRange}}"
patch-num="{{_patchNum}}"
base-patch-num="{{_basePatchNum}}"
revisions="[[_sortedRevisions]]"
on-open-diff-prefs="_handleOpenDiffPrefs"
on-open-download-dialog="_handleOpenDownloadDialog"

22
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@@ -137,6 +137,16 @@
_patchRange: {
type: Object,
},
// These are kept as separate properties from the patchRange so that the
// observer can be aware of the previous value. In order to view sub
// property changes for _patchRange, a complex observer must be used, and
// that only displays the new value.
//
// If a previous value did not exist, the change is not reloaded with the
// new patches. This is just the initial setting from the change view vs.
// an update coming from the two way data binding.
_patchNum: String,
_basePatchNum: String,
_relatedChangesLoading: {
type: Boolean,
value: true,
@@ -211,6 +221,7 @@
'_labelsChanged(_change.labels.*)',
'_paramsAndChangeChanged(params, _change)',
'_updateSortedRevisions(_change.revisions.*)',
'_patchRangeChanged(_patchRange.*)',
],
keyBindings: {
@@ -311,6 +322,15 @@
window.location.reload();
},
/**
* Called when the patch range changes. does not detect sub property
* updates.
*/
_patchRangeChanged() {
this._basePatchNum = this._patchRange.basePatchNum;
this._patchNum = this._patchRange.patchNum;
},
_handleCommitMessageCancel(e) {
this._editingCommitMessage = false;
},
@@ -501,8 +521,8 @@
if (this._initialLoadComplete && patchChanged) {
if (patchRange.patchNum == null) {
patchRange.patchNum = this.computeLatestPatchNum(this._allPatchSets);
this._patchRange = patchRange;
}
this._patchRange = patchRange;
this._reloadPatchNumDependentResources().then(() => {
this.$.jsAPI.handleEvent(this.$.jsAPI.EventType.SHOW_CHANGE, {
change: this._change,

21
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
View File

@@ -778,6 +778,26 @@ limitations under the License.
assert.isNull(element._getUrlParameter('test'));
});
test('navigateToChange called when range select changes', () => {
element._change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
rev2: {_number: 2},
rev1: {_number: 1},
rev13: {_number: 13},
rev3: {_number: 3},
},
status: 'NEW',
labels: {},
};
element._basePatchNum = 1;
element._patchNum = 2;
element._patchNum = 3;
assert.equal(navigateToChangeStub.callCount, 1);
assert.isTrue(navigateToChangeStub.lastCall
.calledWithExactly(element._change, 3, 1));
});
test('revert dialog opened with revert param', done => {
sandbox.stub(element.$.restAPI, 'getLoggedIn', () => {
return Promise.resolve(true);
@@ -794,6 +814,7 @@ limitations under the License.
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
rev1: {_number: 1},
rev2: {_number: 2},
},
current_revision: 'rev1',
status: element.ChangeStatus.MERGED,

12
polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
View File

@@ -102,20 +102,26 @@ limitations under the License.
.editLoaded .showOnEdit {
display: initial;
}
.label {
font-family: var(--font-family-bold);
margin-right: 1em;
}
@media screen and (max-width: 50em) {
.desktop {
display: none;
}
}
</style>
<div class$="patchInfo-header [[_computeEditLoadedClass(editLoaded)]] [[_computePatchInfoClass(patchRange.patchNum, allPatchSets)]]">
<div class$="patchInfo-header [[_computeEditLoadedClass(editLoaded)]] [[_computePatchInfoClass(patchNum, allPatchSets)]]">
<div class="patchInfo-header-wrapper">
<div>
<span class="label">Files</span>
<gr-patch-range-select
id="rangeSelect"
comments="[[comments]]"
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
patch-num="{{patchNum}}"
base-patch-num="{{basePatchNum}}"
available-patches="[[allPatchSets]]"
revisions="[[change.revisions]]"
on-patch-range-change="_handlePatchChange">
@@ -141,7 +147,7 @@ limitations under the License.
id="descriptionLabel"
class="descriptionLabel"
label-text="Add patchset description"
value="[[_computePatchSetDescription(change, patchRange.patchNum)]]"
value="[[_computePatchSetDescription(change, patchNum)]]"
placeholder="[[_computeDescriptionPlaceholder(_descriptionReadOnly)]]"
read-only="[[_descriptionReadOnly]]"
on-changed="_handleDescriptionChanged"></gr-editable-label>

54
polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
View File

@@ -38,8 +38,16 @@
type: String,
notify: true,
},
/** @type {?} */
patchRange: Object,
patchNum: {
type: String,
notify: true,
observer: '_patchOrBaseChanged',
},
basePatchNum: {
type: String,
notify: true,
observer: '_patchOrBaseChanged',
},
revisions: Array,
// Caps the number of files that can be shown and have the 'show diffs' /
// 'hide diffs' buttons still be functional.
@@ -80,7 +88,6 @@
rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : '';
},
/**
* @param {!Object} revisions The revisions object keyed by revision hashes
* @param {?Object} patchSet A revision already fetched from {revisions}
@@ -98,10 +105,10 @@
_handleDescriptionChanged(e) {
const desc = e.detail.trim();
const rev = this.getRevisionByPatchNum(this.change.revisions,
this.patchRange.patchNum);
this.patchNum);
const sha = this._getPatchsetHash(this.change.revisions, rev);
this.$.restAPI.setDescription(this.changeNum,
this.patchRange.patchNum, desc)
this.patchNum, desc)
.then(res => {
if (res.ok) {
this.set(['_change', 'revisions', sha, 'description'], desc);
@@ -137,35 +144,18 @@
this.findSortedIndex(basePatchNum, this.revisions);
},
/**
* Change active patch to the provided patch num.
* @param {number|string} basePatchNum the base patch to be viewed.
* @param {number|string} patchNum the patch number to be viewed.
* @param {boolean} opt_forceParams When set to true, the resulting URL will
* always include the patch range, even if the requested patchNum is
* known to be the latest.
/*
* Triggered by _patchNum and _basePatchNum observer, in order to detect if
* the patch has been previously set or not. The new patch number is not
* explicitly used, because this could be called by either _patchNum or
* _basePatchNum's observer. Since the behavior is the same, they are
* combined.
*/
_changePatchNum(basePatchNum, patchNum, opt_forceParams) {
if (!opt_forceParams) {
let currentPatchNum;
if (this.change.current_revision) {
currentPatchNum =
this.change.revisions[this.change.current_revision]._number;
} else {
currentPatchNum = this.computeLatestPatchNum(this.allPatchSets);
}
if (this.patchNumEquals(patchNum, currentPatchNum) &&
basePatchNum === 'PARENT') {
Gerrit.Nav.navigateToChange(this.change);
return;
}
}
Gerrit.Nav.navigateToChange(this.change, patchNum,
basePatchNum);
},
_patchOrBaseChanged(patchNew, patchOld) {
if (!patchOld) { return; }
_handlePatchChange(e) {
this._changePatchNum(e.detail.leftPatch, e.detail.rightPatch, true);
Gerrit.Nav.navigateToChange(this.change, this.patchNum,
this.basePatchNum);
},
_handlePrefsTap(e) {

48
polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
View File

@@ -43,11 +43,9 @@ limitations under the License.
suite('gr-file-list-header tests', () => {
let element;
let sandbox;
let navigateToChangeStub;
setup(() => {
sandbox = sinon.sandbox.create();
navigateToChangeStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({test: 'config'}); },
getAccount() { return Promise.resolve(null); },
@@ -129,10 +127,9 @@ limitations under the License.
sandbox.stub(element, '_computeDescriptionReadOnly');
element.changeNum = '42';
element.patchRange = {
basePatchNum: 'PARENT',
patchNum: 1,
};
element.basePatchNum = 'PARENT';
element.patchNum = 1;
element.change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
@@ -180,10 +177,8 @@ limitations under the License.
const computeSpy = sandbox.spy(element, '_fileListActionsVisible');
element._files = [];
element.changeNum = '42';
element.patchRange = {
basePatchNum: 'PARENT',
patchNum: '2',
};
element.basePatchNum = 'PARENT';
element.patchNum = '2';
element.shownFileCount = 1;
flush(() => {
assert.isTrue(computeSpy.lastCall.returnValue);
@@ -206,21 +201,8 @@ limitations under the License.
assert.equal(select.nativeSelect.value, 'UNIFIED_DIFF');
});
test('_changePatchNum called when range select changes', () => {
const leftPatch = 1;
const rightPatch = 2;
sandbox.stub(element, '_changePatchNum');
element.$.rangeSelect.fire('patch-range-change', {leftPatch, rightPatch});
assert.isTrue(element._changePatchNum.lastCall
.calledWithExactly(1, 2, true));
});
test('include base patch when not parent', () => {
element.changeNum = '42';
element.patchRange = {
basePatchNum: '2',
patchNum: '3',
};
test('navigateToChange called when range select changes', () => {
const navigateToChangeStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
element.change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
@@ -232,16 +214,12 @@ limitations under the License.
status: 'NEW',
labels: {},
};
element._changePatchNum(2, 13);
assert.isTrue(navigateToChangeStub.lastCall.calledWithExactly(
element.change, 13, 2));
element.patchRange.basePatchNum = 'PARENT';
element._changePatchNum('PARENT', 3);
assert.isTrue(navigateToChangeStub.lastCall.calledWithExactly(
element.change, 3, 'PARENT'));
element.basePatchNum = 1;
element.patchNum = 2;
element.patchNum = 3;
assert.equal(navigateToChangeStub.callCount, 1);
assert.isTrue(navigateToChangeStub.lastCall
.calledWithExactly(element.change, 3, 1));
});
test('class is applied to file list on old patch set', () => {

8
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
View File

@@ -283,11 +283,11 @@ limitations under the License.
<gr-patch-range-select
id="rangeSelect"
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
patch-num="{{_patchNum}}"
base-patch-num="{{_basePatchNum}}"
files-weblinks="[[_filesWeblinks]]"
available-patches="[[_computeAvailablePatches(_change.revisions, _change.revisions.*)]]"
revisions="[[_change.revisions]]"
on-patch-range-change="_handlePatchChange">
available-patches="[[_allPatchSets]]"
revisions="[[_change.revisions]]">
</gr-patch-range-select>
<span class="download desktop">
<span class="separator">/</span>

45
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -63,6 +63,22 @@
},
_patchRange: Object,
// These are kept as separate properties from the patchRange so that the
// observer can be aware of the previous value. In order to view sub
// property changes for _patchRange, a complex observer must be used, and
// that only displays the new value.
//
// If a previous value did not exist, the change is not reloaded with the
// new patches. This is just the initial setting from the change view vs.
// an update coming from the two way data binding.
_patchNum: {
type: String,
observer: '_patchOrBaseChanged',
},
_basePatchNum: {
type: String,
observer: '_patchOrBaseChanged',
},
/**
* @type {{
* subject: string,
@@ -124,7 +140,6 @@
type: Boolean,
computed: '_computeEditLoaded(_patchRange.*)',
},
_isBlameSupported: {
type: Boolean,
value: false,
@@ -134,6 +149,10 @@
type: Boolean,
value: false,
},
_allPatchSets: {
type: Array,
computed: 'computeAllPatchSets(_change, _change.revisions.*)',
},
},
behaviors: [
@@ -147,6 +166,7 @@
'_getProjectConfig(_change.project)',
'_getFiles(_changeNum, _patchRange.*)',
'_setReviewedObserver(_loggedIn, params.*)',
'_patchRangeChanged(_patchRange.*)',
],
keyBindings: {
@@ -563,6 +583,17 @@
this.$.cursor.initialLineNumber = params.lineNum;
},
_patchRangeChanged() {
this._basePatchNum = this._patchRange.basePatchNum;
this._patchNum = this._patchRange.patchNum;
},
_patchOrBaseChanged(patchNew, patchOld) {
if (!patchOld) { return; }
this._handlePatchChange(this._basePatchNum, this._patchNum);
},
_pathChanged(path) {
if (path) {
this.fire('title-change',
@@ -593,12 +624,6 @@
return patchStr;
},
_computeAvailablePatches(revs) {
return this.sortRevisions(Object.values(revs)).map(e => {
return {num: e._number};
});
},
/**
* When the latest patch of the change is selected (and there is no base
* patch) then the patch range need not appear in the URL. Return a patch
@@ -675,11 +700,9 @@
this.$.dropdown.open();
},
_handlePatchChange(e) {
const rightPatch = e.detail.rightPatch;
const leftPatch = e.detail.leftPatch;
_handlePatchChange(basePatchNum, patchNum) {
Gerrit.Nav.navigateToDiff(
this._change, this._path, rightPatch, leftPatch);
this._change, this._path, patchNum, basePatchNum);
},
_handlePrefsTap(e) {

18
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
View File

@@ -163,6 +163,7 @@ limitations under the License.
_number: 42,
revisions: {
a: {_number: 10},
b: {_number: 5},
},
};
element._fileList = ['chell.go', 'glados.txt', 'wheatley.md'];
@@ -438,15 +439,23 @@ limitations under the License.
});
test('_handlePatchChange calls navigateToDiff correctly', () => {
const leftPatch = 'PARENT';
const rightPatch = '3';
const navigateStub = sandbox.stub(Gerrit.Nav, 'navigateToDiff');
element._change = {_number: 321, project: 'foo/bar'};
element._path = 'path/to/file.txt';
element.$.rangeSelect.fire('patch-range-change', {leftPatch, rightPatch});
element._patchRange = {
basePatchNum: 'PARENT',
patchNum: '3',
};
assert.equal(element._basePatchNum, element._patchRange.basePatchNum);
assert.equal(element._patchNum, element._patchRange.patchNum);
element._patchNum = '1';
assert(navigateStub.lastCall.calledWithExactly(element._change,
element._path, rightPatch, leftPatch));
element._path, '1', 'PARENT'));
});
test('download link', () => {
@@ -759,6 +768,7 @@ limitations under the License.
};
test('reviewed checkbox', () => {
sandbox.stub(element, '_handlePatchChange');
element._patchRange = {patchNum: '1'};
// Reviewed checkbox should be shown.
assert.isTrue(isVisible(element.$.reviewed));

47
polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html
View File

@@ -13,11 +13,13 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-dropdown-list/gr-dropdown-list.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">
<dom-module id="gr-patch-range-select">
<template>
@@ -32,27 +34,14 @@ limitations under the License.
.filesWeblinks {
display: none;
}
select {
max-width: 5.25em;
}
}
</style>
Patch set:
<span class="patchRange">
<gr-select id="leftPatchSelect" bind-value="{{_leftSelected}}"
on-change="_handlePatchChange">
<select>
<option value="PARENT">Base</option>
<template is="dom-repeat" items="{{availablePatches}}" as="basePatchNum">
<option value$="[[basePatchNum.num]]"
disabled$="[[_computeLeftDisabled(basePatchNum.num, patchRange.patchNum, _sortedRevisions)]]">
[[basePatchNum.num]]
[[_computePatchSetCommentsString(comments, basePatchNum.num)]]
[[_computePatchSetDescription(revisions, basePatchNum.num)]]
</option>
</template>
</select>
</gr-select>
<gr-dropdown-list
id="basePatchDropdown"
value="{{basePatchNum}}"
items="[[_baseDropdownContent]]">
</gr-dropdown-list>
</span>
<span is="dom-if" if="[[filesWeblinks.meta_a]]" class="filesWeblinks">
<template is="dom-repeat" items="[[filesWeblinks.meta_a]]" as="weblink">
@@ -62,19 +51,11 @@ limitations under the License.
</span>
&rarr;
<span class="patchRange">
<gr-select id="rightPatchSelect" bind-value="{{_rightSelected}}"
on-change="_handlePatchChange">
<select>
<template is="dom-repeat" items="{{availablePatches}}" as="patchNum">
<option value$="[[patchNum.num]]"
disabled$="[[_computeRightDisabled(patchNum.num, patchRange.basePatchNum, _sortedRevisions)]]">
[[patchNum.num]]
[[_computePatchSetCommentsString(comments, patchNum.num)]]
[[_computePatchSetDescription(revisions, patchNum.num)]]
</option>
</template>
</select>
</gr-select>
<gr-dropdown-list
id="patchNumDropdown"
value="{{patchNum}}"
items="[[_patchDropdownContent]]">
</gr-dropdown-list>
<span is="dom-if" if="[[filesWeblinks.meta_b]]" class="filesWeblinks">
<template is="dom-repeat" items="[[filesWeblinks.meta_b]]" as="weblink">
<a target="_blank"

98
polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
View File

@@ -31,28 +31,89 @@
properties: {
availablePatches: Array,
_baseDropdownContent: {
type: Object,
computed: '_computeBaseDropdownContent(availablePatches, patchNum,' +
'_sortedRevisions, revisions)',
},
_patchDropdownContent: {
type: Object,
computed: '_computePatchDropdownContent(availablePatches,' +
'basePatchNum, _sortedRevisions, revisions)',
},
changeNum: String,
comments: Array,
/** @type {{ meta_a: !Array, meta_b: !Array}} */
filesWeblinks: Object,
/** @type {?} */
patchRange: Object,
patchNum: {
type: String,
notify: true,
},
basePatchNum: {
type: String,
notify: true,
},
revisions: Object,
_sortedRevisions: Array,
_rightSelected: String,
_leftSelected: String,
},
observers: [
'_updateSortedRevisions(revisions.*)',
'_updateSelected(patchRange.*)',
],
behaviors: [Gerrit.PatchSetBehavior],
_updateSelected() {
this._rightSelected = this.patchRange.patchNum;
this._leftSelected = this.patchRange.basePatchNum;
_computeBaseDropdownContent(availablePatches, patchNum, _sortedRevisions,
revisions) {
const dropdownContent = [];
dropdownContent.push({
text: 'Base',
value: 'PARENT',
});
for (const basePatch of availablePatches) {
const basePatchNum = basePatch.num;
dropdownContent.push({
disabled: this._computeLeftDisabled(
basePatch.num, patchNum, _sortedRevisions),
triggerText: `Patchset ${basePatchNum}`,
text: `Patchset ${basePatchNum}` +
this._computePatchSetCommentsString(this.comments, basePatchNum),
mobileText: this._computeMobileText(basePatchNum, this.comments,
revisions),
bottomText: `${this._computePatchSetDescription(
revisions, basePatchNum)}`,
value: basePatch.num,
});
}
return dropdownContent;
},
_computeMobileText(patchNum, comments, revisions) {
return `${patchNum}` +
`${this._computePatchSetCommentsString(this.comments, patchNum)}` +
`${this._computePatchSetDescription(revisions, patchNum, true)}`;
},
_computePatchDropdownContent(availablePatches, basePatchNum,
_sortedRevisions, revisions) {
const dropdownContent = [];
for (const patch of availablePatches) {
const patchNum = patch.num;
dropdownContent.push({
disabled: this._computeRightDisabled(patchNum, basePatchNum,
_sortedRevisions),
triggerText: `Patchset ${patchNum}`,
text: `Patchset ${patchNum}` +
`${this._computePatchSetCommentsString(
this.comments, patchNum)}`,
mobileText: this._computeMobileText(patchNum, this.comments,
revisions),
bottomText: `${this._computePatchSetDescription(
revisions, patchNum)}`,
value: patchNum,
});
}
return dropdownContent;
},
_updateSortedRevisions(revisionsRecord) {
@@ -60,13 +121,6 @@
this._sortedRevisions = this.sortRevisions(Object.values(revisions));
},
_handlePatchChange(e) {
const leftPatch = this._leftSelected;
const rightPatch = this._rightSelected;
this.fire('patch-range-change', {rightPatch, leftPatch});
e.target.blur();
},
_computeLeftDisabled(basePatchNum, patchNum, sortedRevisions) {
return this.findSortedIndex(basePatchNum, sortedRevisions) >=
this.findSortedIndex(patchNum, sortedRevisions);
@@ -85,11 +139,11 @@
// debounce these, but because they are detecting two different
// events, sometimes the timing was off and one ended up missing.
_synchronizeSelectionRight() {
this.$.rightPatchSelect.value = this._rightSelected;
this.$.rightPatchSelect.value = this.patchNum;
},
_synchronizeSelectionLeft() {
this.$.leftPatchSelect.value = this._leftSelected;
this.$.leftPatchSelect.value = this.basePatchNum;
},
// Copied from gr-file-list
@@ -145,7 +199,7 @@
}
let commentsStr = '';
if (numComments > 0) {
commentsStr = '(' + numComments + ' comments';
commentsStr = ' (' + numComments + ' comments';
if (numUnresolved > 0) {
commentsStr += ', ' + numUnresolved + ' unresolved';
}
@@ -154,9 +208,15 @@
return commentsStr;
},
_computePatchSetDescription(revisions, patchNum) {
/**
* @param {!Array} revisions
* @param {number|string} patchNum
* @param {boolean=} opt_addFrontSpace
*/
_computePatchSetDescription(revisions, patchNum, opt_addFrontSpace) {
const rev = this.getRevisionByPatchNum(revisions, patchNum);
return (rev && rev.description) ?
(opt_addFrontSpace ? ' ' : '') +
rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : '';
},
});

256
polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
View File

@@ -79,19 +79,74 @@ limitations under the License.
patchRange.basePatchNum, sortedRevisions));
});
test('_updateSelected called with subproperty changes', () => {
sandbox.stub(element, '_updateSelected');
element.patchRange = {patchNum: 1, basePatchNum: 'PARENT'};
assert.equal(element._updateSelected.callCount, 1);
element.set('patchRange.patchNum', 2);
assert.equal(element._updateSelected.callCount, 2);
element.set('patchRange.basePatchNum', 1);
assert.equal(element._updateSelected.callCount, 3);
test('_computeBaseDropdownContent', () => {
element.comments = {};
const availablePatches = [
{num: 1},
{num: 2},
{num: 3},
{num: 'edit'},
];
const revisions = [
{
commit: {},
_number: 2,
description: 'description',
},
{commit: {}},
{commit: {}},
{commit: {}},
];
const patchNum = 1;
const sortedRevisions = [
{_number: 1},
{_number: 2},
{_number: element.EDIT_NAME, basePatchNum: 2},
{_number: 3},
];
const expectedResult = [
{
text: 'Base',
value: 'PARENT',
},
{
disabled: true,
triggerText: 'Patchset 1',
text: 'Patchset 1',
mobileText: '1',
bottomText: '',
value: 1,
},
{
disabled: true,
triggerText: 'Patchset 2',
text: 'Patchset 2',
mobileText: '2 description',
bottomText: 'description',
value: 2,
},
{
disabled: true,
triggerText: 'Patchset 3',
text: 'Patchset 3',
mobileText: '3',
bottomText: '',
value: 3,
},
{
disabled: true,
triggerText: 'Patchset edit',
text: 'Patchset edit',
mobileText: 'edit',
bottomText: '',
value: 'edit',
},
];
assert.deepEqual(element._computeBaseDropdownContent(availablePatches,
patchNum, sortedRevisions, revisions), expectedResult);
});
test('_computeLeftDisabled called when patchNum updates', () => {
test('_computeBaseDropdownContent called when patchNum updates', () => {
element.revisions = [
{commit: {}},
{commit: {}},
@@ -104,118 +159,103 @@ limitations under the License.
{num: 3},
{num: 'edit'},
];
element.patchRange = {patchNum: 2, basePatchNum: 'PARENT'};
element.patchNum = 2;
element.basePatchNum = 'PARENT';
flushAsynchronousOperations();
sandbox.stub(element, '_computeBaseDropdownContent');
// Should be recomputed for each available patch
element.set('patchNum', 1);
assert.equal(element._computeBaseDropdownContent.callCount, 1);
});
test('_computePatchDropdownContent called when basePatchNum updates', () => {
element.revisions = [
{commit: {}},
{commit: {}},
{commit: {}},
{commit: {}},
];
element.availablePatches = [
{num: 1},
{num: 2},
{num: 3},
{num: 'edit'},
];
element.patchNum = 2;
element.basePatchNum = 'PARENT';
flushAsynchronousOperations();
// Should be recomputed for each available patch
sandbox.stub(element, '_computeLeftDisabled');
element.set('patchRange.patchNum', '1');
assert.equal(element._computeLeftDisabled.callCount, 4);
sandbox.stub(element, '_computePatchDropdownContent');
element.set('basePatchNum', 1);
assert.equal(element._computePatchDropdownContent.callCount, 1);
});
test('_computeRightDisabled called when basePatchNum updates', () => {
element.revisions = [
{commit: {}},
{commit: {}},
{commit: {}},
{commit: {}},
];
element.availablePatches = [
test('_computePatchDropdownContent', () => {
element.comments = {};
const availablePatches = [
{num: 1},
{num: 2},
{num: 3},
{num: 'edit'},
];
element.patchRange = {patchNum: 2, basePatchNum: 'PARENT'};
flushAsynchronousOperations();
// Should be recomputed for each available patch
sandbox.stub(element, '_computeRightDisabled');
element.set('patchRange.basePatchNum', '1');
assert.equal(element._computeRightDisabled.callCount, 4);
});
test('changes in patch range fire event', done => {
sandbox.stub(element, '_computeLeftDisabled').returns(false);
sandbox.stub(element, '_computeRightDisabled').returns(false);
const patchRangeChangedStub = sandbox.stub();
element.addEventListener('patch-range-change', patchRangeChangedStub);
const leftSelectEl = element.$.leftPatchSelect;
const rightSelectEl = element.$.rightPatchSelect;
const blurSpy = sandbox.spy(leftSelectEl, 'blur');
element.changeNum = '42';
element.path = 'path/to/file.txt';
element.availablePatches =
[{num: '1'}, {num: '2'}, {num: '3'}, {num: 'edit'}];
element.patchRange = {
basePatchNum: 'PARENT',
patchNum: '3',
};
flushAsynchronousOperations();
let numEvents = 0;
leftSelectEl.addEventListener('change', e => {
numEvents++;
if (numEvents === 1) {
assert.deepEqual(patchRangeChangedStub.lastCall.args[0].detail,
{rightPatch: '3', leftPatch: 'PARENT'});
leftSelectEl.nativeSelect.value = 'edit';
element.fire('change', {}, {node: leftSelectEl});
assert(blurSpy.called, 'Dropdown should be blurred after selection');
} else if (numEvents === 2) {
assert.deepEqual(patchRangeChangedStub.lastCall.args[0].detail,
{rightPatch: '3', leftPatch: 'edit'});
rightSelectEl.nativeSelect.value = '1';
element.fire('change', {}, {node: rightSelectEl});
}
});
rightSelectEl.addEventListener('change', e => {
assert.deepEqual(patchRangeChangedStub.lastCall.args[0].detail,
{rightPatch: '1', leftPatch: 'edit'});
done();
});
leftSelectEl.nativeSelect.value = 'PARENT';
rightSelectEl.nativeSelect.value = '3';
element.fire('change', {}, {node: leftSelectEl});
});
test('diff against dropdown', done => {
element.revisions = [
{commit: {}},
const revisions = [
{
commit: {},
_number: 2,
description: 'description',
},
{commit: {}},
{commit: {}},
{commit: {}},
];
element.availablePatches = [
{num: 1},
{num: 2},
{num: 3},
{num: 'edit'},
const basePatchNum = 1;
const sortedRevisions = [
{_number: 1},
{_number: 2},
{_number: element.EDIT_NAME, basePatchNum: 2},
{_number: 3},
];
element.patchRange = {
basePatchNum: 'PARENT',
patchNum: '3',
};
const patchRangeChangedStub = sandbox.stub();
element.addEventListener('patch-range-change', patchRangeChangedStub);
const expectedResult = [
{
disabled: true,
triggerText: 'Patchset 1',
text: 'Patchset 1',
mobileText: '1',
bottomText: '',
value: 1,
},
{
disabled: false,
triggerText: 'Patchset 2',
text: 'Patchset 2',
mobileText: '2 description',
bottomText: 'description',
value: 2,
},
{
disabled: false,
triggerText: 'Patchset 3',
text: 'Patchset 3',
mobileText: '3',
bottomText: '',
value: 3,
},
{
disabled: false,
triggerText: 'Patchset edit',
text: 'Patchset edit',
mobileText: 'edit',
bottomText: '',
value: 'edit',
},
];
flush(() => {
const selectEl = element.$.leftPatchSelect;
assert.equal(selectEl.nativeSelect.value, 'PARENT');
assert.isTrue(element.$$('#leftPatchSelect option[value="3"]')
.hasAttribute('disabled'));
selectEl.addEventListener('change', () => {
assert.equal(selectEl.nativeSelect.value, 'edit');
assert.deepEqual(patchRangeChangedStub.lastCall.args[0].detail,
{leftPatch: 'edit', rightPatch: '3'});
done();
});
selectEl.nativeSelect.value = 'edit';
element.fire('change', {}, {node: selectEl.nativeSelect});
});
assert.deepEqual(element._computePatchDropdownContent(availablePatches,
basePatchNum, sortedRevisions, revisions), expectedResult);
});
test('filesWeblinks', () => {
@@ -264,12 +304,12 @@ limitations under the License.
};
assert.equal(element._computePatchSetCommentsString(comments, 1),
'(3 comments, 1 unresolved)');
' (3 comments, 1 unresolved)');
// Test string with no unresolved comments.
delete comments['foo'];
assert.equal(element._computePatchSetCommentsString(comments, 1),
'(2 comments)');
' (2 comments)');
// Test string with no comments.
delete comments['bar'];

25
polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
View File

@@ -62,6 +62,7 @@ limitations under the License.
paper-item {
cursor: pointer;
flex-direction: column;
font-size: 1em;
--paper-item: {
min-height: 0;
padding: 10px 16px;
@@ -79,7 +80,7 @@ limitations under the License.
paper-item:not(:last-of-type) {
border-bottom: 1px solid #ddd;
}
gr-button {
#trigger {
color: black;
font: inherit;
padding: .3em 0;
@@ -87,7 +88,7 @@ limitations under the License.
}
.bottomContent {
color: rgba(0,0,0,.54);
font-size: .85em;
font-size: .9em;
line-height: 16px;
}
.bottomContent,
@@ -105,6 +106,16 @@ limitations under the License.
gr-select {
display: none;
}
/* Because the iron dropdown 'area' includes the trigger, and the entire
width of the dropdown, we want to treat tapping the area above the
dropdown content as if it is tapping whatever content is underneath it.
The next two styles allow this to happen. */
iron-dropdown {
pointer-events: none;
}
paper-listbox {
pointer-events: auto;
}
@media only screen and (max-width: 50em) {
gr-select {
display: inline;
@@ -122,7 +133,8 @@ limitations under the License.
link
id="trigger"
class="dropdown-trigger"
on-tap="_showDropdownTapHandler">
on-tap="_showDropdownTapHandler"
slot="dropdown-trigger">
<span>[[text]]</span>
<span
class="downArrow"
@@ -131,13 +143,14 @@ limitations under the License.
<iron-dropdown
id="dropdown"
vertical-align="top"
allow-outside-scroll="true">
allow-outside-scroll="true"
on-tap="_handleDropdownTap">
<paper-listbox
class="dropdown-content"
slot="dropdown-content"
attr-for-selected="value"
on-tap="_handleDropdownTap"
selected="{{value}}">
selected="{{value}}"
on-tap="_handleDropdownTap">
<template is="dom-repeat" items="[[items]]">
<paper-item
disabled="[[item.disabled]]"

1
polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
View File

@@ -95,6 +95,7 @@
const selectedObj = items.find(item => {
return item.value + '' === value + '';
});
if (!selectedObj) { return; }
this.text = selectedObj.triggerText? selectedObj.triggerText :
selectedObj.text;
},