Modify reply dialog send disabled behavior

There were two issues around the reply dialog's sending behavior that
needed to be worked out:
- The keyboard shortcut for sending a reply did not respect the
  _computeSendDisabled state
- It was impossible to use keyboard navigation only for applying a code
  review and sending the reply

The first issue is an obvious fix, but the second issue is more
complicated -- due to Issue 4165, PG must override the default focus
stops that iron-overlay-behavior tries to find. The send button
is the end focus stop and has a dynamic tabindex (-1 or 0). Setting the
end focus stop to something that is unfocusable causes the focus to be
unbounded, cycling into the background content.

With this change, we now:
- Disable sending replies if the send button is disabled, and show the
  user a toast with an explanation
- Fire an event from within the reply dialog when the send button
  disabled state changes
- Handle that event in the change view, which calls a simple function to
  reset the focus stops with the correct end value

Bug: Issue 8627
Change-Id: I2da76e3f346f73677b220f0d72bbdfe3ee438a4c

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

@@ -611,6 +611,7 @@ limitations under the License.
           on-send="_handleReplySent"
           on-cancel="_handleReplyCancel"
           on-autogrow="_handleReplyAutogrow"
+          on-send-disabled-changed="_resetReplyOverlayFocusStops"
           hidden$="[[!_loggedIn]]">
       </gr-reply-dialog>
     </gr-overlay>

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

@@ -999,7 +999,7 @@
      */
     _openReplyDialog(opt_section) {
       this.$.replyOverlay.open().then(() => {
-        this.$.replyOverlay.setFocusStops(this.$.replyDialog.getFocusStops());
+        this._resetReplyOverlayFocusStops();
         this.$.replyDialog.open(opt_section);
         Polymer.dom.flush();
         this.$.replyOverlay.center();
@@ -1554,5 +1554,9 @@
     _handleStopEditTap() {
       Gerrit.Nav.navigateToChange(this._change, this._patchRange.patchNum);
     },
+
+    _resetReplyOverlayFocusStops() {
+      this.$.replyOverlay.setFocusStops(this.$.replyDialog.getFocusStops());
+    },
   });
 })();

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.html

@@ -115,7 +115,7 @@ limitations under the License.
       sandbox.restore();
     });
 
-    test('send blocked when invalid email is supplied to ccs', () => {
+    test('_submit blocked when invalid email is supplied to ccs', () => {
       const sendStub = sandbox.stub(element, 'send').returns(Promise.resolve());
       // Stub the below function to avoid side effects from the send promise
       // resolving.

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

@@ -291,9 +291,10 @@ limitations under the License.
               class="action cancel"
               on-tap="_cancelTapHandler">Cancel</gr-button>
           <gr-button
+              id="sendButton"
               link
               primary
-              disabled="[[_computeSendButtonDisabled(_sendButtonLabel, diffDrafts, draft, _reviewersMutated, _labelsChanged, _includeComments)]]"
+              disabled="[[_sendDisabled]]"
               class="action send"
               has-tooltip
               title$="[[_computeSendButtonTooltip(canBeStarted)]]"

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

@@ -52,6 +52,8 @@
   // googlesource.com.
   const START_REVIEW_MESSAGE = 'This change is ready for review.';
 
+  const EMPTY_REPLY_MESSAGE = 'Cannot send an empty reply.';
+
   Polymer({
     is: 'gr-reply-dialog',
 
@@ -87,6 +89,12 @@
      * @event comment-refresh
      */
 
+     /**
+      * Fires when the state of the send button (enabled/disabled) changes.
+      *
+      * @event send-disabled-changed
+      */
+
     properties: {
       /**
        * @type {{ _number: number, removable_reviewers: Array }}
@@ -206,6 +214,12 @@
         type: String,
         value: '',
       },
+      _sendDisabled: {
+        type: Boolean,
+        computed: '_computeSendButtonDisabled(_sendButtonLabel, diffDrafts, ' +
+            'draft, _reviewersMutated, _labelsChanged, _includeComments)',
+        observer: '_sendDisabledChanged',
+      },
     },
 
     FocusTarget,
@@ -273,9 +287,10 @@
     },
 
     getFocusStops() {
+      const end = this._sendDisabled ? this.$.cancelButton : this.$.sendButton;
       return {
         start: this.$.reviewers.focusStart,
-        end: this.$.sendButton,
+        end,
       };
     },
 
@@ -726,6 +741,13 @@
         // the text field of the CC entry.
         return;
       }
+      if (this._sendDisabled) {
+        this.dispatchEvent(new CustomEvent('show-alert', {
+          bubbles: true,
+          detail: {message: EMPTY_REPLY_MESSAGE},
+        }));
+        return;
+      }
       return this.send(this._includeComments, this.canBeStarted)
           .then(keepReviewers => {
             this._purgeReviewersPendingRemove(false, keepReviewers);
@@ -856,5 +878,9 @@
     setPluginMessage(message) {
       this._pluginMessage = message;
     },
+
+    _sendDisabledChanged(sendDisabled) {
+      this.dispatchEvent(new CustomEvent('send-disabled-changed'));
+    },
   });
 })();

polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html

@@ -1100,6 +1100,33 @@ limitations under the License.
       assert.isFalse(fn('Send', {}, '', false, true, false));
     });
 
+    test('_submit blocked when no mutations exist', () => {
+      const sendStub = sandbox.stub(element, 'send').returns(Promise.resolve());
+      // Stub the below function to avoid side effects from the send promise
+      // resolving.
+      sandbox.stub(element, '_purgeReviewersPendingRemove');
+      element.diffDrafts = {};
+      flushAsynchronousOperations();
+
+      MockInteractions.tap(element.$$('gr-button.send'));
+      assert.isFalse(sendStub.called);
+
+      element.diffDrafts = {test: true};
+      flushAsynchronousOperations();
+
+      MockInteractions.tap(element.$$('gr-button.send'));
+      assert.isTrue(sendStub.called);
+    });
+
+    test('getFocusStops', () => {
+      // Setting diffDrafts to an empty object causes _sendDisabled to be
+      // computed to false.
+      element.diffDrafts = {};
+      assert.equal(element.getFocusStops().end, element.$.cancelButton);
+      element.diffDrafts = {test: true};
+      assert.equal(element.getFocusStops().end, element.$.sendButton);
+    });
+
     test('setPluginMessage', () => {
       element.setPluginMessage('foo');
       assert.equal(element.$.pluginMessage.textContent, 'foo');