opendev/gerrit
Code Issues Proposed changes

Show colorful vote chips in messages rather than the red/green header

Browse Source 
Feature: Issue 8528
Change-Id: I8795cfe110bbaf95dd747268241211cb84c12859
This commit is contained in:
Wyatt Allen
2018-02-28 16:56:18 -08:00
parent 62a9ed0c00
commit 5a96988908
7 changed files with 138 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -529,6 +529,7 @@ limitations under the License.
<gr-messages-list id="messageList"
class="hideOnMobileOverlay"
change-num="[[_changeNum]]"
labels="[[_change.labels]]"
messages="[[_change.messages]]"
reviewer-updates="[[_change.reviewer_updates]]"
change-comments="[[_changeComments]]"

29
polygerrit-ui/app/elements/change/gr-message/gr-message.html
View File

@@ -21,11 +21,13 @@ limitations under the License.
<link rel="import" href="../../shared/gr-formatted-text/gr-formatted-text.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../../styles/gr-voting-styles.html">
<link rel="import" href="../gr-comment-list/gr-comment-list.html">
<dom-module id="gr-message">
<template>
<style include="gr-voting-styles"></style>
<style include="shared-styles">
:host {
border-bottom: 1px solid #ddd;
@@ -131,11 +133,25 @@ limitations under the License.
.replyContainer {
padding: .5em 0 0 0;
}
.positiveVote {
box-shadow: inset 0 3.8em #d4ffd4;
.score {
border: 1px solid rgba(0,0,0,.12);
border-radius: 3px;
color: #000;
display: inline-block;
margin: -.1em 0;
padding: 0 .1em;
}
.negativeVote {
box-shadow: inset 0 3.8em #ffd4d4;
.score.negative {
background-color: var(--vote-color-negative);
}
.score.negative.min {
background-color: var(--vote-color-min);
}
.score.positive {
background-color: var(--vote-color-positive);
}
.score.positive.max {
background-color: var(--vote-color-max);
}
gr-account-label {
--gr-account-label-text-style: {
@@ -154,6 +170,11 @@ limitations under the License.
<gr-account-label
account="[[author]]"
hide-avatar></gr-account-label>
<template is="dom-repeat" items="[[_getScores(message)]]" as="score">
<span class$="score [[_computeScoreClass(score, labelExtremes)]]">
[[score.label]] [[score.value]]
</span>
</template>
</div>
<template is="dom-if" if="[[message.message]]">
<div class="content">

55
polygerrit-ui/app/elements/change/gr-message/gr-message.js
View File

@@ -14,7 +14,6 @@
(function() {
'use strict';
const CI_LABELS = ['Trybot-Ready', 'Tryjob-Request', 'Commit-Queue'];
const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+: /;
const LABEL_TITLE_SCORE_PATTERN = /^([A-Za-z0-9-]+)([+-]\d+)$/;
@@ -79,6 +78,13 @@
type: String,
observer: '_projectNameChanged',
},
/**
* A mapping from label names to objects representing the minimum and
* maximum possible values for that label.
*/
labelExtremes: Object,
/**
* @type {{ commentlinks: Array }}
*/
@@ -175,41 +181,42 @@
return event.type === 'REVIEWER_UPDATE';
},
_isMessagePositive(message) {
if (!message.message) { return null; }
_getScores(message) {
if (!message.message) { return []; }
const line = message.message.split('\n', 1)[0];
const patchSetPrefix = PATCH_SET_PREFIX_PATTERN;
if (!line.match(patchSetPrefix)) { return null; }
if (!line.match(patchSetPrefix)) { return []; }
const scoresRaw = line.split(patchSetPrefix)[1];
if (!scoresRaw) { return null; }
const scores = scoresRaw.split(' ');
if (!scores.length) { return null; }
const {min, max} = scores
if (!scoresRaw) { return []; }
return scoresRaw.split(' ')
.map(s => s.match(LABEL_TITLE_SCORE_PATTERN))
.filter(ms => ms && ms.length === 3)
.filter(([, label]) => !CI_LABELS.includes(label))
.map(([, , score]) => score)
.map(s => parseInt(s, 10))
.reduce(({min, max}, s) =>
({min: (s < min ? s : min), max: (s > max ? s : max)}),
{min: 0, max: 0});
if (max - min === 0) {
return 0;
} else {
return (max + min) > 0 ? 1 : -1;
.map(ms => ({label: ms[1], value: ms[2]}));
},
_computeScoreClass(score, labelExtremes) {
const classes = [];
if (score.value > 0) {
classes.push('positive');
} else if (score.value < 0) {
classes.push('negative');
}
const extremes = labelExtremes[score.label];
if (extremes) {
const intScore = parseInt(score.value, 10);
if (intScore === extremes.max) {
classes.push('max');
} else if (intScore === extremes.min) {
classes.push('min');
}
}
return classes.join(' ');
},
_computeClass(expanded, showAvatar, message) {
const classes = [];
classes.push(expanded ? 'expanded' : 'collapsed');
classes.push(showAvatar ? 'showAvatar' : 'hideAvatar');
const scoreQuality = this._isMessagePositive(message);
if (scoreQuality === 1) {
classes.push('positiveVote');
} else if (scoreQuality === -1) {
classes.push('negativeVote');
}
return classes.join(' ');
},

31
polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
View File

@@ -163,22 +163,29 @@ limitations under the License.
});
});
test('negative vote', () => {
test('votes', () => {
element.message = {
author: {},
expanded: false,
message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Ready+1',
};
assert.isOk(Polymer.dom(element.root).querySelector('.negativeVote'));
});
test('positive vote', () => {
element.message = {
author: {},
expanded: false,
message: 'Patch Set 1: Verified-1 Code-Review+2 Trybot-Ready-1',
element.labelExtremes = {
'Verified': {max: 1, min: -1},
'Code-Review': {max: 2, min: -2},
'Trybot-Ready': {max: 3, min: 0},
};
assert.isOk(Polymer.dom(element.root).querySelector('.positiveVote'));
flushAsynchronousOperations();
const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
assert.equal(scoreChips.length, 3);
assert.isTrue(scoreChips[0].classList.contains('positive'));
assert.isTrue(scoreChips[0].classList.contains('max'));
assert.isTrue(scoreChips[1].classList.contains('negative'));
assert.isTrue(scoreChips[1].classList.contains('min'));
assert.isTrue(scoreChips[2].classList.contains('positive'));
assert.isFalse(scoreChips[2].classList.contains('min'));
});
test('false negative vote', () => {
@@ -187,7 +194,9 @@ limitations under the License.
expanded: false,
message: 'Patch Set 1: Cherry Picked from branch stable-2.14.',
};
assert.isNotOk(Polymer.dom(element.root).querySelector('.negativeVote'));
element.labelExtremes = {};
const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
assert.equal(scoreChips.length, 0);
});
});
</script>

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

@@ -111,6 +111,7 @@ limitations under the License.
project-name="[[projectName]]"
show-reply-button="[[showReplyButtons]]"
on-scroll-to="_handleScrollTo"
label-extremes="[[_labelExtremes]]"
data-message-id$="[[message.id]]"></gr-message>
</template>
<gr-reporting id="reporting" category="message-list"></gr-reporting>

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

@@ -41,6 +41,7 @@
type: Boolean,
value: false,
},
labels: Object,
_expanded: {
type: Boolean,
@@ -66,6 +67,11 @@
type: Array,
value() { return []; },
},
_labelExtremes: {
type: Object,
computed: '_computeLabelExtremes(labels.*)',
},
},
scrollToMessage(messageID) {
@@ -331,5 +337,24 @@
this._numRemaining(visibleMessages, messages, hideAutomated);
return total <= this._getDelta(visibleMessages, messages, hideAutomated);
},
/**
* Compute a mapping from label name to objects representing the minimum and
* maximum possible values for that label.
*/
_computeLabelExtremes(labelRecord) {
const extremes = {};
const labels = labelRecord.base;
if (!labels) { return extremes; }
for (const key of Object.keys(labels)) {
if (!labels[key] || !labels[key].values) { continue; }
const values = Object.keys(labels[key].values)
.map(v => parseInt(v, 10));
values.sort();
if (!values.length) { continue; }
extremes[key] = {min: values[0], max: values[values.length - 1]};
}
return extremes;
},
});
})();

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

