Skip to content

Commit c3a8d0c

Browse files
devversionandrewseguin
devversion
authored and
andrewseguin
committed
fix(button): theme font color being overwritten (#9771)
* Currently buttons with a background color (e.g. flat buttons, raised buttons, fabs) receive a font color by the theme. This font color is accidentally being overwritten by the normal button CSS that sets the `color` for every button to `inherit`. This can cause the text to be invisible in a dark theme. From now on, those buttons will no longer inherit the font color accidentally. * Normal buttons, stroked buttons and icon buttons will still inherit the font color, because it's wrong to assume that those buttons are always placed inside of containers with the default background color. Otherwise those buttons would be invisible in some containers with a different background color (e.g. in a themed toolbar). * 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 #4614. Fixes #9231. Fixes #9634
1 parent 3c8a498 commit c3a8d0c

File tree

7 files changed

+91
-80
lines changed

7 files changed

+91
-80
lines changed

‎src/demo-app/button/button-demo.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<div class="demo-button">
22
<h4 class="section-header">Buttons</h4>
33
<section>
4-
<button mat-button>flat</button>
4+
<button mat-button>normal</button>
55
<button mat-raised-button>raised</button>
66
<button mat-stroked-button>stroked</button>
77
<button mat-flat-button>flat</button>

‎src/demo-app/toolbar/toolbar-demo.html

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,61 @@
11
<div class="demo-toolbar">
2-
32
<p>
43
<mat-toolbar>
5-
<mat-icon class="demo-toolbar-icon">menu</mat-icon>
6-
<span>Default Toolbar</span>
4+
<button mat-icon-button>
5+
<mat-icon>menu</mat-icon>
6+
</button>
77

8+
<span>Default Toolbar</span>
89
<span class="demo-fill-remaining"></span>
910

10-
<mat-icon>code</mat-icon>
11+
<button mat-button>Text</button>
12+
<button mat-icon-button>
13+
<mat-icon>code</mat-icon>
14+
</button>
15+
16+
<button mat-icon-button color="warn">
17+
<mat-icon>code</mat-icon>
18+
</button>
1119
</mat-toolbar>
1220
</p>
1321

1422
<p>
1523
<mat-toolbar color="primary">
16-
<mat-icon class="demo-toolbar-icon">menu</mat-icon>
17-
<span>Primary Toolbar</span>
24+
<button mat-icon-button>
25+
<mat-icon>menu</mat-icon>
26+
</button>
1827

28+
<span>Primary Toolbar</span>
1929
<span class="demo-fill-remaining"></span>
2030

21-
<mat-icon>code</mat-icon>
31+
<button mat-raised-button>Text</button>
32+
<button mat-raised-button color="accent">Accent</button>
33+
<button mat-stroked-button>Stroked</button>
2234
</mat-toolbar>
2335
</p>
2436

2537
<p>
2638
<mat-toolbar color="accent">
27-
<mat-icon class="demo-toolbar-icon">menu</mat-icon>
28-
<span>Accent Toolbar</span>
39+
<button mat-icon-button>
40+
<mat-icon>menu</mat-icon>
41+
</button>
2942

43+
<span>Accent Toolbar</span>
3044
<span class="demo-fill-remaining"></span>
3145

32-
<mat-icon>code</mat-icon>
46+
<button mat-button>Text</button>
47+
<button mat-flat-button>Flat</button>
48+
<button mat-mini-fab color="">
49+
<mat-icon>done</mat-icon>
50+
</button>
51+
<button mat-mini-fab color="primary">
52+
<mat-icon>done</mat-icon>
53+
</button>
3354
</mat-toolbar>
3455
</p>
3556

3657
<p>
37-
<mat-toolbar color="accent">
58+
<mat-toolbar>
3859
<mat-toolbar-row>First Row</mat-toolbar-row>
3960
<mat-toolbar-row>Second Row</mat-toolbar-row>
4061
</mat-toolbar>

‎src/demo-app/toolbar/toolbar-demo.scss

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,7 @@
99
flex: 1 1 auto;
1010
}
1111

12+
button {
13+
margin: 0 4px;
14+
}
1215
}

‎src/lib/button/_button-theme.scss

Lines changed: 25 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
@import '../core/typography/typography-utils';
33

44
// Applies a focus style to an mat-button element for each of the supported palettes.
5-
@mixin _mat-button-focus-color($theme) {
5+
@mixin _mat-button-focus-overlay-color($theme) {
66
$primary: map-get($theme, primary);
77
$accent: map-get($theme, accent);
88
$warn: map-get($theme, warn);
@@ -24,7 +24,7 @@
2424
}
2525
}
2626

