Open reply dialog when "Add reviewer" is tapped

The account entry for the change's reviewers will automatically focus
when the dialog opens in this case. Opening the reply dialog using
the "Reply" button should continue to focus on the main textarea.

Change-Id: I993d96633a6349a14bdb10354b8949bf35240352

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html

@@ -251,7 +251,9 @@ limitations under the License.
               change="{{_change}}"
               commit-info="[[_commitInfo]]"
               server-config="[[serverConfig]]"
-              mutable="[[_loggedIn]]"></gr-change-metadata>
+              mutable="[[_loggedIn]]"
+              on-show-reply-dialog="_handleShowReplyDialog">
+          </gr-change-metadata>
           <gr-change-actions id="actions"
               actions="[[_change.actions]]"
               change-num="[[_changeNum]]"

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js

@@ -316,6 +316,10 @@
       this.$.replyOverlay.close();
     },
 
+    _handleShowReplyDialog: function(e) {
+      this._openReplyDialog(this.$.replyDialog.FocusTarget.REVIEWERS);
+    },
+
     _paramsChanged: function(value) {
       if (value.view !== this.tagName.toLowerCase()) { return; }
 
@@ -502,9 +506,10 @@
       });
     },
 
-    _openReplyDialog: function() {
+    _openReplyDialog: function(opt_section) {
       this.$.replyOverlay.open().then(function() {
         this.$.replyOverlay.setFocusStops(this.$.replyDialog.getFocusStops());
+        this.$.replyDialog.focusOn(opt_section);
       }.bind(this));
     },
 

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js

@@ -14,6 +14,11 @@
 (function() {
   'use strict';
 
+  var FocusTarget = {
+    BODY: 'body',
+    REVIEWERS: 'reviewers',
+  };
+
   Polymer({
     is: 'gr-reply-dialog',
 
@@ -51,6 +56,8 @@
       _reviewers: Array,
     },
 
+    FocusTarget: FocusTarget,
+
     behaviors: [
       Gerrit.RESTClientBehavior,
     ],
@@ -70,9 +77,17 @@
     },
 
     focus: function() {
-      this.async(function() {
+      this.focusOn(FocusTarget.BODY);
-        this.$.textarea.textarea.focus();
+    },
-      }.bind(this));
+
+    focusOn: function(section) {
+      if (section === FocusTarget.BODY) {
+        var textarea = this.$.textarea;
+        textarea.async(textarea.textarea.focus.bind(textarea.textarea));
+      } else if (section === FocusTarget.REVIEWERS) {
+        var reviewerEntry = this.$.reviewers.focusStart;
+        reviewerEntry.async(reviewerEntry.focus);
+      }
     },
 
     getFocusStops: function() {

polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.html

@@ -19,7 +19,6 @@ limitations under the License.
 <link rel="import" href="../../shared/gr-account-chip/gr-account-chip.html">
 <link rel="import" href="../../shared/gr-button/gr-button.html">
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
-<link rel="import" href="../gr-account-entry/gr-account-entry.html">
 
 <dom-module id="gr-reviewer-list">
   <template>
@@ -45,19 +44,12 @@ limitations under the License.
       gr-account-chip {
         margin-top: .3em;
       }
-      .remove,
+      .remove {
-      .cancel {
         color: #999;
       }
       .remove {
         font-size: .9em;
       }
-      .cancel {
-        font-size: 2em;
-        line-height: 1;
-        padding: 0 .15em;
-        text-decoration: none;
-      }
       @media screen and (max-width: 50em), screen and (min-width: 75em) {
         gr-account-chip:first-of-type {
           margin-top: 0;
@@ -72,28 +64,11 @@ limitations under the License.
       </gr-account-chip>
     </template>
     <div class="controlsContainer" hidden$="[[!mutable]]">
-      <div class="autocompleteContainer" hidden$="[[!_showInput]]">
-        <div class="inputContainer">
-          <gr-account-entry
-              id="accountEntry"
-              suggestFrom="[[suggestFrom]]"
-              clear-on-commit
-              change="[[change]]"
-              disabled="[[disabled]]"
-              on-commit="_sendAddRequest"
-              on-cancel="_handleCancelTap"></gr-account-entry>
-          <gr-button
-              link
-              class="cancel"
-              on-tap="_handleCancelTap">×</gr-button>
-        </div>
-      </div>
       <gr-button
           link
           id="addReviewer"
           class="addReviewer"
-          on-tap="_handleAddTap"
+          on-tap="_handleAddTap">Add reviewer</gr-button>
-          hidden$="[[_showInput]]">Add reviewer</gr-button>
     </div>
     <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
   </template>

polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js

@@ -17,6 +17,12 @@
   Polymer({
     is: 'gr-reviewer-list',
 
+    /**
+     * Fired when the "Add reviewer..." button is tapped.
+     *
+     * @event show-reply-dialog
+     */
+
     properties: {
       change: Object,
       disabled: {
@@ -104,52 +110,7 @@
 
     _handleAddTap: function(e) {
       e.preventDefault();
-      this._showInput = true;
+      this.fire('show-reply-dialog');
-      this.$.accountEntry.focus();
-    },
-
-    _handleCancelTap: function(e) {
-      e.preventDefault();
-      this.$.accountEntry.clear();
-      this._cancel();
-    },
-
-    _cancel: function() {
-      this._showInput = false;
-      this.$.accountEntry.clear();
-      this.$.addReviewer.focus();
-    },
-
-    _sendAddRequest: function(e, detail) {
-      var reviewer = detail.value;
-      var reviewerID;
-      if (reviewer.account) {
-        reviewerID = reviewer.account._account_id;
-      } else if (reviewer.group) {
-        reviewerID = reviewer.group.id;
-      }
-
-      this.disabled = true;
-      this._xhrPromise = this._addReviewer(reviewerID).then(function(response) {
-        this.change.reviewers.CC = this.change.reviewers.CC || [];
-        this.disabled = false;
-        if (!response.ok) { return response; }
-
-        return this.$.restAPI.getResponseObject(response).then(function(obj) {
-          obj.reviewers.forEach(function(r) {
-            this.push('change.removable_reviewers', r);
-            this.push('change.reviewers.CC', r);
-          }, this);
-          this.$.accountEntry.focus();
-        }.bind(this));
-      }.bind(this)).catch(function(err) {
-        this.disabled = false;
-        throw err;
-      }.bind(this));
-    },
-
-    _addReviewer: function(id) {
-      return this.$.restAPI.addChangeReviewer(this.change._number, id);
     },
 
     _removeReviewer: function(id) {

polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.html

@@ -34,57 +34,10 @@ limitations under the License.
 <script>
   suite('gr-reviewer-list tests', function() {
     var element;
-    var autocompleteInput;
 
     setup(function() {
       element = fixture('basic');
-      // TODO(logan): Rewrite this test to not delve so deeply into internals.
-      autocompleteInput = element.$.accountEntry.$.input.$.input;
       stub('gr-rest-api-interface', {
-        getChangeSuggestedReviewers: function() {
-          return Promise.resolve([
-            {
-              account: {
-                _account_id: 1021482,
-                name: 'Andrew Bonventre',
-                email: 'andybons@chromium.org',
-              }
-            },
-            {
-              account: {
-                _account_id: 1021863,
-                name: 'Andrew Bonventre',
-                email: 'andybons@google.com',
-              }
-            },
-            {
-              group: {
-                id: 'c7af6dd375c092ff3b23c0937aa910693dc0c41b',
-                name: 'andy',
-              }
-            }
-          ]);
-        },
-        addChangeReviewer: function() {
-          return Promise.resolve({
-            ok: true,
-            text: function() {
-              return Promise.resolve(
-                ')]}\'\n' +
-                JSON.stringify({
-                  reviewers: [{
-                    _account_id: 1021482,
-                    approvals: {
-                      'Code-Review': ' 0'
-                    },
-                    email: 'andybons@chromium.org',
-                    name: 'Andrew Bonventre',
-                  }]
-                })
-              );
-            },
-          });
-        },
         removeChangeReviewer: function() {
           return Promise.resolve({ok: true});
         },
@@ -98,37 +51,11 @@ limitations under the License.
       assert.isFalse(element.$$('.controlsContainer').hasAttribute('hidden'));
     });
 
-    function getActiveElement() {
+    test('add reviewer button opens reply dialog', function(done) {
-      var root = document;
+      element.addEventListener('show-reply-dialog', function() {
-      while (root && root.activeElement.shadowRoot) {
+        done();
-        var shadowRoot = root.activeElement.shadowRoot;
+      });
-        if (!shadowRoot.activeElement) {
-          break;
-        }
-        root = shadowRoot;
-      }
-      return root.activeElement;
-    }
-
-    test('show/hide accountEntry', function() {
-      element.mutable = true;
-      assert.isFalse(element.$$('.addReviewer').hasAttribute('hidden'));
-      assert.isTrue(
-          element.$$('.autocompleteContainer').hasAttribute('hidden'));
-      assert.notEqual(getActiveElement().id, 'input');
-
       MockInteractions.tap(element.$$('.addReviewer'));
-      assert.isTrue(element.$$('.addReviewer').hasAttribute('hidden'));
-      assert.isFalse(
-          element.$$('.autocompleteContainer').hasAttribute('hidden'));
-      assert.equal(getActiveElement().id, 'input');
-
-      MockInteractions.pressAndReleaseKeyOn(autocompleteInput, 27); // 'esc'
-
-      assert.isFalse(element.$$('.addReviewer').hasAttribute('hidden'));
-      assert.isTrue(
-          element.$$('.autocompleteContainer').hasAttribute('hidden'));
-      assert.equal(getActiveElement().id, 'addReviewer');
     });
 
     test('only show remove for removable reviewers', function() {
@@ -186,69 +113,5 @@ limitations under the License.
         }
       });
     });
-
-    test('autocomplete starts at >= 3 chars', function() {
-      element._inputRequestTimeout = 0;
-      element._mutable = true;
-      element.change = {_number: 123};
-
-      element.$.accountEntry.text = 'fo';
-
-      flushAsynchronousOperations();
-
-      assert.isFalse(element.$.restAPI.getChangeSuggestedReviewers.called);
-    });
-
-    test('add/remove reviewer flow', function(done) {
-      element.change = {
-        _number: 42,
-        reviewers: {},
-        removable_reviewers: [],
-        owner: {_account_id: 0},
-      };
-      element._inputRequestTimeout = 0;
-      element._mutable = true;
-      MockInteractions.tap(element.$$('.addReviewer'));
-      flushAsynchronousOperations();
-      element.$.accountEntry.$.input.text = 'andy';
-
-      var stub = element.$.restAPI.getChangeSuggestedReviewers;
-      stub.lastCall.returnValue.then(function() {
-        flushAsynchronousOperations();
-
-        MockInteractions.pressAndReleaseKeyOn(autocompleteInput, 27); // 'esc'
-        assert.isTrue(element.$$('.autocompleteContainer')
-            .hasAttribute('hidden'));
-
-        MockInteractions.tap(element.$$('.addReviewer'));
-
-        element.$.accountEntry.text = 'andyb';
-        stub.lastCall.returnValue.then(function() {
-          MockInteractions.pressAndReleaseKeyOn(
-              autocompleteInput, 13); // 'enter'
-          assert.isTrue(element.disabled);
-
-          element._xhrPromise.then(function() {
-            assert.isFalse(element.disabled);
-            flushAsynchronousOperations();
-            var reviewerEls =
-                Polymer.dom(element.root).querySelectorAll('.reviewer');
-            assert.equal(reviewerEls.length, 1);
-            MockInteractions.tap(element.$$('.reviewer').$$('gr-button'));
-            flushAsynchronousOperations();
-            assert.isTrue(element.disabled);
-
-            element._xhrPromise.then(function() {
-              flushAsynchronousOperations();
-              assert.isFalse(element.disabled);
-              var reviewerEls =
-                  Polymer.dom(element.root).querySelectorAll('.reviewer');
-              assert.equal(reviewerEls.length, 0);
-              done();
-            });
-          });
-        });
-      });
-    });
   });
 </script>