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
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 9be55cf..483882f 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 @@ <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 @@ :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, var(--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, var(--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, var(--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 @@ 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,20 +71,56 @@ :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]]" + disabled="[[_computeDisabled(disabled, loading)]]" tabindex="-1"> <content></content> <i class="downArrow"></i>
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 8e66e11..f368c8e 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, }, + loading: { + type: Boolean, + value: false, + reflectToAttribute: true, + }, tertiary: { type: Boolean, value: false, @@ -54,6 +59,10 @@ keydown: '_handleKeydown', }, + observers: [ + '_computeDisabled(disabled, loading)', + ], + behaviors: [ Gerrit.KeyboardShortcutBehavior, Gerrit.TooltipBehavior, @@ -78,6 +87,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 d78427b..c0ceb33 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 @@ 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);