Update reply counter when adding drafts to inline diffs
Also do a small cleanup of _boundScrollHandler in favor of
Polymer.{listen|unlisten}
Bug: Issue 4119
Change-Id: I16e0f74dd3f480f99b837cdf6664603a94dcc06f
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 9a7537b..3175517 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
@@ -225,7 +225,7 @@
<span class="header-actions">
<gr-button hidden
class="reply"
- primary$="[[_computeReplyButtonHighlighted(_diffDrafts)]]"
+ primary$="[[_computeReplyButtonHighlighted(_diffDrafts.*)]]"
hidden$="[[!_loggedIn]]"
on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
<gr-button class="download" on-tap="_handleDownloadTap">Download</gr-button>
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 428d067..bc57dd9 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
@@ -55,7 +55,10 @@
},
_commitInfo: Object,
_changeNum: String,
- _diffDrafts: Object,
+ _diffDrafts: {
+ type: Object,
+ value: function() { return {}; },
+ },
_patchRange: Object,
_allPatchSets: {
type: Array,
@@ -69,14 +72,10 @@
_headerContainerEl: Object,
_headerEl: Object,
_projectConfig: Object,
- _boundScrollHandler: {
- type: Function,
- value: function() { return this._handleBodyScroll.bind(this); },
- },
_replyButtonLabel: {
type: String,
value: 'Reply',
- computed: '_computeReplyButtonLabel(_diffDrafts)',
+ computed: '_computeReplyButtonLabel(_diffDrafts.*)',
},
},
@@ -94,11 +93,14 @@
this._loggedIn = loggedIn;
}.bind(this));
- window.addEventListener('scroll', this._boundScrollHandler);
+ this.addEventListener('comment-save', this._handleCommentSave.bind(this));
+ this.addEventListener('comment-discard',
+ this._handleCommentDiscard.bind(this));
+ this.listen(window, 'scroll', '_handleBodyScroll');
},
detached: function() {
- window.removeEventListener('scroll', this._boundScrollHandler);
+ this.unlisten(window, 'scroll', '_handleBodyScroll');
},
_handleBodyScroll: function(e) {
@@ -124,6 +126,67 @@
el.classList.remove('pinned');
},
+ _handleCommentSave: function(e) {
+ if (!e.target.comment.__draft) { return; }
+
+ var draft = e.target.comment;
+ draft.patch_set = draft.patch_set || this._patchRange.patchNum;
+
+ // The use of path-based notification helpers (set, push) can’t be used
+ // because the paths could contain dots in them. A new object must be
+ // created to satisfy Polymer’s dirty checking.
+ // https://github.com/Polymer/polymer/issues/3127
+ // TODO(andybons): Polyfill for Object.assign in IE.
+ var diffDrafts = Object.assign({}, this._diffDrafts);
+ if (!diffDrafts[draft.path]) {
+ diffDrafts[draft.path] = [draft];
+ this._diffDrafts = diffDrafts;
+ return;
+ }
+ for (var i = 0; i < this._diffDrafts[draft.path].length; i++) {
+ if (this._diffDrafts[draft.path][i].id === draft.id) {
+ diffDrafts[draft.path][i] = draft;
+ this._diffDrafts = diffDrafts;
+ return;
+ }
+ }
+ diffDrafts[draft.path].push(draft);
+ this._diffDrafts = diffDrafts;
+ },
+
+ _handleCommentDiscard: function(e) {
+ if (!e.target.comment.__draft) { return; }
+
+ var draft = e.target.comment;
+ if (!this._diffDrafts[draft.path]) {
+ return;
+ }
+ var index = -1;
+ for (var i = 0; i < this._diffDrafts[draft.path].length; i++) {
+ if (this._diffDrafts[draft.path][i].id === draft.id) {
+ index = i;
+ break;
+ }
+ }
+ if (index === -1) {
+ throw Error('Unable to find draft with id ' + draft.id);
+ }
+
+ draft.patch_set = draft.patch_set || this._patchRange.patchNum;
+
+ // The use of path-based notification helpers (set, push) can’t be used
+ // because the paths could contain dots in them. A new object must be
+ // created to satisfy Polymer’s dirty checking.
+ // https://github.com/Polymer/polymer/issues/3127
+ // TODO(andybons): Polyfill for Object.assign in IE.
+ var diffDrafts = Object.assign({}, this._diffDrafts);
+ diffDrafts[draft.path].splice(index, 1);
+ if (diffDrafts[draft.path].length === 0) {
+ delete diffDrafts[draft.path];
+ }
+ this._diffDrafts = diffDrafts;
+ },
+
_handlePatchChange: function(e) {
var patchNum = e.target.value;
var currentPatchNum =
@@ -313,12 +376,13 @@
return result;
},
- _computeReplyButtonHighlighted: function(drafts) {
- return Object.keys(drafts || {}).length > 0;
+ _computeReplyButtonHighlighted: function(changeRecord) {
+ var drafts = (changeRecord && changeRecord.base) || {};
+ return Object.keys(drafts).length > 0;
},
- _computeReplyButtonLabel: function(drafts) {
- drafts = drafts || {};
+ _computeReplyButtonLabel: function(changeRecord) {
+ var drafts = (changeRecord && changeRecord.base) || {};
var draftCount = Object.keys(drafts).reduce(function(count, file) {
return count + drafts[file].length;
}, 0);
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 7fcbd32..f4a0a52 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
@@ -81,6 +81,42 @@
assert.equal(replyButton.textContent, 'Reply (3)');
});
+ test('comment events properly update diff drafts', function() {
+ element._patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: 2,
+ };
+ var draft = {
+ __draft: true,
+ id: 'id1',
+ path: '/foo/bar.txt',
+ text: 'hello',
+ };
+ element._handleCommentSave({target: {comment: draft}});
+ draft.patch_set = 2;
+ assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft]});
+ draft.patch_set = null;
+ draft.text = 'hello, there';
+ element._handleCommentSave({target: {comment: draft}});
+ draft.patch_set = 2;
+ assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft]});
+ var draft2 = {
+ __draft: true,
+ id: 'id2',
+ path: '/foo/bar.txt',
+ text: 'hola',
+ };
+ element._handleCommentSave({target: {comment: draft2}});
+ draft2.patch_set = 2;
+ assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft, draft2]});
+ draft.patch_set = null;
+ element._handleCommentDiscard({target: {comment: draft}});
+ draft.patch_set = 2;
+ assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft2]});
+ element._handleCommentDiscard({target: {comment: draft2}});
+ assert.deepEqual(element._diffDrafts, {});
+ });
+
test('patch num change', function(done) {
element._changeNum = '42';
element._patchRange = {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index ded5108..646dfde 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -35,6 +35,12 @@
* @event comment-discard
*/
+ /**
+ * Fired when this comment is saved.
+ *
+ * @event comment-save
+ */
+
properties: {
changeNum: String,
comment: {
@@ -86,6 +92,7 @@
}
this.comment = comment;
this.editing = false;
+ this.fire('comment-save');
return obj;
}.bind(this));
