From fc18085eeb82894ab8edbde174bca332ea4681f4 Mon Sep 17 00:00:00 2001
From: Becky Siegel <beckysiegel@google.com>
Date: Thu, 8 Mar 2018 17:29:09 -0800
Subject: [PATCH] Update gr-diff-comment unresolved toggle

This changes updates the unresolved toggle so that it only attempts to
save when explicitly tapped (as opposed to the value of resolved
changing on its own).

This was causing issues with comment synchronization between
gr-thread-view and diff comments, and also re-rendering the order of
comments in gr-thread-list within the dom-repeat.

This change also lets the resolved checkbox get selected via the enter
key.

Change-Id: I6065c745a0eabcf19df38f3cd6fd2c0d7e7434f3
---
 .../diff/gr-diff-comment/gr-diff-comment.html      |  3 ++-
 .../diff/gr-diff-comment/gr-diff-comment.js        | 14 ++------------
 .../diff/gr-diff-comment/gr-diff-comment_test.html |  7 ++++++-
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 514bc80bfe..e1ebbf2db9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -290,7 +290,8 @@ limitations under the License.
           <div class="action resolve hideOnPublished">
             <label>
               <input type="checkbox"
-                  checked$="[[resolved]]"
+                  id="resolvedCheckbox"
+                  checked="[[resolved]]"
                   on-change="_handleToggleResolved">
               Resolved
             </label>
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 e45702cf36..c48f7ea7fb 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
@@ -113,10 +113,7 @@
       },
       commentSide: String,
 
-      resolved: {
-        type: Boolean,
-        observer: '_toggleResolved',
-      },
+      resolved: Boolean,
 
       _numPendingDraftRequests: {
         type: Object,
@@ -581,17 +578,10 @@
 
     _handleToggleResolved() {
       this.resolved = !this.resolved;
-    },
-
-    _toggleResolved(resolved, previousValue) {
-      // Do not proceed if this call is for the initial definition of the
-      // resolved property.
-      if (previousValue === undefined) { return; }
-
       // Modify payload instead of this.comment, as this.comment is passed from
       // the parent by ref.
       const payload = this._getEventPayload();
-      payload.comment.unresolved = !resolved;
+      payload.comment.unresolved = !this.$.resolvedCheckbox.checked;
       this.fire('comment-update', payload);
       if (!this.editing) {
         // Save the resolved state immediately.
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 c09e289dce..8c7894687a 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
@@ -636,7 +636,9 @@ limitations under the License.
 
     test('draft prevent save when disabled', () => {
       const saveStub = sandbox.stub(element, 'save');
+      element.showActions = true;
       element.draft = true;
+      MockInteractions.tap(element.$.header);
       MockInteractions.tap(element.$$('.edit'));
       element._messageText = 'good news, everyone!';
       element.flushDebouncer('fire-update');
@@ -680,7 +682,7 @@ limitations under the License.
       assert.isFalse(element.$$('.resolve input').checked);
     });
 
-    test('resolved checkbox saves when !editing', () => {
+    test('resolved checkbox saves with tap when !editing', () => {
       element.editing = false;
       const save = sandbox.stub(element, 'save');
 
@@ -688,6 +690,9 @@ limitations under the License.
       assert.isTrue(element.$$('.resolve input').checked);
       element.comment = {unresolved: true};
       assert.isFalse(element.$$('.resolve input').checked);
+      assert.isFalse(save.called);
+      MockInteractions.tap(element.$.resolvedCheckbox);
+      assert.isTrue(element.$$('.resolve input').checked);
       assert.isTrue(save.called);
     });