Properly remove discarded comments from model in gr-new-diff
Change-Id: Icb5d3ba1edd2cd3a75aa6568a152bacc5e0babda
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
index 9e8156b..39477bc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
@@ -36,7 +36,7 @@
}
</style>
<div id="container">
- <template id="commentList" is="dom-repeat" items="{{_orderedComments}}" as="comment">
+ <template id="commentList" is="dom-repeat" items="[[_orderedComments]]" as="comment">
<gr-diff-comment
comment="{{comment}}"
change-num="[[changeNum]]"
@@ -46,7 +46,7 @@
project-config="[[projectConfig]]"
on-height-change="_handleCommentHeightChange"
on-reply="_handleCommentReply"
- on-discard="_handleCommentDiscard"
+ on-comment-discard="_handleCommentDiscard"
on-done="_handleCommentDone"></gr-diff-comment>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index b019730..c7bbff2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -26,7 +26,7 @@
/**
* Fired when the thread should be discarded.
*
- * @event discard
+ * @event thread-discard
*/
properties: {
@@ -204,10 +204,6 @@
},
_handleCommentDiscard: function(e) {
- // TODO(andybons): In Shadow DOM, the event bubbles up, while in Shady
- // DOM, it respects the bubbles property.
- // https://github.com/Polymer/polymer/issues/3226
- e.stopPropagation();
var diffCommentEl = Polymer.dom(e).rootTarget;
var idx = this._indexOf(diffCommentEl.comment, this.comments);
if (idx == -1) {
@@ -216,7 +212,7 @@
}
this.splice('comments', idx, 1);
if (this.comments.length == 0) {
- this.fire('discard', null, {bubbles: false});
+ this.fire('thread-discard');
return;
}
this.async(this._heightChanged.bind(this), 1);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
index 2a6c999..82b0faf 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -235,7 +235,7 @@
var draftEl =
Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
assert.ok(draftEl);
- draftEl.addEventListener('discard', function() {
+ draftEl.addEventListener('comment-discard', function() {
server.respond();
var drafts = element.comments.filter(function(c) {
return c.__draft == true;
@@ -243,7 +243,7 @@
assert.equal(drafts.length, 0);
done();
});
- draftEl.fire('discard', null, {bubbles: false});
+ draftEl.fire('comment-discard', null, {bubbles: false});
});
});
</script>
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 ca0bedb..dcdb881 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
@@ -38,7 +38,7 @@
/**
* Fired when this comment is discarded.
*
- * @event discard
+ * @event comment-discard
*/
properties: {
@@ -190,7 +190,7 @@
_handleCancel: function(e) {
this._preventDefaultAndBlur(e);
if (this.comment.message == null || this.comment.message.length == 0) {
- this.fire('discard', null, {bubbles: false});
+ this.fire('comment-discard');
return;
}
this._editDraft = this.comment.message;
@@ -205,11 +205,11 @@
this.disabled = true;
var commentID = this.comment.id;
if (!commentID) {
- this.fire('discard', null, {bubbles: false});
+ this.fire('comment-discard');
return;
}
this._send('DELETE', this._restEndpoint(commentID)).then(function(req) {
- this.fire('discard', null, {bubbles: false});
+ this.fire('comment-discard');
}.bind(this)).catch(function(err) {
alert('Your draft couldn’t be deleted. Check the console and ' +
'contact the PolyGerrit team for assistance.');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index 799dbf2..c95a9a4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -205,7 +205,7 @@
assert.isTrue(disabled, 'save button should be disabled.');
var numDiscardEvents = 0;
- element.addEventListener('discard', function(e) {
+ element.addEventListener('comment-discard', function(e) {
numDiscardEvents++;
if (numDiscardEvents == 3) {
done();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-side/gr-diff-side.js b/polygerrit-ui/app/elements/diff/gr-diff-side/gr-diff-side.js
index 98afd78..b97d7b7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-side/gr-diff-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-side/gr-diff-side.js
@@ -404,7 +404,7 @@
var threadEl = document.createElement('gr-diff-comment-thread');
threadEl.addEventListener('height-change',
this._handleCommentThreadHeightChange.bind(this));
- threadEl.addEventListener('discard',
+ threadEl.addEventListener('thread-discard',
this._handleCommentThreadDiscard.bind(this));
threadEl.setAttribute('data-index', index);
threadEl.changeNum = this.changeNum;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 91f5f3b..e46bbb8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -220,7 +220,7 @@
// Discard the right thread and ensure the left comment heights are
// back to their original values.
- newThreadEls[1].addEventListener('discard', function() {
+ newThreadEls[1].addEventListener('thread-discard', function() {
rightFillerEls =
Polymer.dom(element.$.rightDiff.root).querySelectorAll(
'[data-index="' + index + '"]');
@@ -236,7 +236,7 @@
done();
});
var commentEl = newThreadEls[1].$$('gr-diff-comment');
- commentEl.fire('discard', null, {bubbles: false});
+ commentEl.fire('comment-discard');
});
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.js b/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.js
index 1a24596..aafea43 100644
--- a/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff.js
@@ -82,6 +82,11 @@
this._getLoggedIn().then(function(loggedIn) {
this._loggedIn = loggedIn;
}.bind(this));
+
+ this.addEventListener('thread-discard',
+ this._handleThreadDiscard.bind(this));
+ this.addEventListener('comment-discard',
+ this._handleCommentDiscard.bind(this));
},
reload: function() {
@@ -277,20 +282,42 @@
}
threadEl = this._builder.createCommentThread(this.changeNum, patchNum,
this.path, side, this.projectConfig);
- // TODO(andybons): Remove once migration is made to gr-new-diff.
- threadEl.addEventListener('discard',
- this._handleThreadDiscard.bind(this));
contentEl.appendChild(threadEl);
}
threadEl.addDraft(opt_lineNum);
},
_handleThreadDiscard: function(e) {
- e.stopPropagation();
var el = Polymer.dom(e).rootTarget;
el.parentNode.removeChild(el);
},
+ _handleCommentDiscard: function(e) {
+ var comment = Polymer.dom(e).rootTarget.comment;
+ this._removeComment(comment);
+ },
+
+ _removeComment: function(comment) {
+ if (!comment.id) { return; }
+ this._removeCommentFromSide(comment, DiffSide.LEFT) ||
+ this._removeCommentFromSide(comment, DiffSide.RIGHT);
+ },
+
+ _removeCommentFromSide: function(comment, side) {
+ var idx = -1;
+ for (var i = 0; i < this._comments[side].length; i++) {
+ if (this._comments[side][i].id === comment.id) {
+ idx = i;
+ break;
+ }
+ }
+ if (idx !== -1) {
+ this.splice('_comments.' + side, idx, 1);
+ return true;
+ }
+ return false;
+ },
+
_handleMouseDown: function(e) {
var el = Polymer.dom(e).rootTarget;
var side;
@@ -480,12 +507,10 @@
},
_projectConfigChanged: function(projectConfig) {
- var threadEls =
- Polymer.dom(this.root).querySelectorAll('gr-diff-comment-thread');
+ var threadEls = this._getCommentThreads();
for (var i = 0; i < threadEls.length; i++) {
threadEls[i].projectConfig = projectConfig;
}
},
-
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff_test.html b/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff_test.html
index eaa0618..b015df4 100644
--- a/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-new-diff/gr-new-diff_test.html
@@ -142,5 +142,103 @@
done();
});
});
+
+ test('remove comment', function() {
+ element._comments = {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1'},
+ {id: 'bc2'},
+ {id: 'bd1', __draft: true},
+ {id: 'bd2', __draft: true},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ };
+
+ element._removeComment({});
+ assert.deepEqual(element._comments, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1'},
+ {id: 'bc2'},
+ {id: 'bd1', __draft: true},
+ {id: 'bd2', __draft: true},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ });
+
+ element._removeComment({id: 'bc2'});
+ assert.deepEqual(element._comments, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1'},
+ {id: 'bd1', __draft: true},
+ {id: 'bd2', __draft: true},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ {id: 'd2', __draft: true},
+ ],
+ });
+
+ element._removeComment({id: 'd2'});
+ assert.deepEqual(element._comments, {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1'},
+ {id: 'bd1', __draft: true},
+ {id: 'bd2', __draft: true},
+ ],
+ right: [
+ {id: 'c1'},
+ {id: 'c2'},
+ {id: 'd1', __draft: true},
+ ],
+ });
+ });
});
</script>
