Fix logic for empty comment threads removing themselves
When you create a new comment thread and then CANCEL we are removing
the comment thread element from the DOM. But the logic was too
aggressive. It would even remove threads that never made it into
editing mode.
Google-Bug-Id: b/208568882
Change-Id: Idc423bcb58dd7e87e94017c4760c0f9fcc8bf0f8
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index de8a02d..a895a5b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -599,7 +599,10 @@
}
}
if (changed.has('editing')) {
- if (!this.editing) {
+ // changed.get('editing') contains the old value. We only want to trigger
+ // when changing from editing to non-editing (user has cancelled/saved).
+ // We do *not* want to trigger on first render (old value is `null`)
+ if (!this.editing && changed.get('editing') === true) {
this.unsavedComment = undefined;
if (this.thread?.comments.length === 0) {
this.remove();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index ab08996..347e1e0 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -39,6 +39,7 @@
} from '../../../test/test-data-generators';
import {tap} from '@polymer/iron-test-helpers/mock-interactions';
import {SinonStub} from 'sinon';
+import {waitUntil} from '@open-wc/testing-helpers';
const basicFixture = fixtureFromElement('gr-comment-thread');
@@ -271,6 +272,32 @@
});
});
+ suite('self removal when empty thread changed to editing:false', () => {
+ let threadEl: GrCommentThread;
+
+ setup(async () => {
+ threadEl = basicFixture.instantiate();
+ threadEl.thread = createThread();
+ });
+
+ test('new thread el normally has a parent and an unsaved comment', async () => {
+ await waitUntil(() => threadEl.editing);
+ assert.isOk(threadEl.unsavedComment);
+ assert.isOk(threadEl.parentElement);
+ });
+
+ test('thread el removed after clicking CANCEL', async () => {
+ await waitUntil(() => threadEl.editing);
+
+ const commentEl = queryAndAssert(threadEl, 'gr-comment');
+ const buttonEl = queryAndAssert(commentEl, 'gr-button.cancel');
+ tap(buttonEl);
+
+ await waitUntil(() => !threadEl.editing);
+ assert.isNotOk(threadEl.parentElement);
+ });
+ });
+
test('comments are sorted correctly', () => {
const comments: CommentInfo[] = [
{
