Skip to content

Commit 1725324

Browse files
committed
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 #4614. Fixes #9231. Fixes #9634
1 parent c720198 commit 1725324

File tree

8 files changed

+115
-67
lines changed

8 files changed

+115
-67
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: 21 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,37 @@
7676
$foreground: map-get($theme, foreground);
7777

7878
.mat-button, .mat-icon-button, .mat-stroked-button {
79-
background: transparent;
80-
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.
8779
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');
80+
background: transparent;
9281

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-
}
82+
@include _mat-button-theme-property($theme, 'color', default);
83+
@include _mat-button-focus-overlay-color($theme);
9684

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);
85+
// Setup the ripple color to be based on the color palette. The opacity can be a bit weaker
86+
// than for icon-buttons, because normal and stroked buttons have a focus overlay.
87+
@include _mat-button-ripple-color($theme, default);
10088
}
10189

102-
.mat-flat-button {
103-
// Default properties when not using any [color] value.
90+
.mat-flat-button, .mat-raised-button, .mat-fab, .mat-mini-fab {
91+
// Default font and background color when not using any color palette.
10492
color: mat-color($foreground, text);
105-
10693
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');
10994

110-
// Add ripple effect with contrast color to buttons that don't have a focus overlay.
95+
@include _mat-button-theme-property($theme, 'color', default-contrast);
96+
@include _mat-button-theme-property($theme, 'background-color', default);
11197
@include _mat-button-ripple-color($theme, default-contrast);
11298
}
11399

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.
100+
// Since icon buttons don't have a focus overlay, the ripple opacity should be the higher
101+
// than the default value.
116102
.mat-icon-button {
117-
@include _mat-button-ripple-color($theme, default);
103+
@include _mat-button-ripple-color($theme, default, 0.2);
118104
}
119105
}
120106

