From f2247d41df942da56cb0f67ca235f334dcbad1a1 Mon Sep 17 00:00:00 2001
From: Wyatt Allen <wyatta@google.com>
Date: Thu, 26 Oct 2017 18:01:18 -0700
Subject: [PATCH] Move Save button to left side and mark as tertiary

The save button is moved to the left side of the reply dialog, is marked
as tertiary so that it's text color becomes black, and is given an
explanatory tooltip.

Bug: Issue 7466
Change-Id: I34b198ec150f4ebaa90e5bc157d7efc393176424
---
 .../gr-reply-dialog/gr-reply-dialog.html      | 21 ++++++++++++-------
 .../change/gr-reply-dialog/gr-reply-dialog.js | 15 +++++++++++++
 .../elements/shared/gr-button/gr-button.html  |  3 +++
 .../elements/shared/gr-button/gr-button.js    |  5 +++++
 polygerrit-ui/app/styles/app-theme.html       |  1 +
 5 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index 593b932da7..3d287a61a4 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -60,7 +60,7 @@ limitations under the License.
         display: flex;
         justify-content: space-between;
       }
-      .actions gr-button {
+      .actions .right gr-button {
         margin-left: 1em;
       }
       .peopleContainer,
@@ -267,6 +267,16 @@ limitations under the License.
       </section>
       <section class="actions">
         <div class="left">
+          <template is="dom-if" if="[[canBeStarted]]">
+            <gr-button
+                link
+                tertiary
+                disabled="[[_isState(knownLatestState, 'not-latest')]]"
+                class="action save"
+                has-tooltip
+                title="[[_saveTooltip]]"
+                on-tap="_saveTapHandler">Save</gr-button>
+          </template>
           <span
               id="checkingStatusLabel"
               hidden$="[[!_isState(knownLatestState, 'checking')]]">
@@ -290,14 +300,9 @@ limitations under the License.
               primary
               disabled="[[_computeSendButtonDisabled(knownLatestState, _sendButtonLabel, diffDrafts, draft, _reviewersMutated, _labelsChanged, _includeComments)]]"
               class="action send"
+              has-tooltip
+              title$="[[_computeSendButtonTooltip(canBeStarted)]]"
               on-tap="_sendTapHandler">[[_sendButtonLabel]]</gr-button>
-          <template is="dom-if" if="[[canBeStarted]]">
-            <gr-button
-                link
-                disabled="[[_isState(knownLatestState, 'not-latest')]]"
-                class="action save"
-                on-tap="_saveTapHandler">Save</gr-button>
-          </template>
         </div>
       </section>
     </div>
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index ef9bc8d026..a1719da840 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -39,6 +39,12 @@
     SEND: 'Send',
   };
 
+  const ButtonTooltips = {
+    SAVE: 'Save reply but do not send',
+    START_REVIEW: 'Mark as ready for review and send reply',
+    SEND: 'Send reply',
+  };
+
   // TODO(logan): Remove once the fix for issue 6841 is stable on
   // googlesource.com.
   const START_REVIEW_MESSAGE = 'This change is ready for review.';
@@ -188,6 +194,11 @@
         type: Boolean,
         value: false,
       },
+      _saveTooltip: {
+        type: String,
+        value: ButtonTooltips.SAVE,
+        readOnly: true,
+      },
     },
 
     FocusTarget,
@@ -798,6 +809,10 @@
       return canBeStarted ? ButtonLabels.START_REVIEW : ButtonLabels.SEND;
     },
 
+    _computeSendButtonTooltip(canBeStarted) {
+      return canBeStarted ? ButtonTooltips.START_REVIEW : ButtonTooltips.SEND;
+    },
+
     _computeCCsEnabled(serverConfig) {
       return serverConfig && serverConfig.note_db_enabled;
     },
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
index 7efd8972f1..f4b54aa9ac 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -41,6 +41,9 @@ limitations under the License.
         font-family: var(--font-family-bold);
         text-transform: none;
       }
+      :host([link][tertiary]) {
+        color: var(--color-link-tertiary);
+      }
       :host([link]) paper-button {
         margin: 0;
         padding: 0;
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
index ef5e489a6f..8e66e11b27 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -28,6 +28,11 @@
         value: false,
         reflectToAttribute: true,
       },
+      tertiary: {
+        type: Boolean,
+        value: false,
+        reflectToAttribute: true,
+      },
       disabled: {
         type: Boolean,
         observer: '_disabledChanged',
diff --git a/polygerrit-ui/app/styles/app-theme.html b/polygerrit-ui/app/styles/app-theme.html
index 6a811580fa..2fe7662b87 100644
--- a/polygerrit-ui/app/styles/app-theme.html
+++ b/polygerrit-ui/app/styles/app-theme.html
@@ -38,6 +38,7 @@ limitations under the License.
 
   /* Follow are a part of the design refresh */
   --color-link: #2a66d9;
+  --color-link-tertiary: #000;
   /* 12% darker */
   --color-button-hover: #0B47BA;
 }