Merge "Modify gr-change-requirements to use label chips"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index 6ea7cf3..ae2a7d0 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -98,10 +98,10 @@
font-family: var(--monospace-font-family);
}
.u-green {
- color: #388E3C;
+ color: var(--vote-text-color-recommended);
}
.u-red {
- color: #D32F2F;
+ color: var(--vote-text-color-disliked);
}
.label.u-green:not(.u-monospace),
.label.u-red:not(.u-monospace) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html
index ff9e547..b056233 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html
@@ -145,7 +145,6 @@
sandbox.stub(Gerrit, '_arePluginsLoaded').returns(true);
Gerrit._setPluginsPending([]);
element = createElement();
- sandbox.stub(element, '_computeCanDeleteVote').returns(true);
labelChangeStub = sandbox.stub();
plugin.changeMetadata().onLabelsChanged(labelChangeStub);
@@ -156,8 +155,18 @@
assert.equal(labelChangeStub.callCount, 1);
assert.isTrue(labelChangeStub.calledWithExactly(labels));
assert.equal(labelChangeStub.args[0][0]['CI'].all.length, 2);
- MockInteractions.tap(Polymer.dom(element.root).querySelector(
- 'gr-account-chip').$.remove);
+ element.set(['change', 'labels'], {
+ CI: {
+ all: [
+ {value: 1, name: 'user 2', _account_id: 1},
+ ],
+ values: {
+ ' 0': 'Don\'t submit as-is',
+ '+1': 'No score',
+ '+2': 'Looks good to me',
+ },
+ },
+ });
// Wait for fake rest API response.
flush(() => {
assert.equal(labelChangeStub.callCount, 2);
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index cd7ec2b..c326bf8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -27,7 +27,6 @@
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
<link rel="import" href="../../shared/gr-editable-label/gr-editable-label.html">
-<link rel="import" href="../../shared/gr-label/gr-label.html">
<link rel="import" href="../../shared/gr-limited-text/gr-limited-text.html">
<link rel="import" href="../../shared/gr-linked-chip/gr-linked-chip.html">
<link rel="import" href="../../shared/gr-tooltip-content/gr-tooltip-content.html">
@@ -38,7 +37,6 @@
<dom-module id="gr-change-metadata">
<template>
- <style include="gr-voting-styles"></style>
<style include="shared-styles">
.hideDisplay {
display: none;
@@ -66,35 +64,6 @@
gr-editable-label {
max-width: 9em;
}
- .labelValueContainer:not(:first-of-type) {
- margin-top: .25em;
- }
- .labelValueContainer span {
- align-items: baseline;
- display: inline-flex;
- }
- .labelValueContainer {
- border-radius: 3px;
- padding: .1em .3em;
- }
- gr-label {
- margin-right: .3em;
- padding: .05em .85em;
- text-align: center;
- @apply --vote-chip-styles;
- }
- .max {
- background-color: var(--vote-color-approved);
- }
- .min {
- background-color: var(--vote-color-rejected);
- }
- .positive {
- background-color: var(--vote-color-recommended);
- }
- .negative {
- background-color: var(--vote-color-disliked);
- }
.webLink {
display: block;
}
@@ -133,6 +102,11 @@
--arrow-color: #ffa62f;
display: inline-block;
}
+ .separatedSection {
+ border-top: 1px solid var(--border-color);
+ margin-top: .5em;
+ padding-top: .5em;
+ }
@media screen and (max-width: 50em), screen and (min-width: 75em) {
:host {
display: table;
@@ -308,43 +282,12 @@
</span>
</section>
</template>
- <template is="dom-repeat"
- items="[[_computeLabelNames(labels)]]" as="labelName">
- <section>
- <span class="title">[[labelName]]</span>
- <span class="value">
- <template is="dom-repeat"
- items="[[_computeLabelValues(labelName, labels.*)]]"
- as="label">
- <div class="labelValueContainer">
- <span>
- <gr-label
- has-tooltip
- title="[[_computeValueTooltip(change, label.value, labelName)]]"
- class$="[[label.className]] voteChip">
- [[label.value]]
- </gr-label>
- <gr-account-chip
- account="[[label.account]]"
- data-account-id$="[[label.account._account_id]]"
- label-name="[[labelName]]"
- removable="[[_computeCanDeleteVote(label.account, mutable)]]"
- transparent-background
- on-remove="_onDeleteVote"></gr-account-chip>
- </span>
- </div>
- </template>
- </span>
- </section>
- </template>
- <template is="dom-if" if="[[_showRequirements]]">
- <section class="requirementsStatus">
- <span class="title">Submit Status</span>
- <span class="value">
- <gr-change-requirements change="[[change]]"></gr-change-requirements>
- </span>
- </section>
- </template>
+ <div class="separatedSection">
+ <gr-change-requirements
+ change="{{change}}"
+ account="[[account]]"
+ mutable="[[mutable]]"></gr-change-requirements>
+ </div>
<section id="webLinks" hidden$="[[!_computeWebLinks(commitInfo)]]">
<span class="title">Links</span>
<span class="value">
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index aaada4f..113be67 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -46,10 +46,14 @@
type: Object,
notify: true,
},
+ account: Object,
/** @type {?} */
revision: Object,
commitInfo: Object,
- mutable: Boolean,
+ mutable: {
+ type: Boolean,
+ computed: '_computeIsMutable(account)',
+ },
/**
* @type {{ note_db_enabled: string }}
*/
@@ -156,60 +160,6 @@
return Object.keys(labels).sort();
},
- _computeLabelValues(labelName, _labels) {
- const result = [];
- const labels = _labels.base;
- const labelInfo = labels[labelName];
- if (!labelInfo) { return result; }
- if (!labelInfo.values) {
- if (labelInfo.rejected || labelInfo.approved) {
- const ok = labelInfo.approved || !labelInfo.rejected;
- return [{
- value: ok ? '👍️' : '👎️',
- className: ok ? 'positive' : 'negative',
- account: ok ? labelInfo.approved : labelInfo.rejected,
- }];
- }
- return result;
- }
- const approvals = labelInfo.all || [];
- const values = Object.keys(labelInfo.values);
- for (const label of approvals) {
- if (label.value && label.value != labels[labelName].default_value) {
- let labelClassName;
- let labelValPrefix = '';
- if (label.value > 0) {
- labelValPrefix = '+';
- if (parseInt(label.value, 10) ===
- parseInt(values[values.length - 1], 10)) {
- labelClassName = 'max';
- } else {
- labelClassName = 'positive';
- }
- } else if (label.value < 0) {
- if (parseInt(label.value, 10) === parseInt(values[0], 10)) {
- labelClassName = 'min';
- } else {
- labelClassName = 'negative';
- }
- }
- result.push({
- value: labelValPrefix + label.value,
- className: labelClassName,
- account: label,
- });
- }
- }
- return result;
- },
-
- _computeValueTooltip(change, score, labelName) {
- if (!change.labels[labelName] ||
- !change.labels[labelName].values ||
- !change.labels[labelName].values[score]) { return ''; }
- return change.labels[labelName].values[score];
- },
-
_handleTopicChanged(e, topic) {
const lastTopic = this.change.topic;
if (!topic.length) { topic = null; }
@@ -298,75 +248,6 @@
return hasRequirements || hasLabels || !!change.work_in_progress;
},
- /**
- * A user is able to delete a vote iff the mutable property is true and the
- * reviewer that left the vote exists in the list of removable_reviewers
- * received from the backend.
- *
- * @param {!Object} reviewer An object describing the reviewer that left the
- * vote.
- * @param {boolean} mutable this.mutable describes whether the
- * change-metadata section is modifiable by the current user.
- */
- _computeCanDeleteVote(reviewer, mutable) {
- if (!mutable || !this.change || !this.change.removable_reviewers) {
- return false;
- }
- for (let i = 0; i < this.change.removable_reviewers.length; i++) {
- if (this.change.removable_reviewers[i]._account_id ===
- reviewer._account_id) {
- return true;
- }
- }
- return false;
- },
-
- /**
- * Closure annotation for Polymer.prototype.splice is off.
- * For now, supressing annotations.
- *
- * TODO(beckysiegel) submit Polymer PR
- *
- * @suppress {checkTypes} */
- _onDeleteVote(e) {
- e.preventDefault();
- const target = Polymer.dom(e).rootTarget;
- target.disabled = true;
- const labelName = target.labelName;
- const accountID = parseInt(target.getAttribute('data-account-id'), 10);
- this._xhrPromise =
- this.$.restAPI.deleteVote(this.change._number, accountID, labelName)
- .then(response => {
- target.disabled = false;
- if (!response.ok) { return response; }
- const label = this.change.labels[labelName];
- const labels = label.all || [];
- let wasChanged = false;
- for (let i = 0; i < labels.length; i++) {
- if (labels[i]._account_id === accountID) {
- for (const key in label) {
- if (label.hasOwnProperty(key) &&
- label[key]._account_id === accountID) {
- // Remove special label field, keeping change label values
- // in sync with the backend.
- this.change.labels[labelName][key] = null;
- wasChanged = true;
- }
- }
- this.change.labels[labelName].all.splice(i, 1);
- wasChanged = true;
- break;
- }
- }
- if (wasChanged) {
- this.notifyPath('change.labels');
- }
- }).catch(err => {
- target.disabled = false;
- return;
- });
- },
-
_computeProjectURL(project) {
return Gerrit.Nav.getUrlForProjectChanges(project);
},
@@ -458,5 +339,9 @@
parentIsCurrent ? 'current' : 'notCurrent',
].join(' ');
},
+
+ _computeIsMutable(account) {
+ return !!Object.keys(account).length;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
index 1330451..17c3d70 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -296,32 +296,6 @@
element._computeShowUploaderHide(change), 'hideDisplay');
});
- test('_computeValueTooltip', () => {
- // Existing label.
- const change = {labels: {'Foo-bar': {values: {0: 'Baz'}}}};
- let score = '0';
- let labelName = 'Foo-bar';
- let actual = element._computeValueTooltip(change, score, labelName);
- assert.equal(actual, 'Baz');
-
- // Non-extsistent label.
- labelName = 'xyz';
- actual = element._computeValueTooltip(change, score, labelName);
- assert.equal(actual, '');
-
- // Non-extsistent score.
- score = '2';
- actual = element._computeValueTooltip(change, score, labelName);
- assert.equal(actual, '');
-
- // No values on label.
- labelName = 'abcd';
- score = '0';
- change.labels.abcd = {};
- actual = element._computeValueTooltip(change, score, labelName);
- assert.equal(actual, '');
- });
-
test('_computeParents', () => {
const parents = [{commit: '123', subject: 'abc'}];
assert.isUndefined(element._computeParents(
@@ -403,7 +377,7 @@
});
test('topic read only hides delete button', () => {
- element.mutable = false;
+ element.account = {};
element.change = change;
flushAsynchronousOperations();
const button = element.$$('gr-linked-chip').$$('gr-button');
@@ -411,7 +385,7 @@
});
test('topic not read only does not hide delete button', () => {
- element.mutable = true;
+ element.account = {test: true};
change.actions.topic.enabled = true;
element.change = change;
flushAsynchronousOperations();
@@ -463,7 +437,7 @@
note_db_enabled: true,
};
flushAsynchronousOperations();
- element.mutable = false;
+ element.account = {};
element.change = change;
flushAsynchronousOperations();
const button = element.$$('gr-linked-chip').$$('gr-button');
@@ -475,7 +449,7 @@
note_db_enabled: true,
};
flushAsynchronousOperations();
- element.mutable = true;
+ element.account = {test: true};
change.actions.hashtags.enabled = true;
element.change = change;
flushAsynchronousOperations();
@@ -486,7 +460,6 @@
suite('remove reviewer votes', () => {
setup(() => {
- sandbox.stub(element, '_computeValueTooltip').returns('');
sandbox.stub(element, '_computeTopicReadOnly').returns(true);
element.change = {
_number: 42,
@@ -507,44 +480,56 @@
flushAsynchronousOperations();
});
- test('_computeCanDeleteVote hides delete button', () => {
- const button = element.$$('gr-account-chip').$$('gr-button');
- assert.isTrue(button.hasAttribute('hidden'));
- element.mutable = true;
- assert.isTrue(button.hasAttribute('hidden'));
- });
-
- test('_computeCanDeleteVote shows delete button', () => {
- element.change.removable_reviewers = [
- {
- _account_id: 1,
- name: 'bojack',
- },
- ];
- element.mutable = true;
- const button = element.$$('gr-account-chip').$$('gr-button');
- assert.isFalse(button.hasAttribute('hidden'));
- });
-
- test('deletes votes', () => {
- const deleteResponse = Promise.resolve({ok: true});
- const deleteStub = sandbox.stub(
- element.$.restAPI, 'deleteVote').returns(deleteResponse);
-
- element.change.removable_reviewers = [{
+ suite('assignee field', () => {
+ const dummyAccount = {
_account_id: 1,
name: 'bojack',
- }];
- element.change.labels.test.recommended = {_account_id: 1};
- element.mutable = true;
- const chip = element.$$('gr-account-chip');
- const button = chip.$$('gr-button');
- MockInteractions.tap(button);
- assert.isTrue(chip.disabled);
- return deleteResponse.then(() => {
- assert.isFalse(chip.disabled);
- assert.notOk(element.change.labels.test.recommended);
- assert.isTrue(deleteStub.calledWithExactly(42, 1, 'test'));
+ };
+ const change = {
+ actions: {
+ assignee: {enabled: false},
+ },
+ assignee: dummyAccount,
+ };
+ let deleteStub;
+ let setStub;
+
+ setup(() => {
+ deleteStub = sandbox.stub(element.$.restAPI, 'deleteAssignee');
+ setStub = sandbox.stub(element.$.restAPI, 'setAssignee');
+ });
+
+ test('changing change recomputes _assignee', () => {
+ assert.isFalse(!!element._assignee.length);
+ const change = element.change;
+ change.assignee = dummyAccount;
+ element._changeChanged(change);
+ assert.deepEqual(element._assignee[0], dummyAccount);
+ });
+
+ test('modifying _assignee calls API', () => {
+ assert.isFalse(!!element._assignee.length);
+ element.set('_assignee', [dummyAccount]);
+ assert.isTrue(setStub.calledOnce);
+ assert.deepEqual(element.change.assignee, dummyAccount);
+ element.set('_assignee', [dummyAccount]);
+ assert.isTrue(setStub.calledOnce);
+ element.set('_assignee', []);
+ assert.isTrue(deleteStub.calledOnce);
+ assert.equal(element.change.assignee, undefined);
+ element.set('_assignee', []);
+ assert.isTrue(deleteStub.calledOnce);
+ });
+
+ test('_computeAssigneeReadOnly', () => {
+ let mutable = false;
+ assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
+ mutable = true;
+ assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
+ change.actions.assignee.enabled = true;
+ assert.isFalse(element._computeAssigneeReadOnly(mutable, change));
+ mutable = false;
+ assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
});
});
@@ -600,59 +585,6 @@
assert.equal(element.change.hashtags, newHashtag);
});
});
-
- suite('assignee field', () => {
- const dummyAccount = {
- _account_id: 1,
- name: 'bojack',
- };
- const change = {
- actions: {
- assignee: {enabled: false},
- },
- assignee: dummyAccount,
- };
- let deleteStub;
- let setStub;
-
- setup(() => {
- deleteStub = sandbox.stub(element.$.restAPI, 'deleteAssignee');
- setStub = sandbox.stub(element.$.restAPI, 'setAssignee');
- });
-
- test('changing change recomputes _assignee', () => {
- assert.isFalse(!!element._assignee.length);
- const change = element.change;
- change.assignee = dummyAccount;
- element._changeChanged(change);
- assert.deepEqual(element._assignee[0], dummyAccount);
- });
-
- test('modifying _assignee calls API', () => {
- assert.isFalse(!!element._assignee.length);
- element.set('_assignee', [dummyAccount]);
- assert.isTrue(setStub.calledOnce);
- assert.deepEqual(element.change.assignee, dummyAccount);
- element.set('_assignee', [dummyAccount]);
- assert.isTrue(setStub.calledOnce);
- element.set('_assignee', []);
- assert.isTrue(deleteStub.calledOnce);
- assert.equal(element.change.assignee, undefined);
- element.set('_assignee', []);
- assert.isTrue(deleteStub.calledOnce);
- });
-
- test('_computeAssigneeReadOnly', () => {
- let mutable = false;
- assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
- mutable = true;
- assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
- change.actions.assignee.enabled = true;
- assert.isFalse(element._computeAssigneeReadOnly(mutable, change));
- mutable = false;
- assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
- });
- });
});
suite('plugin endpoints', () => {
@@ -678,105 +610,5 @@
});
});
});
-
- suite('label colors', () => {
- test('valueless label rejected', () => {
- element.change = {
- labels: {
- 'Do-Not-Submit': {
- rejected: {name: 'someone'},
- },
- },
- };
- flushAsynchronousOperations();
- const labels = Polymer.dom(element.root).querySelectorAll('gr-label');
- assert.isTrue(labels[0].classList.contains('negative'));
- });
-
- test('valueless label approved', () => {
- element.change = {
- labels: {
- 'To-The-Infinity': {
- approved: {name: 'someone'},
- },
- },
- };
- flushAsynchronousOperations();
- const labels = Polymer.dom(element.root).querySelectorAll('gr-label');
- assert.isTrue(labels[0].classList.contains('positive'));
- });
-
- test('-2 to +2', () => {
- element.change = {
- labels: {
- 'Code-Review': {
- all: [
- {value: 2, name: 'user 2'},
- {value: 1, name: 'user 1'},
- {value: -1, name: 'user 3'},
- {value: -2, name: 'user 4'},
- ],
- values: {
- '-2': 'Awful',
- '-1': 'Don\'t submit as-is',
- ' 0': 'No score',
- '+1': 'Looks good to me',
- '+2': 'Ready to submit',
- },
- },
- },
- };
- flushAsynchronousOperations();
- const labels = Polymer.dom(element.root).querySelectorAll('gr-label');
- assert.isTrue(labels[0].classList.contains('max'));
- assert.isTrue(labels[1].classList.contains('positive'));
- assert.isTrue(labels[2].classList.contains('negative'));
- assert.isTrue(labels[3].classList.contains('min'));
- });
-
- test('-1 to +1', () => {
- element.change = {
- labels: {
- CI: {
- all: [
- {value: 1, name: 'user 1'},
- {value: -1, name: 'user 2'},
- ],
- values: {
- '-1': 'Don\'t submit as-is',
- ' 0': 'No score',
- '+1': 'Looks good to me',
- },
- },
- },
- };
- flushAsynchronousOperations();
- const labels = Polymer.dom(element.root).querySelectorAll('gr-label');
- assert.isTrue(labels[0].classList.contains('max'));
- assert.isTrue(labels[1].classList.contains('min'));
- });
-
- test('0 to +2', () => {
- element.change = {
- labels: {
- CI: {
- all: [
- {value: 1, name: 'user 2'},
- {value: 2, name: 'user '},
- ],
- values: {
- ' 0': 'Don\'t submit as-is',
- '+1': 'No score',
- '+2': 'Looks good to me',
- },
- },
- },
- };
- flushAsynchronousOperations();
- const labels = Polymer.dom(element.root).querySelectorAll('gr-label');
- assert.isTrue(labels[0].classList.contains('positive'));
- assert.isTrue(labels[1].classList.contains('max'));
- });
- });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
index 439677a..b49fa77 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
@@ -18,60 +18,142 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-hovercard/gr-hovercard.html">
<link rel="import" href="../../shared/gr-icons/gr-icons.html">
+<link rel="import" href="../../shared/gr-label/gr-label.html">
+<link rel="import" href="../../shared/gr-label-info/gr-label-info.html">
<dom-module id="gr-change-requirements">
<template strip-whitespace>
<style include="shared-styles">
.status {
display: inline-block;
- font-weight: initial;
+ font-family: var(--monospace-font-family);
text-align: center;
}
- .unsatisfied .icon {
+ .neutral .status {
color: #FFA62F;
}
- .satisfied .icon {
- color: #388E3C;
+ .positive .status {
+ color: var(--vote-text-color-recommended);
+ }
+ .negative .status {
+ color: var(--vote-text-color-disliked);
+ }
+ iron-icon {
+ color: inherit;
}
.requirement {
- padding: .1em 0;
+ align-items: center;
+ background-color: var(--table-header-background-color);
+ border: 1px solid var(--border-color);
+ border-radius: 1em;
+ display: flex;
+ font-weight: bold;
+ height: 2rem;
+ justify-content: space-between;
+ margin: .25em;
+ min-width: 3em;
+ padding: .2em .65em
}
- .requirementContainer:not(:first-of-type) {
- margin-top: .25em;
+ .status {
+ margin-left: .3em;
}
- .labelName, .changeIsWip {
+ .title {
font-weight: bold;
}
+ .required,
+ .optional {
+ display: block;
+ }
+ .optional {
+ margin-top: .5em;
+ }
+ .fieldContainer {
+ display: flex;
+ flex-wrap: wrap;
+ margin-top: .25em;
+ max-width: 25em;
+ }
+ .hidden {
+ display: none;
+ }
</style>
- <template is="dom-if" if="[[_showWip]]">
- <div class="requirement unsatisfied changeIsWip">
- <span class="status"><iron-icon class="icon" icon="gr-icons:hourglass"></iron-icon></span>
- Work in Progress
+ <div class="required">
+ <span class="title">Required for submission</span>
+ <div class="fieldContainer">
+ <template
+ is="dom-repeat"
+ items="[[_requiredLabels]]">
+ <gr-label
+ tabindex="0"
+ id$="[[item.label]]"
+ class$="requirement [[item.style]]">
+ <span class="labelName">[[_computeLabelShortcut(item.label)]]</span>
+ <span class="status">
+ <template is="dom-if" if="[[item.icon]]">
+ <iron-icon class="icon" icon="[[item.icon]]"></iron-icon>
+ </template>
+ <template is="dom-if" if="[[!item.icon]]">
+ <span>[[_computeLabelValue(item.labelInfo.value)]]</span>
+ </template>
+ </span>
+ </gr-label>
+ <gr-hovercard position="top" for="[[item.label]]">
+ <gr-label-info
+ change="{{change}}"
+ account="[[account]]"
+ mutable="[[mutable]]"
+ label="[[item.label]]"
+ label-info="[[item.labelInfo]]"></gr-label-info>
+ </gr-hovercard>
+ </template>
+ <template
+ is="dom-repeat"
+ items="[[_requirements]]">
+ <gr-label
+ tabindex="0"
+ class$="requirement [[_computeRequirementClass(item.satisfied)]]"
+ has-tooltip$="[[item.tooltip]]" title$="[[item.tooltip]]">
+ [[item.fallback_text]]
+ <span class="status">
+ <iron-icon class="icon" icon="[[_computeRequirementIcon(item.satisfied)]]"></iron-icon>
+ </span>
+ </gr-label>
+ </template>
</div>
- </template>
- <template is="dom-if" if="[[_showLabels]]">
- <template
- is="dom-repeat"
- items="[[labels]]">
- <div class$="requirement [[item.style]]">
- <span class="status">
- <iron-icon class="icon" icon="[[item.icon]]"></iron-icon>
- </span>
- Label <span class="labelName">[[item.label]]</span>
- </div>
- </template>
- </template>
- <template
- is="dom-repeat"
- items="[[requirements]]">
- <div class$="requirement [[_computeRequirementClass(item.satisfied)]]">
- <span class="status">
- <iron-icon class="icon" icon="[[_computeRequirementIcon(item.satisfied)]]"></iron-icon>
- </span>
- [[item.fallback_text]]
+ </div>
+ <div class$="optional [[_computeShowOptional(_optionalLabels.*)]]">
+ <span class="title">Optional for submission</span>
+ <div class="fieldContainer">
+ <template
+ is="dom-repeat"
+ items="[[_optionalLabels]]">
+ <gr-label
+ tabindex="0"
+ id$="[[item.label]]"
+ class$="requirement [[item.style]]">
+ <span class="labelName">[[_computeLabelShortcut(item.label)]]</span>
+ <span class="status">
+ <template is="dom-if" if="[[item.icon]]">
+ <iron-icon class="icon" icon="[[item.icon]]"></iron-icon>
+ </template>
+ <template is="dom-if" if="[[!item.icon]]">
+ <span>[[_computeLabelValue(item.labelInfo.value)]]</span>
+ </template>
+ </span>
+ </gr-label>
+ <gr-hovercard position="top" for="[[item.label]]">
+ <gr-label-info
+ change="{{change}}"
+ account="[[account]]"
+ mutable="[[mutable]]"
+ label="[[item.label]]"
+ label-info="[[item.labelInfo]]"></gr-label-info>
+ </gr-hovercard>
+ </template>
</div>
- </template>
+ </div>
</template>
<script src="gr-change-requirements.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
index 765f639..14b7875 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
@@ -23,31 +23,33 @@
properties: {
/** @type {?} */
change: Object,
- requirements: {
+ account: Object,
+ mutable: Boolean,
+ _requirements: {
type: Array,
computed: '_computeRequirements(change)',
},
- labels: {
+ _requiredLabels: {
type: Array,
- computed: '_computeLabels(change)',
+ value: () => [],
+ },
+ _optionalLabels: {
+ type: Array,
+ value: () => [],
},
_showWip: {
type: Boolean,
computed: '_computeShowWip(change)',
},
- _showLabels: {
- type: Boolean,
- computed: '_computeShowLabelStatus(change)',
- },
},
behaviors: [
Gerrit.RESTClientBehavior,
],
- _computeShowLabelStatus(change) {
- return change.status === this.ChangeStatus.NEW;
- },
+ observers: [
+ '_computeLabels(change.labels.*)',
+ ],
_computeShowWip(change) {
return change.work_in_progress;
@@ -62,41 +64,75 @@
_requirements.push(requirement);
}
}
+ if (change.work_in_progress) {
+ _requirements.push({
+ fallback_text: 'WIP',
+ tooltip: 'Change must not be in \'Work in Progress\' state.',
+ });
+ }
return _requirements;
},
- _computeLabels(change) {
- const labels = change.labels;
- const _labels = [];
-
- for (const label in labels) {
- if (!labels.hasOwnProperty(label)) { continue; }
- const obj = labels[label];
- if (obj.optional) { continue; }
-
- const icon = this._computeRequirementIcon(obj.approved);
- const style = this._computeRequirementClass(obj.approved);
- _labels.push({label, icon, style});
- }
-
- return _labels;
- },
-
_computeRequirementClass(requirementStatus) {
- if (requirementStatus) {
- return 'satisfied';
- } else {
- return 'unsatisfied';
- }
+ return requirementStatus ? 'positive' : 'neutral';
},
_computeRequirementIcon(requirementStatus) {
- if (requirementStatus) {
- return 'gr-icons:check';
- } else {
- return 'gr-icons:hourglass';
+ return requirementStatus ? 'gr-icons:check' : 'gr-icons:hourglass';
+ },
+
+ _computeLabels(labelsRecord) {
+ const labels = labelsRecord.base;
+ this._optionalLabels = [];
+ this._requiredLabels = [];
+
+ for (const label in labels) {
+ if (!labels.hasOwnProperty(label)) { continue; }
+
+ const labelInfo = labels[label];
+ const icon = this._computeLabelIcon(labelInfo);
+ const style = this._computeLabelClass(labelInfo);
+ const path = labelInfo.optional ? '_optionalLabels' : '_requiredLabels';
+
+ this.push(path, {label, icon, style, labelInfo});
}
},
+
+ /**
+ * @param {Object} labelInfo
+ * @return {string|undefined} The icon name, or undefined if no icon should
+ * be used.
+ */
+ _computeLabelIcon(labelInfo) {
+ if (labelInfo.approved) { return 'gr-icons:check'; }
+ if (labelInfo.rejected) { return 'gr-icons:close'; }
+ // If there is an intermediate vote (e.g. +1 on a label with max value
+ // +2), the value field will be populated.
+ if (!labelInfo.value) { return 'gr-icons:hourglass'; }
+ return undefined;
+ },
+
+ /**
+ * @param {Object} labelInfo
+ */
+ _computeLabelClass(labelInfo) {
+ const value = labelInfo.value || 0;
+ if (value > 0 || labelInfo.approved) { return 'positive'; }
+ if (value < 0 || labelInfo.rejected) { return 'negative'; }
+ return 'neutral';
+ },
+
+ _computeLabelShortcut(label) {
+ return label.split('-').reduce((a, i) => a + i[0].toUpperCase(), '');
+ },
+
+ _computeShowOptional(optionalFieldsRecord) {
+ return optionalFieldsRecord.base.length ? '' : 'hidden';
+ },
+
+ _computeLabelValue(value) {
+ return (value > 0 ? '+' : '') + value;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
index efac0c2..ff939be 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
@@ -40,19 +40,77 @@
element = fixture('basic');
});
- test('computed fields', () => {
- assert.isTrue(element._computeShowLabelStatus({status: 'NEW'}));
- assert.isFalse(element._computeShowLabelStatus({status: 'MERGED'}));
- assert.isFalse(element._computeShowLabelStatus({status: 'ABANDONED'}));
-
+ test('requirements computed fields', () => {
assert.isTrue(element._computeShowWip({work_in_progress: true}));
assert.isFalse(element._computeShowWip({work_in_progress: false}));
- assert.equal(element._computeRequirementClass(true), 'satisfied');
- assert.equal(element._computeRequirementClass(false), 'unsatisfied');
+ assert.equal(element._computeRequirementClass(true), 'positive');
+ assert.equal(element._computeRequirementClass(false), 'neutral');
assert.equal(element._computeRequirementIcon(true), 'gr-icons:check');
- assert.equal(element._computeRequirementIcon(false), 'gr-icons:hourglass');
+ assert.equal(element._computeRequirementIcon(false),
+ 'gr-icons:hourglass');
+ });
+
+ test('label computed fields', () => {
+ assert.equal(element._computeLabelIcon({approved: []}), 'gr-icons:check');
+ assert.equal(element._computeLabelIcon({rejected: []}), 'gr-icons:close');
+ assert.equal(element._computeLabelIcon({}), 'gr-icons:hourglass');
+ assert.equal(element._computeLabelIcon({value: 1}), undefined);
+
+ assert.equal(element._computeLabelClass({approved: []}), 'positive');
+ assert.equal(element._computeLabelClass({value: 1}), 'positive');
+ assert.equal(element._computeLabelClass({rejected: []}), 'negative');
+ assert.equal(element._computeLabelClass({value: -1}), 'negative');
+ assert.equal(element._computeLabelClass({}), 'neutral');
+ assert.equal(element._computeLabelClass({value: 0}), 'neutral');
+
+ assert.equal(element._computeLabelShortcut('Code-Review'), 'CR');
+ assert.equal(element._computeLabelShortcut('Verified'), 'V');
+ assert.equal(element._computeLabelShortcut('Library-Compliance'), 'LC');
+ assert.equal(element._computeLabelShortcut('PolyGerrit-Review'), 'PR');
+ assert.equal(element._computeLabelShortcut('polygerrit-review'), 'PR');
+ assert.equal(element._computeLabelShortcut(
+ 'Some-Special-Label-7'), 'SSL7');
+
+ assert.equal(element._computeLabelValue(1), '+1');
+ assert.equal(element._computeLabelValue(-1), '-1');
+ assert.equal(element._computeLabelValue(0), '0');
+ });
+
+ test('_computeLabels', () => {
+ assert.equal(element._optionalLabels.length, 0);
+ assert.equal(element._requiredLabels.length, 0);
+ element._computeLabels({base: {
+ test: {
+ all: [{_account_id: 1, name: 'bojack', value: 1}],
+ default_value: 0,
+ values: [],
+ value: 1,
+ },
+ opt_test: {
+ all: [{_account_id: 1, name: 'bojack', value: 1}],
+ default_value: 0,
+ values: [],
+ optional: true,
+ },
+ }});
+ assert.equal(element._optionalLabels.length, 1);
+ assert.equal(element._requiredLabels.length, 1);
+
+ assert.equal(element._optionalLabels[0].label, 'opt_test');
+ assert.equal(element._optionalLabels[0].icon, 'gr-icons:hourglass');
+ assert.equal(element._optionalLabels[0].style, 'neutral');
+ assert.ok(element._optionalLabels[0].labelInfo);
+ });
+
+ test('optional show/hide', () => {
+ const optionalSection = element.$$('.optional');
+ assert.isTrue(isHidden(optionalSection));
+ element._optionalLabels = [{label: 'test'}];
+ flushAsynchronousOperations();
+
+ assert.isFalse(isHidden(optionalSection));
});
test('properly converts satisfied labels', () => {
@@ -67,10 +125,10 @@
};
flushAsynchronousOperations();
- const labelName = element.$$('.satisfied .labelName');
+ const labelName = element.$$('.positive .labelName');
assert.ok(labelName);
assert.isFalse(labelName.hasAttribute('hidden'));
- assert.equal(labelName.innerHTML, 'Verified');
+ assert.equal(labelName.innerHTML, 'V');
});
test('properly converts unsatisfied labels', () => {
@@ -84,10 +142,10 @@
};
flushAsynchronousOperations();
- const labelName = element.$$('.unsatisfied .labelName');
+ const labelName = element.$$('.neutral .labelName');
assert.ok(labelName);
assert.isFalse(labelName.hasAttribute('hidden'));
- assert.equal(labelName.innerHTML, 'Verified');
+ assert.equal(labelName.innerHTML, 'V');
});
test('properly displays Work In Progress', () => {
@@ -99,13 +157,10 @@
};
flushAsynchronousOperations();
- const changeIsWip = element.$$('.changeIsWip.unsatisfied');
+ const changeIsWip = element.$$('gr-label.neutral');
assert.ok(changeIsWip);
- assert.isFalse(changeIsWip.hasAttribute('hidden'));
- assert.notEqual(changeIsWip.innerHTML.indexOf('Work in Progress'), -1);
});
-
test('properly displays a satisfied requirement', () => {
element.change = {
status: 'NEW',
@@ -117,12 +172,12 @@
};
flushAsynchronousOperations();
- const satisfiedRequirement = element.$$('.satisfied');
+ const satisfiedRequirement = element.$$('.positive');
assert.ok(satisfiedRequirement);
assert.isFalse(satisfiedRequirement.hasAttribute('hidden'));
- // Extract the content of the text node (second element, after the span)
- const textNode = satisfiedRequirement.childNodes[1].nodeValue.trim();
+ // Extract the content of the text node.
+ const textNode = satisfiedRequirement.childNodes[0].nodeValue.trim();
assert.equal(textNode, 'Resolve all comments');
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index b26d649..494bca7 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -396,10 +396,10 @@
<gr-change-metadata
id="metadata"
change="{{_change}}"
+ account="[[_account]]"
revision="[[_selectedRevision]]"
commit-info="[[_commitInfo]]"
server-config="[[_serverConfig]]"
- mutable="[[_loggedIn]]"
parent-is-current="[[_parentIsCurrent]]"
on-show-reply-dialog="_handleShowReplyDialog">
</gr-change-metadata>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 41e5227..2ec224a 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -150,10 +150,10 @@
min-width: 3.5em;
}
.added {
- color: #388E3C;
+ color: var(--vote-text-color-recommended);
}
.removed {
- color: #D32F2F;
+ color: var(--vote-text-color-disliked);
text-align: left;
}
.drafts {
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
index e9d8ce8..bd985cd 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
@@ -83,7 +83,7 @@
color: #1b5e20;
}
.submittableCheck {
- color: #388E3C;
+ color: var(--vote-text-color-recommended);
display: none;
}
.submittableCheck.submittable {
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
index 89ab8fe..543ed85 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
@@ -70,6 +70,7 @@
.transparentBackground,
gr-button.transparentBackground {
background-color: transparent;
+ padding: 0;
}
:host([disabled]) {
opacity: .6;
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
index 750ba1c..a1737b2 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
@@ -177,7 +177,9 @@
_computeShowPlaceholder(labelInfo, changeLabelsRecord) {
if (labelInfo.all) {
for (const label of labelInfo.all) {
- if (label.value) { return 'hidden'; }
+ if (label.value && label.value != labelInfo.default_value) {
+ return 'hidden';
+ }
}
}
return '';
diff --git a/polygerrit-ui/app/styles/gr-voting-styles.html b/polygerrit-ui/app/styles/gr-voting-styles.html
index 96c8026..3b1ee64 100644
--- a/polygerrit-ui/app/styles/gr-voting-styles.html
+++ b/polygerrit-ui/app/styles/gr-voting-styles.html
@@ -21,9 +21,9 @@
:host {
--vote-chip-styles: {
border: 1px solid rgba(0,0,0,.12);
- border-radius: 12px;
+ border-radius: 1em;
box-shadow: none;
- min-width: 40px;
+ min-width: 3em;
}
}
</style>
diff --git a/polygerrit-ui/app/styles/themes/app-theme.html b/polygerrit-ui/app/styles/themes/app-theme.html
index 21db329..81c195a 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.html
+++ b/polygerrit-ui/app/styles/themes/app-theme.html
@@ -76,6 +76,9 @@
--vote-color-disliked: #f7c4cb;
--vote-color-neutral: #ebf5fb;
+ --vote-text-color-recommended: #388E3C;
+ --vote-text-color-disliked: #D32F2F;
+
/* Diff colors */
--diff-selection-background-color: #c7dbf9;
--light-remove-highlight-color: #FFEBEE;