121107
@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 {
108+
.mat-button, .mat-raised-button, .mat-icon-button, .mat-stroked-button,
109+
.mat-flat-button, .mat-fab, .mat-mini-fab {
124110
font: {
125111
family: mat-font-family($config, button);
126112
size: mat-font-size($config, button);

src/lib/button/button.scss

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,6 @@
6767
}
6868
}
6969

70-
// The text and icon should be vertical aligned inside a button
71-
.mat-button, .mat-raised-button, .mat-icon-button, .mat-fab, .mat-mini-fab {
72-
color: currentColor;
73-
.mat-button-wrapper > * {
74-
vertical-align: middle;
75-
}
76-
}
77-
7870
// The ripple container should match the bounds of the entire button.
7971
.mat-button-ripple, .mat-button-focus-overlay {
8072
@include mat-fill;
@@ -111,6 +103,14 @@
111103
z-index: 1;
112104
}
113105

106+
// Elements inside of all type of buttons should be vertical aligned in the middle.
107+
.mat-button, .mat-flat-button, .mat-stroked-button, .mat-raised-button, .mat-icon-button,
108+
.mat-fab, .mat-mini-fab {
109+
.mat-button-wrapper > * {
110+
vertical-align: middle;
111+
}
112+
}
113+
114114
// Add an outline to make it more visible in high contrast mode.
115115
@include cdk-high-contrast {
116116
.mat-button, .mat-raised-button, .mat-icon-button, .mat-fab, .mat-mini-fab {

src/lib/button/button.spec.ts

Lines changed: 26 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', () => {
@@ -41,6 +41,30 @@ describe('MatButton', () => {
4141
expect(aDebugElement.nativeElement.classList).not.toContain('mat-accent');
4242
});
4343

44+
it('should mark buttons without a background color and theme as plain buttons', () => {
45+
const fixture = TestBed.createComponent(TestApp);
46+
const buttonDebugEl = fixture.debugElement.query(By.css('button'));
47+
const anchorDebugEl = fixture.debugElement.query(By.css('a'));
48+
const fabDebugEl = fixture.debugElement.query(By.css('[mat-fab]'));
49+
50+
fixture.detectChanges();
51+
52+
// Buttons that have no background color and theme palette are considered as plain buttons.
53+
expect(buttonDebugEl.nativeElement.classList).toContain('mat-plain-button');
54+
expect(anchorDebugEl.nativeElement.classList).toContain('mat-plain-button');
55+
expect(fabDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
56+
57+
fixture.componentInstance.buttonColor = 'primary';
58+
fixture.detectChanges();
59+
60+
// Buttons that have no background color, but use an explicit theme palette, are not
61+
// considered as plain buttons.
62+
expect(buttonDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
63+
expect(anchorDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
64+
expect(fabDebugEl.nativeElement.classList).not.toContain('mat-plain-button');
65+
});
66+
67+
4468
it('should expose the ripple instance', () => {
4569
const fixture = TestBed.createComponent(TestApp);
4670
const button = fixture.debugElement.query(By.css('button')).componentInstance as MatButton;
@@ -259,6 +283,7 @@ class TestApp {
259283
clickCount: number = 0;
260284
isDisabled: boolean = false;
261285
rippleDisabled: boolean = false;
286+
buttonColor: ThemePalette;
262287

263288
increment() {
264289
this.clickCount++;

src/lib/button/button.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export const _MatButtonMixinBase = mixinColor(mixinDisabled(mixinDisableRipple(M
6565
exportAs: 'matButton',
6666
host: {
6767
'[disabled]': 'disabled || null',
68+
'[class.mat-button-plain]': '_isPlainButton()',
6869
},
6970
templateUrl: 'button.html',
7071
styleUrls: ['button.css'],
@@ -76,12 +77,18 @@ export const _MatButtonMixinBase = mixinColor(mixinDisabled(mixinDisableRipple(M
7677
export class MatButton extends _MatButtonMixinBase
7778
implements OnDestroy, CanDisable, CanColor, CanDisableRipple {
7879

80+
/** Whether the button is a normal button. */
81+
_isNormalButton: boolean = this._hasHostAttributes('mat-button');
82+
7983
/** Whether the button is round. */
8084
_isRoundButton: boolean = this._hasHostAttributes('mat-fab', 'mat-mini-fab');
8185

8286
/** Whether the button is icon button. */
8387
_isIconButton: boolean = this._hasHostAttributes('mat-icon-button');
8488

89+
/** Whether the button is a stroked button. */
90+
_isStrokedButton: boolean = this._hasHostAttributes('mat-stroked-button');
91+
8592
/** Reference to the MatRipple instance of the button. */
8693
@ViewChild(MatRipple) ripple: MatRipple;
8794

@@ -124,15 +131,16 @@ export class MatButton extends _MatButtonMixinBase
124131
return this.disableRipple || this.disabled;
125132
}
126133

134+
/**
135+
* Whether the button is a plain button. Plain buttons are buttons without a background
136+
* color and theme color set.
137+
*/
138+
_isPlainButton() {
139+
return !this.color && (this._isIconButton || this._isNormalButton || this._isStrokedButton);
140+
}
141+
127142
/** Gets whether the button has one of the given attributes. */
128143
_hasHostAttributes(...attributes: string[]) {
129-
// If not on the browser, say that there are none of the attributes present.
130-
// Since these only affect how the ripple displays (and ripples only happen on the client),
131-
// detecting these attributes isn't necessary when not on the browser.
132-
if (!this._platform.isBrowser) {
133-
return false;
134-
}
135-
136144
return attributes.some(attribute => this._getHostElement().hasAttribute(attribute));
137145
}
138146
}
@@ -149,6 +157,7 @@ export class MatButton extends _MatButtonMixinBase
149157
'[attr.tabindex]': 'disabled ? -1 : 0',
150158
'[attr.disabled]': 'disabled || null',
151159
'[attr.aria-disabled]': 'disabled.toString()',
160+
'[class.mat-button-plain]': '_isPlainButton()',
152161
'(click)': '_haltDisabledEvents($event)',
153162
},
154163
inputs: ['disabled', 'disableRipple', 'color'],
@@ -159,10 +168,8 @@ export class MatButton extends _MatButtonMixinBase
159168
changeDetection: ChangeDetectionStrategy.OnPush,
160169
})
161170
export class MatAnchor extends MatButton {
162-
constructor(
163-
platform: Platform,
164-
focusMonitor: FocusMonitor,
165-
elementRef: ElementRef) {
171+
172+
constructor(platform: Platform, focusMonitor: FocusMonitor, elementRef: ElementRef) {
166173
super(elementRef, platform, focusMonitor);
167174
}
168175

src/lib/toolbar/toolbar.scss

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ $mat-toolbar-row-padding: 16px !default;
3030
// Per Material specs a toolbar cannot have multiple lines inside of a single row.
3131
// Disable text wrapping inside of the toolbar. Developers are still able to overwrite it.
3232
white-space: nowrap;
33+
34+
// Plain buttons (buttons without a background color and color palette) should inherit the color
35+
// from the toolbar row, because otherwise the text will be unreadable on themed toolbars.
36+
.mat-button-plain {
37+
color: inherit;
38+
}
3339
}
3440

3541
.mat-toolbar-multiple-rows {

0 commit comments

Comments
 (0)