@@ -540,5 +540,40 @@ limitations under the License.
assert.equal(messages.length, 5);
assert.isFalse(element._hasAutomatedMessages(messages));
});
test('_computeLabelExtremes', () => {
const computeSpy = sandbox.spy(element, '_computeLabelExtremes');
element.labels = null;
assert.isTrue(computeSpy.calledOnce);
assert.deepEqual(computeSpy.lastCall.returnValue, {});
element.labels = {};
assert.isTrue(computeSpy.calledTwice);
assert.deepEqual(computeSpy.lastCall.returnValue, {});
element.labels = {'my-label': {}};
assert.isTrue(computeSpy.calledThrice);
assert.deepEqual(computeSpy.lastCall.returnValue, {});
element.labels = {'my-label': {values: {}}};
assert.equal(computeSpy.callCount, 4);
assert.deepEqual(computeSpy.lastCall.returnValue, {});
element.labels = {'my-label': {values: {'-12': {}}}};
assert.equal(computeSpy.callCount, 5);
assert.deepEqual(computeSpy.lastCall.returnValue,
{'my-label': {min: -12, max: -12}});
element.labels = {
'my-label': {values: {'-12': {}}},
'other-label': {values: {'-1': {}, ' 0': {}, '+1': {}}},
};
assert.equal(computeSpy.callCount, 6);
assert.deepEqual(computeSpy.lastCall.returnValue, {
'my-label': {min: -12, max: -12},
'other-label': {min: -1, max: 1},
});
});
});
</script>