Merge "Redesign Change Log"
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
index a7e65a3..d9786c1 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
@@ -36,18 +36,15 @@
word-wrap: break-word;
}
.file {
- border-top: 1px solid var(--border-color);
- font-weight: var(--font-weight-bold);
- margin: 10px 0 3px;
- padding: 10px 0 5px;
+ padding: var(--spacing-s) 0;
}
.container {
display: flex;
- margin: var(--spacing-m) 0;
+ padding: var(--spacing-s) 0;
}
.lineNum {
- margin-right: var(--spacing-m);
- min-width: 10em;
+ margin-right: var(--spacing-s);
+ min-width: 135px;
text-align: right;
}
.message {
@@ -66,7 +63,7 @@
}
</style>
<template is="dom-repeat" items="[[_computeFilesFromComments(comments)]]" as="file">
- <div class="file">[[computeDisplayPath(file)]]:</div>
+ <div class="file"><a class="fileLink" href="[[_computeDiffURL(file, changeNum, comments)]]">[[computeDisplayPath(file)]]</a></div>
<template is="dom-repeat"
items="[[_computeCommentsForFile(comments, file)]]" as="comment">
<div class="container">
@@ -74,7 +71,7 @@
href$="[[_computeDiffLineURL(file, changeNum, comment.patch_set, comment)]]">
<span hidden$="[[!comment.line]]">
<span>[[_computePatchDisplayName(comment)]]</span>
- Line <span>[[comment.line]]</span>:
+ Line <span>[[comment.line]]</span>
</span>
<span hidden$="[[comment.line]]">
File comment:
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
index 58bdbbc..16c55cd 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
@@ -52,18 +52,29 @@
return comment.side === 'PARENT';
}
- _computeDiffLineURL(file, changeNum, patchNum, comment) {
+ _computeDiffURL(filePath, changeNum, allComments) {
+ if ([filePath, changeNum, allComments].some(arg => arg === undefined)) {
+ return;
+ }
+ const fileComments = this._computeCommentsForFile(allComments, filePath);
+ // This can happen for files that don't exist anymore in the current ps.
+ if (fileComments.length === 0) return;
+ return Gerrit.Nav.getUrlForDiffById(changeNum, this.projectName,
+ filePath, fileComments[0].patch_set);
+ }
+
+ _computeDiffLineURL(filePath, changeNum, patchNum, comment) {
const basePatchNum = comment.hasOwnProperty('parent') ?
-comment.parent : null;
- return Gerrit.Nav.getUrlForDiffById(this.changeNum, this.projectName,
- file, patchNum, basePatchNum, comment.line,
+ return Gerrit.Nav.getUrlForDiffById(changeNum, this.projectName,
+ filePath, patchNum, basePatchNum, comment.line,
this._isOnParent(comment));
}
- _computeCommentsForFile(comments, file) {
+ _computeCommentsForFile(comments, filePath) {
// Changes are not picked up by the dom-repeat due to the array instance
// identity not changing even when it has elements added/removed from it.
- return (comments[file] || []).slice();
+ return (comments[filePath] || []).slice();
}
_computePatchDisplayName(comment) {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index fbd5d68d..5dbc1b8 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -45,44 +45,19 @@
:host(.expanded) {
cursor: auto;
}
- :host > div {
- padding: 0 var(--spacing-l);
- }
- gr-avatar {
- position: absolute;
- left: var(--spacing-l);
- }
.collapsed .contentContainer {
- align-items: baseline;
+ align-items: center;
color: var(--deemphasized-text-color);
display: flex;
white-space: nowrap;
}
.contentContainer {
- margin-left: calc(var(--spacing-l) + 2.5em);
- padding: var(--spacing-m) 0;
+ padding: var(--spacing-m) var(--spacing-l);
}
- .showAvatar.collapsed .contentContainer {
- margin-left: calc(var(--spacing-l) + 1.75em);
- }
- .hideAvatar.collapsed .contentContainer,
- .hideAvatar.expanded .contentContainer {
- margin-left: 0;
- }
- .showAvatar.collapsed .contentContainer,
- .hideAvatar.collapsed .contentContainer,
- .hideAvatar.expanded .contentContainer {
- padding: var(--spacing-m) 0;
- }
- .collapsed gr-avatar {
- top: var(--spacing-m);
- height: var(--line-height-normal);
- width: var(--line-height-normal);
- }
- .expanded gr-avatar {
- top: var(--spacing-l);
- height: var(--line-height-h1);
- width: var(--line-height-h1);
+ .collapsed .contentContainer {
+ /* For expanded state we inherit the alternating background color
+ that is set in gr-messages-list. */
+ background-color: var(--background-color-primary);
}
.name {
font-weight: var(--font-weight-bold);
@@ -129,15 +104,26 @@
color: var(--primary-text-color);
margin-right: var(--spacing-s);
}
+ .authorLabel {
+ min-width: 160px;
+ display: inline-block;
+ }
.expanded .author {
cursor: pointer;
margin-bottom: var(--spacing-s);
}
+ .expanded .content {
+ padding-left: 40px;
+ }
.dateContainer {
position: absolute;
right: var(--spacing-l);
top: 10px;
}
+ .dateContainer .patchset {
+ margin-right: var(--spacing-m);
+ color: var(--deemphasized-text-color);
+ }
span.date {
color: var(--deemphasized-text-color);
}
@@ -148,16 +134,23 @@
cursor: pointer;
vertical-align: top;
}
- .replyContainer {
- padding: var(--spacing-m) 0 0 0;
- }
.score {
- border: 1px solid rgba(0,0,0,.12);
border-radius: var(--border-radius);
color: var(--primary-text-color);
display: inline-block;
- margin: -1px 0;
- padding: 0 var(--spacing-xxs);
+ padding: 0 var(--spacing-s);
+ text-align: center;
+ }
+ .score,
+ .commentsSummary {
+ margin-right: var(--spacing-s);
+ min-width: 115px;
+ }
+ .expanded .commentsSummary {
+ display: none;
+ }
+ .commentsIcon {
+ vertical-align: top;
}
.score.negative {
background-color: var(--vote-color-disliked);
@@ -177,34 +170,39 @@
};
}
</style>
- <div class$="[[_computeClass(_expanded, showAvatar, message)]]">
- <gr-avatar account="[[author]]" image-size="100"></gr-avatar>
+ <div class$="[[_computeClass(_expanded)]]">
<div class="contentContainer">
<div class="author" on-click="_handleAuthorClick">
<span hidden$="[[!showOnBehalfOf]]">
<span class="name">[[message.real_author.name]]</span>
on behalf of
</span>
- <gr-account-label
- account="[[author]]"
- hide-avatar></gr-account-label>
+ <gr-account-label account="[[author]]" class="authorLabel"></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="[[_commentCountText]]">
+ <div class="commentsSummary">
+ <iron-icon icon="gr-icons:comment" class="commentsIcon"></iron-icon>
+ <span class="numberOfComments">[[_commentCountText]]</span>
+ </div>
+ </template>
<template is="dom-if" if="[[message.message]]">
<div class="content">
- <div class="message hideOnOpen">[[message.message]]</div>
+ <div class="message hideOnOpen">[[_messageContentCollapsed]]</div>
<gr-formatted-text
no-trailing-margin
class="message hideOnCollapsed"
- content="[[message.message]]"
+ content="[[_messageContentExpanded]]"
config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
- <div class="replyContainer" hidden$="[[!showReplyButton]]" hidden>
- <gr-button link small on-click="_handleReplyTap">Reply</gr-button>
- </div>
+ <template is="dom-if" if="[[!_isMessageContentEmpty(message.message)]]">
+ <div class="replyContainer" hidden$="[[!showReplyButton]]" hidden>
+ <gr-button link small on-click="_handleReplyTap">Reply</gr-button>
+ </div>
+ </template>
<gr-comment-list
comments="[[comments]]"
change-num="[[changeNum]]"
@@ -228,6 +226,9 @@
</div>
</template>
<span class="dateContainer">
+ <template is="dom-if" if="[[message._revision_number]]">
+ <span class="patchset">Patchset [[message._revision_number]]</span>
+ </template>
<template is="dom-if" if="[[!message.id]]">
<span class="date">
<gr-date-formatter
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 59f12f5..95109bb 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -69,10 +69,6 @@
type: Boolean,
computed: '_computeIsAutomated(message)',
},
- showAvatar: {
- type: Boolean,
- computed: '_computeShowAvatar(author, config)',
- },
showOnBehalfOf: {
type: Boolean,
computed: '_computeShowOnBehalfOf(message)',
@@ -101,6 +97,18 @@
type: Object,
computed: '_computeExpanded(message.expanded)',
},
+ _messageContentExpanded: {
+ type: String,
+ computed: '_computeMessageContentExpanded(message.message)',
+ },
+ _messageContentCollapsed: {
+ type: String,
+ computed: '_computeMessageContentCollapsed(message.message)',
+ },
+ _commentCountText: {
+ type: Number,
+ computed: '_computeCommentCountText(comments)',
+ },
_loggedIn: {
type: Boolean,
value: false,
@@ -140,12 +148,51 @@
}
}
- _computeAuthor(message) {
- return message.author || message.updated_by;
+ _computeCommentCountText(comments) {
+ if (!comments) return undefined;
+ let count = 0;
+ for (const file in comments) {
+ if (comments.hasOwnProperty(file)) {
+ const commentArray = comments[file] || [];
+ count += commentArray.length;
+ }
+ }
+ if (count === 0) {
+ return undefined;
+ } else if (count === 1) {
+ return '1 comment';
+ } else {
+ return `${count} comments`;
+ }
}
- _computeShowAvatar(author, config) {
- return !!(author && config && config.plugin && config.plugin.has_avatars);
+ _computeMessageContentExpanded(content) {
+ return this._computeMessageContent(content, true);
+ }
+
+ _computeMessageContentCollapsed(content) {
+ return this._computeMessageContent(content, false);
+ }
+
+ _computeMessageContent(content, isExpanded) {
+ if (!content) return '';
+ const lines = content.split('\n');
+ const filteredLines = lines.filter(line => {
+ if (!isExpanded && line.startsWith('>')) return false;
+ if (line.startsWith('Patch Set ')) return false;
+ if (line.startsWith('(') && line.endsWith(' comment)')) return false;
+ if (line.startsWith('(') && line.endsWith(' comments)')) return false;
+ return true;
+ });
+ return filteredLines.join('\n').trim();
+ }
+
+ _isMessageContentEmpty(content) {
+ return this._computeMessageContent(content).trim().length === 0;
+ }
+
+ _computeAuthor(message) {
+ return message.author || message.updated_by;
}
_computeShowOnBehalfOf(message) {
@@ -236,10 +283,9 @@
return classes.join(' ');
}
- _computeClass(expanded, showAvatar, message) {
+ _computeClass(expanded) {
const classes = [];
classes.push(expanded ? 'expanded' : 'collapsed');
- classes.push(showAvatar ? 'showAvatar' : 'hideAvatar');
return classes.join(' ');
}
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
index d71beb4..8d76510 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
@@ -64,6 +64,12 @@
align-items: center;
display: flex;
}
+ gr-message:nth-child(2n) {
+ background-color: var(--background-color-secondary);
+ }
+ gr-message:nth-child(2n+1) {
+ background-color: var(--background-color-tertiary);
+ }
</style>
<div class="header">
<span
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
index 5ecae00..75ab77a 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -54,6 +54,8 @@
<g id="info"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z"></path></g>
<!-- This SVG is a copy from material.io https://material.io/icons/#ic_hourglass_full-->
<g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"/><path d="M0 0h24v24H0V0z" fill="none"/></g>
+ <!-- This SVG is a copy from material.io https://material.io/icons/#mode_comment-->
+ <g id="comment"><path d="M21.99 4c0-1.1-.89-2-1.99-2H4c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h14l4 4-.01-18z"/><path d="M0 0h24v24H0z" fill="none"/></g>
<!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
<g id="error"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-2h2v2zm0-4h-2V7h2v6z"></path></g>
<!-- This is a custom PolyGerrit SVG -->
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.js
index 1c62ca3..d599563 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.js
@@ -147,15 +147,16 @@
*/
GrReviewerUpdatesParser.prototype._getUpdateMessage = function(prev, state) {
if (prev === 'REMOVED' || !prev) {
- return 'added to ' + state + ': ';
+ return 'Added to ' + state.toLowerCase() + ': ';
} else if (state === 'REMOVED') {
if (prev) {
- return 'removed from ' + prev + ': ';
+ return 'Removed from ' + prev.toLowerCase() + ': ';
} else {
- return 'removed : ';
+ return 'Removed : ';
}
} else {
- return 'moved from ' + prev + ' to ' + state + ': ';
+ return 'Moved from ' + prev.toLowerCase() + ' to ' + state.toLowerCase() +
+ ': ';
}
};
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.html
index 818bfa9..593fcec 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser_test.html
@@ -239,15 +239,15 @@
assert.equal(change.reviewer_updates[1].updates.length, 3);
let items = change.reviewer_updates[0].updates;
- assert.equal(items[0].message, 'added to CC: ');
+ assert.equal(items[0].message, 'Added to cc: ');
assert.deepEqual(items[0].reviewers, [reviewer1, reviewer2]);
items = change.reviewer_updates[1].updates;
- assert.equal(items[0].message, 'moved from CC to REVIEWER: ');
+ assert.equal(items[0].message, 'Moved from cc to reviewer: ');
assert.deepEqual(items[0].reviewers, [reviewer1]);
- assert.equal(items[1].message, 'removed from REVIEWER: ');
+ assert.equal(items[1].message, 'Removed from reviewer: ');
assert.deepEqual(items[1].reviewers, [reviewer1]);
- assert.equal(items[2].message, 'added to REVIEWER: ');
+ assert.equal(items[2].message, 'Added to reviewer: ');
assert.deepEqual(items[2].reviewers, [reviewer1, reviewer2]);
});
