Emphasize the reply button when there are drafts
Bug: Issue 3844
Change-Id: I85cb4ad343f2a27598b8f84559cc8330e997e7a3
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 f143fc2..a394b2b 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
@@ -21,9 +21,10 @@
<link rel="import" href="../../shared/gr-ajax/gr-ajax.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-change-star/gr-change-star.html">
-<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
<link rel="import" href="../../shared/gr-linked-text/gr-linked-text.html">
+<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-change-actions/gr-change-actions.html">
<link rel="import" href="../gr-change-metadata/gr-change-metadata.html">
@@ -79,7 +80,7 @@
}
.download,
.patchSelectLabel {
- margin-left: var(--default-horizontal-margin);
+ margin-left: 1em;
}
.header select {
margin-left: .5em;
@@ -242,8 +243,12 @@
<span class="changeStatus">[[_computeChangeStatus(_change, _patchNum)]]</span>
</span>
<span class="header-actions">
- <gr-button class="reply" hidden$="[[!_loggedIn]]" hidden on-tap="_handleReplyTap">Reply</gr-button>
- <gr-button link class="download" on-tap="_handleDownloadTap">Download</gr-button>
+ <gr-button hidden
+ class="reply"
+ primary$="[[_computeReplyButtonHighlighted(_diffDrafts)]]"
+ hidden$="[[!_loggedIn]]"
+ on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
+ <gr-button class="download" on-tap="_handleDownloadTap">Download</gr-button>
<span>
<label class="patchSelectLabel" for="patchSetSelect">Patch set</label>
<select id="patchSetSelect" on-change="_handlePatchChange">
@@ -290,6 +295,7 @@
change-num="[[_changeNum]]"
patch-num="[[_patchNum]]"
comments="[[_comments]]"
+ drafts="[[_diffDrafts]]"
selected-index="{{viewState.selectedFileIndex}}"></gr-file-list>
<gr-messages-list id="messageList"
change-num="[[_changeNum]]"
@@ -314,10 +320,12 @@
patch-num="[[_patchNum]]"
labels="[[_change.labels]]"
permitted-labels="[[_change.permitted_labels]]"
+ diff-drafts="[[_diffDrafts]]"
on-send="_handleReplySent"
on-cancel="_handleReplyCancel"
hidden$="[[!_loggedIn]]">Reply</gr-reply-dialog>
</gr-overlay>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
<script src="gr-change-view.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index f784f0a..9af6aa9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -49,6 +49,7 @@
},
_commitInfo: Object,
_changeNum: String,
+ _diffDrafts: Object,
_patchNum: String,
_allPatchSets: {
type: Array,
@@ -66,6 +67,11 @@
type: Function,
value: function() { return this._handleBodyScroll.bind(this); },
},
+ _replyButtonLabel: {
+ type: String,
+ value: 'Reply',
+ computed: '_computeReplyButtonLabel(_diffDrafts)',
+ },
},
behaviors: [
@@ -74,13 +80,14 @@
],
ready: function() {
- app.accountReady.then(function() {
- this._loggedIn = app.loggedIn;
- }.bind(this));
this._headerEl = this.$$('.header');
},
attached: function() {
+ this._getLoggedIn().then(function(loggedIn) {
+ this._loggedIn = loggedIn;
+ }.bind(this));
+
window.addEventListener('scroll', this._boundScrollHandler);
},
@@ -145,9 +152,6 @@
},
_handleReplyOverlayOpen: function(e) {
- this.$.replyDialog.reload().then(function() {
- this.async(function() { this.$.replyOverlay.center() }, 1);
- }.bind(this));
this.$.replyDialog.focus();
},
@@ -186,8 +190,8 @@
}
}.bind(this), 1);
- app.accountReady.then(function() {
- if (!this._loggedIn) { return; }
+ this._getLoggedIn().then(function(loggedIn) {
+ if (!loggedIn) { return; }
if (this.viewState.showReplyDialog) {
this.$.replyOverlay.open();
@@ -301,6 +305,23 @@
return result;
},
+ _computeReplyButtonHighlighted: function(drafts) {
+ return Object.keys(drafts || {}).length > 0;
+ },
+
+ _computeReplyButtonLabel: function(drafts) {
+ drafts = drafts || {};
+ var draftCount = Object.keys(drafts).reduce(function(count, file) {
+ return count + drafts[file].length;
+ }, 0);
+
+ var label = 'Reply';
+ if (draftCount > 0) {
+ label += ' (' + draftCount + ')';
+ }
+ return label;
+ },
+
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
@@ -322,9 +343,34 @@
page.show(this.changePath(this._changeNum));
},
+ _getDiffDrafts: function() {
+ return this.$.restAPI.getDiffDrafts(this._changeNum).then(
+ function(drafts) { return this._diffDrafts = drafts; }.bind(this));
+ },
+
+ _getLoggedIn: function() {
+ return this.$.restAPI.getLoggedIn();
+ },
+
+ _reloadDiffDrafts: function() {
+ this._diffDrafts = {};
+ this._getDiffDrafts().then(function() {
+ if (this.$.replyOverlay.opened) {
+ this.async(function() { this.$.replyOverlay.center(); }, 1);
+ }
+ }.bind(this));
+ },
+
_reload: function() {
+ this._getLoggedIn().then(function(loggedIn) {
+ if (!loggedIn) { return; }
+
+ this._reloadDiffDrafts();
+ }.bind(this));
+
var detailCompletes = this.$.detailXHR.generateRequest().completes;
this.$.commentsXHR.generateRequest();
+
var reloadPatchNumDependentResources = function() {
return Promise.all([
this.$.commitInfoXHR.generateRequest().completes,
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index ef9fdb6..af14e06 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -79,6 +79,25 @@
assert.isFalse(overlayEl.opened);
});
+ test('reply button is highlighted when there are drafts', function() {
+ var replyButton = element.$$('gr-button.reply');
+ assert.ok(replyButton);
+ assert.isFalse(replyButton.hasAttribute('primary'));
+
+ element._diffDrafts = null;
+ assert.isFalse(replyButton.hasAttribute('primary'));
+
+ element._diffDrafts = {};
+ assert.isFalse(replyButton.hasAttribute('primary'));
+
+ element._diffDrafts = {
+ 'file1.txt': [{}],
+ 'file2.txt': [{}, {}],
+ };
+ assert.isTrue(replyButton.hasAttribute('primary'));
+ assert.equal(replyButton.textContent, 'Reply (3)');
+ });
+
test('patch num change', function(done) {
element._changeNum = '42';
element._patchNum = 2;
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 e010468..7d69d7e 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
@@ -115,9 +115,6 @@
<gr-ajax id="filesXHR"
url="[[_computeFilesURL(changeNum, patchNum)]]"
on-response="_handleResponse"></gr-ajax>
- <gr-ajax id="draftsXHR"
- url="[[_computeDraftsURL(changeNum, patchNum)]]"
- last-response="{{_drafts}}"></gr-ajax>
<gr-ajax id="reviewedXHR"
url="[[_computeReviewedURL(changeNum, patchNum)]]"
last-response="{{_reviewed}}"></gr-ajax>
@@ -145,7 +142,7 @@
[[_computeFileDisplayName(file.__path)]]
</a>
<div class="comments">
- <span class="drafts">[[_computeDraftsString(_drafts, file.__path)]]</span>
+ <span class="drafts">[[_computeDraftsString(drafts, patchNum, file.__path)]]</span>
[[_computeCommentsString(comments, patchNum, file.__path)]]
</div>
<div class$="[[_computeClass('stats', file.__path)]]">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 3c66289..ead5d74 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -23,6 +23,7 @@
patchNum: String,
changeNum: String,
comments: Object,
+ drafts: Object,
files: Array,
selectedIndex: {
type: Number,
@@ -32,12 +33,10 @@
type: Object,
value: function() { return document.body; },
},
-
_loggedIn: {
type: Boolean,
value: false,
},
- _drafts: Object,
_reviewed: {
type: Array,
value: function() { return []; },
@@ -62,7 +61,6 @@
app.accountReady.then(function() {
this._loggedIn = app.loggedIn;
if (!app.loggedIn) { return; }
- this.$.draftsXHR.generateRequest();
this._reviewedRequestPromise =
this.$.reviewedXHR.generateRequest().completes;
}.bind(this)),
@@ -74,13 +72,22 @@
},
_computeCommentsString: function(comments, patchNum, path) {
+ return this._computeCountString(comments, patchNum, path, 'comment');
+ },
+
+ _computeDraftsString: function(drafts, patchNum, path) {
+ return this._computeCountString(drafts, patchNum, path, 'draft');
+ },
+
+ _computeCountString: function(comments, patchNum, path, noun) {
+ if (!comments) { return ''; }
+
var patchComments = (comments[path] || []).filter(function(c) {
return c.patch_set == patchNum;
});
var num = patchComments.length;
if (num == 0) { return ''; }
- if (num == 1) { return '1 comment'; }
- if (num > 1) { return num + ' comments'; }
+ return num + ' ' + noun + (num > 1 ? 's' : '');
},
_computeReviewedURL: function(changeNum, patchNum) {
@@ -111,17 +118,6 @@
}.bind(this));
},
- _computeDraftsURL: function(changeNum, patchNum) {
- return this.changeBaseURL(changeNum, patchNum) + '/drafts';
- },
-
- _computeDraftsString: function(drafts, path) {
- var num = (drafts[path] || []).length;
- if (num == 0) { return ''; }
- if (num == 1) { return '1 draft'; }
- if (num > 1) { return num + ' drafts'; }
- },
-
_handleResponse: function(e, req) {
var result = e.detail.response;
var paths = Object.keys(result).sort();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index bc7b2a6..48e8cc7 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -248,24 +248,24 @@
],
};
assert.equal(
- element._computeCommentsString(comments, '1', '/COMMIT_MSG'),
+ element._computeCountString(comments, '1', '/COMMIT_MSG', 'comment'),
'2 comments');
assert.equal(
- element._computeCommentsString(comments, '1', 'myfile.txt'),
+ element._computeCountString(comments, '1', 'myfile.txt', 'comment'),
'1 comment');
assert.equal(
- element._computeCommentsString(comments, '1',
- 'file_added_in_rev2.txt'),
+ element._computeCountString(comments, '1',
+ 'file_added_in_rev2.txt', 'comment'),
'');
assert.equal(
- element._computeCommentsString(comments, '2', '/COMMIT_MSG'),
+ element._computeCountString(comments, '2', '/COMMIT_MSG', 'comment'),
'1 comment');
assert.equal(
- element._computeCommentsString(comments, '2', 'myfile.txt'),
+ element._computeCountString(comments, '2', 'myfile.txt', 'comment'),
'2 comments');
assert.equal(
- element._computeCommentsString(comments, '2',
- 'file_added_in_rev2.txt'),
+ element._computeCountString(comments, '2',
+ 'file_added_in_rev2.txt', 'comment'),
'');
});
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index a2c7e19..1a2fbab 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -18,7 +18,6 @@
<link rel="import" href="../../../bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
<link rel="import" href="../../../bower_components/iron-selector/iron-selector.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
-<link rel="import" href="../../shared/gr-ajax/gr-ajax.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-request/gr-request.html">
@@ -101,9 +100,6 @@
}
</style>
<template>
- <gr-ajax id="draftsXHR"
- url="[[_computeDraftsURL(changeNum)]]"
- last-response="{{_drafts}}"></gr-ajax>
<div class="container">
<section class="textareaContainer">
<iron-autogrow-textarea
@@ -131,10 +127,10 @@
</div>
</template>
</section>
- <section class="draftsContainer" hidden$="[[_computeHideDraftList(_drafts)]]">
- <h3>[[_computeDraftsTitle(_drafts)]]</h3>
+ <section class="draftsContainer" hidden$="[[_computeHideDraftList(diffDrafts)]]">
+ <h3>[[_computeDraftsTitle(diffDrafts)]]</h3>
<gr-comment-list
- comments="[[_drafts]]"
+ comments="[[diffDrafts]]"
change-num="[[changeNum]]"
patch-num="[[patchNum]]"></gr-comment-list>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 7b4b17a..6ec4d87 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -41,11 +41,11 @@
type: String,
value: '',
},
+ diffDrafts: Object,
labels: Object,
permittedLabels: Object,
_account: Object,
- _drafts: Object,
_xhrPromise: Object, // Used for testing.
},
@@ -59,20 +59,12 @@
}.bind(this));
},
- reload: function() {
- return this.$.draftsXHR.generateRequest().completes;
- },
-
focus: function() {
this.async(function() {
this.$.textarea.textarea.focus();
}.bind(this));
},
- _computeDraftsURL: function(changeNum) {
- return '/changes/' + changeNum + '/drafts';
- },
-
_computeHideDraftList: function(drafts) {
return Object.keys(drafts || {}).length == 0;
},
@@ -125,7 +117,6 @@
_cancelTapHandler: function(e) {
e.preventDefault();
- this._drafts = null;
this.fire('cancel', null, {bubbles: false});
},
@@ -149,7 +140,6 @@
this.fire('send', null, {bubbles: false});
this.draft = '';
this.disabled = false;
- this._drafts = null;
}.bind(this)).catch(function(err) {
alert('Oops. Something went wrong. Check the console and bug the ' +
'PolyGerrit team for assistance.');
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 27cd989..72570d9 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -98,6 +98,12 @@
return this._fetchSharedCacheURL('/accounts/self/detail');
},
+ getLoggedIn: function() {
+ return this.getAccount().then(function(account) {
+ return account != null;
+ });
+ },
+
getPreferences: function() {
return this._fetchSharedCacheURL('/accounts/self/preferences');
},
@@ -143,38 +149,46 @@
encodeURIComponent(path) + '/diff';
},
- getDiffComments: function(changeNum, basePatchNum, patchNum, path) {
- return this._getDiffComments(changeNum, basePatchNum, patchNum, path,
- '/comments');
+ getDiffComments: function(changeNum, opt_basePatchNum, opt_patchNum,
+ opt_path) {
+ return this._getDiffComments(changeNum, '/comments', opt_basePatchNum,
+ opt_patchNum, opt_path);
},
- getDiffDrafts: function(changeNum, basePatchNum, patchNum, path) {
- return this._getDiffComments(changeNum, basePatchNum, patchNum, path,
- '/drafts');
+ getDiffDrafts: function(changeNum, opt_basePatchNum, opt_patchNum,
+ opt_path) {
+ return this._getDiffComments(changeNum, '/drafts', opt_basePatchNum,
+ opt_patchNum, opt_path);
},
- _getDiffComments: function(changeNum, basePatchNum, patchNum, path,
- endpoint) {
+ _getDiffComments: function(changeNum, endpoint, opt_basePatchNum,
+ opt_patchNum, opt_path) {
+ if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
+ return this.fetchJSON(
+ this._getDiffCommentsFetchURL(changeNum, null, '/drafts'));
+ }
+
function onlyParent(c) { return c.side == PARENT_PATCH_NUM; }
function withoutParent(c) { return c.side != PARENT_PATCH_NUM; }
var promises = [];
var comments;
var baseComments;
- var url = this._getDiffCommentsFetchURL(changeNum, patchNum, endpoint);
+ var url =
+ this._getDiffCommentsFetchURL(changeNum, endpoint, opt_patchNum);
promises.push(this.fetchJSON(url).then(function(response) {
- comments = response[path] || [];
- if (basePatchNum == PARENT_PATCH_NUM) {
+ comments = response[opt_path] || [];
+ if (opt_basePatchNum == PARENT_PATCH_NUM) {
baseComments = comments.filter(onlyParent);
}
comments = comments.filter(withoutParent);
}.bind(this)));
- if (basePatchNum != PARENT_PATCH_NUM) {
- var baseURL = this._getDiffCommentsFetchURL(changeNum, basePatchNum,
- endpoint);
+ if (opt_basePatchNum != PARENT_PATCH_NUM) {
+ var baseURL = this._getDiffCommentsFetchURL(changeNum, endpoint,
+ opt_basePatchNum);
promises.push(this.fetchJSON(baseURL).then(function(response) {
- baseComments = (response[path] || []).filter(withoutParent);
+ baseComments = (response[opt_path] || []).filter(withoutParent);
}));
}
@@ -186,14 +200,14 @@
});
},
- _getDiffCommentsFetchURL: function(changeNum, patchNum, endpoint) {
- return this._changeBaseURL(changeNum, patchNum) + endpoint;
+ _getDiffCommentsFetchURL: function(changeNum, endpoint, opt_patchNum) {
+ return this._changeBaseURL(changeNum, opt_patchNum) + endpoint;
},
- _changeBaseURL: function(changeNum, patchNum) {
+ _changeBaseURL: function(changeNum, opt_patchNum) {
var v = '/changes/' + changeNum;
- if (patchNum) {
- v += '/revisions/' + patchNum;
+ if (opt_patchNum) {
+ v += '/revisions/' + opt_patchNum;
}
return v;
},
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 3902707f..44e49cb 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -129,7 +129,7 @@
],
});
});
- element._getDiffComments('42', 'PARENT', 1, 'sieve.go', '').then(
+ element._getDiffComments('42', '', 'PARENT', 1, 'sieve.go').then(
function(obj) {
assert.equal(obj.baseComments.length, 1);
assert.deepEqual(obj.baseComments[0], {
@@ -178,7 +178,7 @@
});
}
});
- element._getDiffComments('42', 1, 2, 'sieve.go', '').then(
+ element._getDiffComments('42', '', 1, 2, 'sieve.go').then(
function(obj) {
assert.equal(obj.baseComments.length, 1);
assert.deepEqual(obj.baseComments[0], {
