From fd2548fb47e5f667ddbbbd481ca98b908b41b34d Mon Sep 17 00:00:00 2001
From: Becky Siegel <beckysiegel@google.com>
Date: Fri, 3 Nov 2017 12:22:24 -0700
Subject: [PATCH] Consolidate button styles and update disabled

Mostly general cleanup, also the following changes:
- Disabled flat buttons will have a fully transparent background (before
  it was a partially transparent white background)
- Disabled raised buttons have a (light) grey background and (darker)
  grey text. There is no difference in the disabled styling whether the
  button is primary or not.
- Add loading as a property on gr-button, translate that into disabled
  on the paper-button, which removes some CSS rules.

Bug: Issue 7496
Change-Id: Iae0564d099508827c8f13fbb943c6646d631ce1d
---
 .../elements/shared/gr-button/gr-button.html  | 124 +++++++++---------
 .../elements/shared/gr-button/gr-button.js    |  13 ++
 .../shared/gr-button/gr-button_test.html      |  10 ++
 3 files changed, 86 insertions(+), 61 deletions(-)

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 2246150654..483882ff58 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -24,6 +24,7 @@ limitations under the License.
 <dom-module id="gr-button">
   <template strip-whitespace>
     <style include="shared-styles">
+      /* general styles for all buttons */
       :host {
         display: inline-block;
         font-family: var(--font-family-bold);
@@ -33,56 +34,14 @@ limitations under the License.
       :host([hidden]) {
         display: none;
       }
-      :host([link]) {
-        background-color: transparent;
-        border: none;
-        color: var(--color-link);
-        font-size: inherit;
-        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;
-        @apply --gr-button;
-      }
-      paper-button[raised] {
-        background-color: var(--gr-button-background, #fff);
-        color: var(--gr-button-color, --color-link);
-      }
       :host([no-uppercase]) paper-button {
         text-transform: none;
       }
-      /* todo (beckysiegel) switch all secondary to primary as there is no color
-        distinction anymore. */
-      :host([primary]) paper-button[raised],
-      :host([secondary]) paper-button[raised] {
-        background-color: var(--color-link);
-        color: #fff;
-      }
-      :host([primary][disabled]) paper-button[raised],
-      :host([disabled]) paper-button {
-        opacity: .5;
-      }
-      :host([link]) paper-button:hover,
-      :host([link]) paper-button:focus,
-      paper-button[raised]:hover,
-      paper-button[raised]:focus  {
-        color: var(--gr-button-hover-color, --color-button-hover);
-      }
-      :host([primary]) paper-button[raised]:hover,
-      :host([primary]) paper-button[raised]:focus,
-      :host([secondary]) paper-button[raised]:hover,
-      :host([secondary]) paper-button[raised]:focus {
-        background-color: var(--gr-button-hover-background-color, --color-button-hover);
-        color: var(--gr-button-color, #fff);
-      }
-      paper-button,
-      paper-button[raised],
-      paper-button[link] {
+      paper-button {
+        /* Some of these are overridden for link style buttons since buttons
+         without the link attribute are raised */
+        background-color: var(--gr-button-background, #fff);
+        color: var(--gr-button-color, var(--color-link));
         display: flex;
         align-items: center;
         justify-content: center;
@@ -91,11 +50,15 @@ limitations under the License.
         padding: .4em .85em;
         @apply --gr-button;
       }
-      :host([link]) paper-button {
-        --paper-button: {
-          padding: 0;
-        }
+      paper-button:hover,
+      paper-button:focus {
+        color: var(--gr-button-hover-color, var(--color-button-hover));
       }
+      :host([disabled]) paper-button {
+        color: #a8a8a8;
+        cursor: wait;
+      }
+      /* styles for the optional down arrow */
       :host:not([down-arrow]) .downArrow {display: none; }
       :host([down-arrow]) .downArrow {
         border-top: .36em solid var(--gr-button-arrow-color, #ccc);
@@ -108,21 +71,60 @@ limitations under the License.
       :host([down-arrow]) paper-button:hover .downArrow {
         border-top-color: var(--gr-button-arrow-hover-color, #666);
       }
-      :host([loading]) paper-button,
-      :host([disabled]) paper-button {
-        color: #aaa;
+
+      /* styles for raised buttons specifically*/
+      :host([primary]) paper-button[raised],
+      :host([secondary]) paper-button[raised] {
+        background-color: var(--color-link);
+        color: #fff;
       }
-      :host([loading]) paper-button,
-      :host([loading][disabled]) paper-button {
-        cursor: wait;
-        background-color: #efefef;
-        color: #aaa;
+      :host([primary]) paper-button[raised]:hover,
+      :host([primary]) paper-button[raised]:focus,
+      :host([secondary]) paper-button[raised]:hover,
+      :host([secondary]) paper-button[raised]:focus {
+        background-color: var(--gr-button-hover-background-color, var(--color-button-hover));
+        color: var(--gr-button-color, #fff);
+      }
+      :host([disabled]) paper-button[raised] {
+        background-color: #eaeaea;
+        color: #a8a8a8;
+      }
+      /* styles for link buttons specifically */
+      :host([link]) {
+        background-color: transparent;
+        border: none;
+        color: var(--color-link);
+        font-size: inherit;
+        font-family: var(--font-family-bold);
+        text-transform: none;
+      }
+      :host([link][tertiary]) {
+        color: var(--color-link-tertiary);
+      }
+      :host([link]) paper-button {
+        background-color: transparent;
+        margin: 0;
+        padding: 0;
+        --paper-button: {
+          padding: 0;
+        }
+        @apply --gr-button;
+      }
+      :host([disabled][link]) paper-button {
+        background-color: transparent;
+      }
+      :host([link]) paper-button:hover,
+      :host([link]) paper-button:focus {
+        color: var(--gr-button-hover-color, var(--color-button-hover));
       }
     </style>
-    <paper-button raised="[[!link]]" disabled="[[disabled]]" tabindex="-1">
+    <paper-button
+        raised="[[!link]]"
+        disabled="[[_computeDisabled(disabled, loading)]]"
+        tabindex="-1">
       <content></content>
       <i class="downArrow"></i>
     </paper-button>
   </template>
   <script src="gr-button.js"></script>
-</dom-module>
\ No newline at end of file
+</dom-module>
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 d552eb6825..b5a1d94467 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -27,6 +27,11 @@
         value: false,
         reflectToAttribute: true,
       },
+      loading: {
+        type: Boolean,
+        value: false,
+        reflectToAttribute: true,
+      },
       tertiary: {
         type: Boolean,
         value: false,
@@ -53,6 +58,10 @@
       keydown: '_handleKeydown',
     },
 
+    observers: [
+      '_computeDisabled(disabled, loading)',
+    ],
+
     behaviors: [
       Gerrit.KeyboardShortcutBehavior,
       Gerrit.TooltipBehavior,
@@ -77,6 +86,10 @@
       this.setAttribute('tabindex', disabled ? '-1' : this._enabledTabindex);
     },
 
+    _computeDisabled(disabled, loading) {
+      return disabled || loading;
+    },
+
     _handleKeydown(e) {
       if (this.modifierPressed(e)) { return; }
       e = this.getKeyboardEvent(e);
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
index d78427bbca..c0ceb33ff5 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
@@ -51,6 +51,16 @@ limitations under the License.
       sandbox.restore();
     });
 
+    test('disabled is set by disabled or loading', () => {
+      assert.isFalse(element.$$('paper-button').disabled);
+      element.disabled = true;
+      assert.isTrue(element.$$('paper-button').disabled);
+      element.disabled = false;
+      assert.isFalse(element.$$('paper-button').disabled);
+      element.loading = true;
+      assert.isTrue(element.$$('paper-button').disabled);
+    });
+
     for (const eventName of ['tap', 'click']) {
       test('dispatches ' + eventName + ' event', () => {
         const spy = addSpyOn(eventName);