fix(button): no color set for normal, stroked and icon buttons

* Currently for normal buttons, stroked buttons and icon buttons, the font color is not set by the theme. This can cause the text to be invisible on those buttons in a dark theme. From now on, those buttons will also receive a font color by the theme. * Normal buttons, stroked buttons and icon buttons inside of a `<mat-toolbar>` will inherit the font color from the toolbar row. This ensures that those buttons are looking as expected in themed toolbars (e.g. primary, accent, warn) * Removes the SSR check for `hasHostAttributes`. This method is essential for the color of the buttons, and needs to run inside of SSR. (now possible with Domino anyway) * Cleans up the button theme while being at it Fixes angular#4614. Fixes angular#9231. Fixes angular#9634
devversion · Feb 3, 2018 · 1725324 · 1725324
1 parent c720198
commit 1725324
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 67 deletions.
diff --git a/src/demo-app/button/button-demo.html b/src/demo-app/button/button-demo.html
@@ -1,7 +1,7 @@
 <div class="demo-button">
   <h4 class="section-header">Buttons</h4>
   <section>
-    <button mat-button>flat</button>
+    <button mat-button>normal</button>
     <button mat-raised-button>raised</button>
     <button mat-stroked-button>stroked</button>
     <button mat-flat-button>flat</button>

diff --git a/src/demo-app/toolbar/toolbar-demo.html b/src/demo-app/toolbar/toolbar-demo.html
@@ -1,40 +1,61 @@
 <div class="demo-toolbar">
-
   <p>
     <mat-toolbar>
-      <mat-icon class="demo-toolbar-icon">menu</mat-icon>
-      <span>Default Toolbar</span>
+      <button mat-icon-button>
+        <mat-icon>menu</mat-icon>
+      </button>
 
+      <span>Default Toolbar</span>
       <span class="demo-fill-remaining"></span>
 
-      <mat-icon>code</mat-icon>
+      <button mat-button>Text</button>
+      <button mat-icon-button>
+        <mat-icon>code</mat-icon>
+      </button>
+
+      <button mat-icon-button color="warn">
+        <mat-icon>code</mat-icon>
+      </button>
     </mat-toolbar>
   </p>
 
   <p>
     <mat-toolbar color="primary">
-      <mat-icon class="demo-toolbar-icon">menu</mat-icon>
-      <span>Primary Toolbar</span>
+      <button mat-icon-button>
+        <mat-icon>menu</mat-icon>
+      </button>
 
+      <span>Primary Toolbar</span>
       <span class="demo-fill-remaining"></span>
 
-      <mat-icon>code</mat-icon>
+      <button mat-raised-button>Text</button>
+      <button mat-raised-button color="accent">Accent</button>
+      <button mat-stroked-button>Stroked</button>
     </mat-toolbar>
   </p>
 
   <p>
     <mat-toolbar color="accent">
-      <mat-icon class="demo-toolbar-icon">menu</mat-icon>
-      <span>Accent Toolbar</span>
+      <button mat-icon-button>
+        <mat-icon>menu</mat-icon>
+      </button>
 
+      <span>Accent Toolbar</span>
       <span class="demo-fill-remaining"></span>
 
-      <mat-icon>code</mat-icon>
+      <button mat-button>Text</button>
+      <button mat-flat-button>Flat</button>
+      <button mat-mini-fab color="">
+        <mat-icon>done</mat-icon>
+      </button>
+      <button mat-mini-fab color="primary">
+        <mat-icon>done</mat-icon>
+      </button>
     </mat-toolbar>
   </p>
 
   <p>
-    <mat-toolbar color="accent">
+    <mat-toolbar>
       <mat-toolbar-row>First Row</mat-toolbar-row>
       <mat-toolbar-row>Second Row</mat-toolbar-row>
     </mat-toolbar>

diff --git a/src/demo-app/toolbar/toolbar-demo.scss b/src/demo-app/toolbar/toolbar-demo.scss
@@ -9,4 +9,7 @@
     flex: 1 1 auto;
   }
 