27-
@mixin _mat-button-ripple-color($theme, $hue, $opacity: 0.2) {
27+
@mixin _mat-button-ripple-color($theme, $hue, $opacity: 0.1) {
2828
$primary: map-get($theme, primary);
2929
$accent: map-get($theme, accent);
3030
$warn: map-get($theme, warn);
@@ -43,21 +43,21 @@
4343
}
4444

4545
// Applies a property to an mat-button element for each of the supported palettes.
46-
@mixin _mat-button-theme-color($theme, $property, $color: 'default') {
46+
@mixin _mat-button-theme-property($theme, $property, $hue) {
4747
$primary: map-get($theme, primary);
4848
$accent: map-get($theme, accent);
4949
$warn: map-get($theme, warn);
5050
$background: map-get($theme, background);
5151
$foreground: map-get($theme, foreground);
5252

5353
&.mat-primary {
54-
#{$property}: mat-color($primary, $color);
54+
#{$property}: mat-color($primary, $hue);
5555
}
5656
&.mat-accent {
57-
#{$property}: mat-color($accent, $color);
57+
#{$property}: mat-color($accent, $hue);
5858
}
5959
&.mat-warn {
60-
#{$property}: mat-color($warn, $color);
60+
#{$property}: mat-color($warn, $hue);
6161
}
6262

6363
&.mat-primary, &.mat-accent, &.mat-warn, &[disabled] {
@@ -76,51 +76,41 @@
7676
$foreground: map-get($theme, foreground);
7777

7878
.mat-button, .mat-icon-button, .mat-stroked-button {
79+
// Buttons without a background color should inherit the font color. This is necessary to
80+
// ensure that the button is readable on custom background colors. It's wrong to always assume
81+
// that those buttons are always placed inside of containers with the default background
82+
// color of the theme (e.g. themed toolbars).
83+
color: inherit;
7984
background: transparent;
8085

81-
@include _mat-button-focus-color($theme);
82-
@include _mat-button-theme-color($theme, 'color');
83-
}
84-
85-
.mat-raised-button, .mat-fab, .mat-mini-fab {
86-
// Default properties when not using any [color] value.
87-
color: mat-color($foreground, text);
88-
background-color: mat-color($background, raised-button);
89-
90-
@include _mat-button-theme-color($theme, 'color', default-contrast);
91-
@include _mat-button-theme-color($theme, 'background-color');
86+
@include _mat-button-theme-property($theme, 'color', default);
87+
@include _mat-button-focus-overlay-color($theme);
9288

93-
// Add ripple effect with contrast color to buttons that don't have a focus overlay.
94-
@include _mat-button-ripple-color($theme, default-contrast);
95-
}
96-
97-
// Add ripple effect with default color to flat buttons, which also have a focus overlay.
98-
.mat-button {
99-
@include _mat-button-ripple-color($theme, default, 0.1);
89+
// Setup the ripple color to be based on the color palette. The opacity can be a bit weaker
90+
// than for icon-buttons, because normal and stroked buttons have a focus overlay.
91+
@include _mat-button-ripple-color($theme, default);
10092
}
10193

102-
.mat-flat-button {
103-
// Default properties when not using any [color] value.
94+
.mat-flat-button, .mat-raised-button, .mat-fab, .mat-mini-fab {
95+
// Default font and background color when not using any color palette.
10496
color: mat-color($foreground, text);
105-
10697
background-color: mat-color($background, raised-button);
107-
@include _mat-button-theme-color($theme, 'color', default-contrast);
108-
@include _mat-button-theme-color($theme, 'background-color');
10998

110-
// Add ripple effect with contrast color to buttons that don't have a focus overlay.
99+
@include _mat-button-theme-property($theme, 'color', default-contrast);
100+
@include _mat-button-theme-property($theme, 'background-color', default);
111101
@include _mat-button-ripple-color($theme, default-contrast);
112102
}
113103

114-
// Add ripple effect with default color to the icon button. Ripple color needs to be stronger
115-
// since the icon button doesn't have a focus overlay.
104+
// Since icon buttons don't have a focus overlay, the ripple opacity should be the higher
105+
// than the default value.
116106
.mat-icon-button {
117-
@include _mat-button-ripple-color($theme, default);
107+
@include _mat-button-ripple-color($theme, default, 0.2);
118108
}
119109
}
120110

121111
@mixin mat-button-typography($config) {
122-
.mat-button, .mat-raised-button, .mat-icon-button, .mat-stroked-button, .mat-flat-button,
123-
.mat-fab, .mat-mini-fab {
112+
.mat-button, .mat-raised-button, .mat-icon-button, .mat-stroked-button,
113+
.mat-flat-button, .mat-fab, .mat-mini-fab {
124114
font: {
125115
family: mat-font-family($config, button);
126116
size: mat-font-size($config, button);

‎src/lib/button/button.scss

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -65,27 +65,6 @@
6565
}
6666
}
6767

68-
// Align icon-buttons correctly inside of standard, fill, and outline form-field appearances.
69-
.mat-form-field:not(.mat-form-field-appearance-legacy) {
70-
.mat-form-field-prefix,
71-
.mat-form-field-suffix {
72-
.mat-icon-button {
73-
display: block;
74-
font-size: inherit;
75-
width: 2.5em;
76-
height: 2.5em;
77-
}
78-
}
79-
}
80-
81-
// The text and icon should be vertical aligned inside a button
82-
.mat-button, .mat-raised-button, .mat-icon-button, .mat-fab, .mat-mini-fab {
83-
color: currentColor;
84-
.mat-button-wrapper > * {
85-
vertical-align: middle;
86-
}
87-
}
88-
8968
// The ripple container should match the bounds of the entire button.
9069
.mat-button-ripple, .mat-button-focus-overlay {
9170
@include mat-fill;
@@ -122,6 +101,27 @@
122101
z-index: 1;
123102
}
124103

104+
// Elements inside of all type of buttons should be vertical aligned in the middle.
105+
.mat-button, .mat-flat-button, .mat-stroked-button, .mat-raised-button, .mat-icon-button,
106+
.mat-fab, .mat-mini-fab {
107+
.mat-button-wrapper > * {
108+
vertical-align: middle;
109+
}
110+
}
111+
112+
// Align icon-buttons correctly inside of standard, fill, and outline form-field appearances.
113+
.mat-form-field:not(.mat-form-field-appearance-legacy) {
114+
.mat-form-field-prefix,
115+
.mat-form-field-suffix {
116+
.mat-icon-button {
117+
display: block;
118+
font-size: inherit;
119+
width: 2.5em;
120+
height: 2.5em;
121+
}
122+
}
123+
}
124+
125125
// Add an outline to make buttons more visible in high contrast mode. Stroked buttons
126126
// don't need a special look in high-contrast mode, because those already have an outline.
127127
@include cdk-high-contrast {

‎src/lib/button/button.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {async, ComponentFixture, TestBed} from '@angular/core/testing';
22
import {Component, DebugElement} from '@angular/core';
33
import {By} from '@angular/platform-browser';
44
import {MatButtonModule, MatButton} from './index';
5-
import {MatRipple} from '@angular/material/core';
5+
import {MatRipple, ThemePalette} from '@angular/material/core';
66

77

88
describe('MatButton', () => {
@@ -259,6 +259,7 @@ class TestApp {
259259
clickCount: number = 0;
260260
isDisabled: boolean = false;
261261
rippleDisabled: boolean = false;
262+
buttonColor: ThemePalette;
262263

263264
increment() {
264265
this.clickCount++;

‎src/lib/button/button.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ export class MatButton extends _MatButtonMixinBase
8585
@ViewChild(MatRipple) ripple: MatRipple;
8686

8787
constructor(elementRef: ElementRef,
88+
/**
89+
* @deprecated Platform checks for SSR are no longer needed
90+
* @deletion-target 7.0.0
91+
*/
92+
// tslint:disable-next-line:no-unused-variable
8893
private _platform: Platform,
8994
private _focusMonitor: FocusMonitor) {
9095
super(elementRef);
@@ -125,13 +130,6 @@ export class MatButton extends _MatButtonMixinBase
125130

126131
/** Gets whether the button has one of the given attributes. */
127132
_hasHostAttributes(...attributes: string[]) {
128-
// If not on the browser, say that there are none of the attributes present.
129-
// Since these only affect how the ripple displays (and ripples only happen on the client),
130-
// detecting these attributes isn't necessary when not on the browser.
131-
if (!this._platform.isBrowser) {
132-
return false;
133-
}
134-
135133
return attributes.some(attribute => this._getHostElement().hasAttribute(attribute));
136134
}
137135
}
@@ -157,10 +155,8 @@ export class MatButton extends _MatButtonMixinBase
157155
changeDetection: ChangeDetectionStrategy.OnPush,
158156
})
159157
export class MatAnchor extends MatButton {
160-
constructor(
161-
platform: Platform,
162-
focusMonitor: FocusMonitor,
163-
elementRef: ElementRef) {
158+
159+
constructor(platform: Platform, focusMonitor: FocusMonitor, elementRef: ElementRef) {
164160
super(elementRef, platform, focusMonitor);
165161
}
166162

0 commit comments

Comments
 (0)