+  button {
+    margin: 0 4px;
+  }
 }
diff --git a/src/lib/button/_button-theme.scss b/src/lib/button/_button-theme.scss
@@ -2,7 +2,7 @@
 @import '../core/typography/typography-utils';
 
 // Applies a focus style to an mat-button element for each of the supported palettes.
-@mixin _mat-button-focus-color($theme) {
+@mixin _mat-button-focus-overlay-color($theme) {
   $primary: map-get($theme, primary);
   $accent: map-get($theme, accent);
   $warn: map-get($theme, warn);
@@ -24,7 +24,7 @@
   }
 }
 
-@mixin _mat-button-ripple-color($theme, $hue, $opacity: 0.2) {
+@mixin _mat-button-ripple-color($theme, $hue, $opacity: 0.1) {
   $primary: map-get($theme, primary);
   $accent: map-get($theme, accent);
   $warn: map-get($theme, warn);
@@ -43,21 +43,21 @@
 }
 
 // Applies a property to an mat-button element for each of the supported palettes.
-@mixin _mat-button-theme-color($theme, $property, $color: 'default') {
+@mixin _mat-button-theme-property($theme, $property, $hue) {
   $primary: map-get($theme, primary);
   $accent: map-get($theme, accent);
   $warn: map-get($theme, warn);
   $background: map-get($theme, background);
   $foreground: map-get($theme, foreground);
 
   &.mat-primary {
-    #{$property}: mat-color($primary, $color);
+    #{$property}: mat-color($primary, $hue);
   }
   &.mat-accent {
-    #{$property}: mat-color($accent, $color);
+    #{$property}: mat-color($accent, $hue);
   }
   &.mat-warn {
-    #{$property}: mat-color($warn, $color);
+    #{$property}: mat-color($warn, $hue);
   }
 
   &.mat-primary, &.mat-accent, &.mat-warn, &[disabled] {
@@ -76,51 +76,37 @@
   $foreground: map-get($theme, foreground);
 
   .mat-button, .mat-icon-button, .mat-stroked-button {
-    background: transparent;
-
-    @include _mat-button-focus-color($theme);
-    @include _mat-button-theme-color($theme, 'color');
-  }
-
-  .mat-raised-button, .mat-fab, .mat-mini-fab {
-    // Default properties when not using any [color] value.
     color: mat-color($foreground, text);
-    background-color: mat-color($background, raised-button);
-
-    @include _mat-button-theme-color($theme, 'color', default-contrast);
-    @include _mat-button-theme-color($theme, 'background-color');
+    background: transparent;
 
-    // Add ripple effect with contrast color to buttons that don't have a focus overlay.
-    @include _mat-button-ripple-color($theme, default-contrast);
-  }
+    @include _mat-button-theme-property($theme, 'color', default);
+    @include _mat-button-focus-overlay-color($theme);
 
-  // Add ripple effect with default color to flat buttons, which also have a focus overlay.
-  .mat-button {
-    @include _mat-button-ripple-color($theme, default, 0.1);
+    // Setup the ripple color to be based on the color palette. The opacity can be a bit weaker
+    // than for icon-buttons, because normal and stroked buttons have a focus overlay.
+    @include _mat-button-ripple-color($theme, default);
   }
 
-  .mat-flat-button {
-    // Default properties when not using any [color] value.
+  .mat-flat-button, .mat-raised-button, .mat-fab, .mat-mini-fab {
+    // Default font and background color when not using any color palette.
     color: mat-color($foreground, text);
-
     background-color: mat-color($background, raised-button);
-    @include _mat-button-theme-color($theme, 'color', default-contrast);
-    @include _mat-button-theme-color($theme, 'background-color');
 
-    // Add ripple effect with contrast color to buttons that don't have a focus overlay.
+    @include _mat-button-theme-property($theme, 'color', default-contrast);
+    @include _mat-button-theme-property($theme, 'background-color', default);
     @include _mat-button-ripple-color($theme, default-contrast);
   }
 
-  // Add ripple effect with default color to the icon button. Ripple color needs to be stronger
-  // since the icon button doesn't have a focus overlay.
+  // Since icon buttons don't have a focus overlay, the ripple opacity should be the higher
+  // than the default value.
   .mat-icon-button {
-    @include _mat-button-ripple-color($theme, default);
+    @include _mat-button-ripple-color($theme, default, 0.2);
   }
 }
 
 @mixin mat-button-typography($config) {
-  .mat-button, .mat-raised-button, .mat-icon-button, .mat-stroked-button, .mat-flat-button,
-  .mat-fab, .mat-mini-fab {
+  .mat-button, .mat-raised-button, .mat-icon-button, .mat-stroked-button,
+  .mat-flat-button, .mat-fab, .mat-mini-fab {
     font: {
       family: mat-font-family($config, button);
       size: mat-font-size($config, button);

diff --git a/src/lib/button/button.scss b/src/lib/button/button.scss
@@ -67,14 +67,6 @@
   }
 }
 
-// The text and icon should be vertical aligned inside a button
-.mat-button, .mat-raised-button, .mat-icon-button, .mat-fab, .mat-mini-fab {
-  color: currentColor;
-  .mat-button-wrapper > * {
-    vertical-align: middle;
-  }
-}
-
 // The ripple container should match the bounds of the entire button.
 .mat-button-ripple, .mat-button-focus-overlay {
   @include mat-fill;
@@ -111,6 +103,14 @@
   z-index: 1;
 }
 
+// Elements inside of all type of buttons should be vertical aligned in the middle.
+.mat-button, .mat-flat-button, .mat-stroked-button, .mat-raised-button, .mat-icon-button,
+.mat-fab, .mat-mini-fab {
+  .mat-button-wrapper > * {
+    vertical-align: middle;
+  }
+}
+
 // Add an outline to make it more visible in high contrast mode.
 @include cdk-high-contrast {
   .mat-button, .mat-raised-button, .mat-icon-button, .mat-fab, .mat-mini-fab {

diff --git a/src/lib/button/button.spec.ts b/src/lib/button/button.spec.ts
@@ -2,7 +2,7 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
 import {Component, DebugElement} from '@angular/core';
 import {By} from '@angular/platform-browser';
 import {MatButtonModule, MatButton} from './index';
-import {MatRipple} from '@angular/material/core';
+import {MatRipple, ThemePalette} from '@angular/material/core';
 
 
 describe('MatButton', () => {
@@ -41,6 +41,30 @@ describe('MatButton', () => {
     expect(aDebugElement.nativeElement.classList).not.toContain('mat-accent');
   });
 
+  it('should mark buttons without a background color and theme as plain buttons', () => {
+    const fixture = TestBed.createComponent(TestApp);
+    const buttonDebugEl = fixture.debugElement.query(By.css('button'));
+    const anchorDebugEl = fixture.debugElement.query(By.css('a'));
+    const fabDebugEl = fixture.debugElement.query(By.css('[mat-fab]'));
+
+    fixture.detectChanges();
+
+    // Buttons that have no background color and theme palette are considered as plain buttons.
+    expect(buttonDebugEl.nativeElement.classList).toContain('mat-plain-button');
+    expect(anchorDebugEl.nativeElement.classList).toContain('mat-plain-button');
+    expect(fabDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
+
+    fixture.componentInstance.buttonColor = 'primary';
+    fixture.detectChanges();
+
+    // Buttons that have no background color, but use an explicit theme palette, are not
+    // considered as plain buttons.
+    expect(buttonDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
+    expect(anchorDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
+    expect(fabDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
+  });
+
+
   it('should expose the ripple instance', () => {
     const fixture = TestBed.createComponent(TestApp);
     const button = fixture.debugElement.query(By.css('button')).componentInstance as MatButton;
@@ -259,6 +283,7 @@ class TestApp {
   clickCount: number = 0;
   isDisabled: boolean = false;
   rippleDisabled: boolean = false;
+  buttonColor: ThemePalette;
 
   increment() {
     this.clickCount++;

diff --git a/src/lib/button/button.ts b/src/lib/button/button.ts
@@ -65,6 +65,7 @@ export const _MatButtonMixinBase = mixinColor(mixinDisabled(mixinDisableRipple(M
   exportAs: 'matButton',
   host: {
     '[disabled]': 'disabled || null',
+    '[class.mat-button-plain]': '_isPlainButton()',
   },
   templateUrl: 'button.html',
   styleUrls: ['button.css'],
@@ -76,12 +77,18 @@ export const _MatButtonMixinBase = mixinColor(mixinDisabled(mixinDisableRipple(M
 export class MatButton extends _MatButtonMixinBase
     implements OnDestroy, CanDisable, CanColor, CanDisableRipple {
 
+  /** Whether the button is a normal button. */
+  _isNormalButton: boolean = this._hasHostAttributes('mat-button');
+
   /** Whether the button is round. */
   _isRoundButton: boolean = this._hasHostAttributes('mat-fab', 'mat-mini-fab');
 
   /** Whether the button is icon button. */
   _isIconButton: boolean = this._hasHostAttributes('mat-icon-button');
 
+  /** Whether the button is a stroked button. */
+  _isStrokedButton: boolean = this._hasHostAttributes('mat-stroked-button');
+
   /** Reference to the MatRipple instance of the button. */
   @ViewChild(MatRipple) ripple: MatRipple;
 
@@ -124,15 +131,16 @@ export class MatButton extends _MatButtonMixinBase
     return this.disableRipple || this.disabled;
   }
 
+  /**
+   * Whether the button is a plain button. Plain buttons are buttons without a background
+   * color and theme color set.
+   */
+  _isPlainButton() {
+    return !this.color && (this._isIconButton || this._isNormalButton || this._isStrokedButton);
+  }
+
   /** Gets whether the button has one of the given attributes. */
   _hasHostAttributes(...attributes: string[]) {
-    // If not on the browser, say that there are none of the attributes present.
-    // Since these only affect how the ripple displays (and ripples only happen on the client),
-    // detecting these attributes isn't necessary when not on the browser.
-    if (!this._platform.isBrowser) {
-      return false;
-    }
-
     return attributes.some(attribute => this._getHostElement().hasAttribute(attribute));
   }
 }
@@ -149,6 +157,7 @@ export class MatButton extends _MatButtonMixinBase
     '[attr.tabindex]': 'disabled ? -1 : 0',
     '[attr.disabled]': 'disabled || null',
     '[attr.aria-disabled]': 'disabled.toString()',
+    '[class.mat-button-plain]': '_isPlainButton()',
     '(click)': '_haltDisabledEvents($event)',
   },
   inputs: ['disabled', 'disableRipple', 'color'],
@@ -159,10 +168,8 @@ export class MatButton extends _MatButtonMixinBase
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class MatAnchor extends MatButton {
-  constructor(
-      platform: Platform,
-      focusMonitor: FocusMonitor,
-      elementRef: ElementRef) {
+
+  constructor(platform: Platform, focusMonitor: FocusMonitor, elementRef: ElementRef) {
     super(elementRef, platform, focusMonitor);
   }
 

diff --git a/src/lib/toolbar/toolbar.scss b/src/lib/toolbar/toolbar.scss
@@ -30,6 +30,12 @@ $mat-toolbar-row-padding: 16px !default;
   // Per Material specs a toolbar cannot have multiple lines inside of a single row.
   // Disable text wrapping inside of the toolbar. Developers are still able to overwrite it.
   white-space: nowrap;
+
+  // Plain buttons (buttons without a background color and color palette) should inherit the color
+  // from the toolbar row, because otherwise the text will be unreadable on themed toolbars.
+  .mat-button-plain {
+    color: inherit;
+  }
 }
 
 .mat-toolbar-multiple-rows {