Skip to content

fix(button): no color set for flat and icon buttons #9536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix(button): no color set for flat and icon buttons #9536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions src/demo-app/toolbar/toolbar-demo.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,58 @@
<div class="demo-toolbar">

<p>
<mat-toolbar>
<mat-icon class="demo-toolbar-icon">menu</mat-icon>
<span>Default Toolbar</span>
<button mat-icon-button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we want to keep one mat-icon without button in the demo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I have such plain <mat-icon> elements in the demo toolbars below.

<mat-icon>menu</mat-icon>
</button>

<span>Default Toolbar</span>
<span class="demo-fill-remaining"></span>

<mat-icon>code</mat-icon>
<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>
</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-raised-button>Text</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>
Expand Down
3 changes: 3 additions & 0 deletions src/demo-app/toolbar/toolbar-demo.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@
flex: 1 1 auto;
}

button {
margin: 0 4px;
}
}
3 changes: 2 additions & 1 deletion src/lib/button/_button-theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@
$foreground: map-get($theme, foreground);

.mat-button, .mat-icon-button {
background: transparent;
color: mat-color($foreground, text);
background-color: transparent;

@include _mat-button-focus-color($theme);
@include _mat-button-theme-color($theme, 'color');
Expand Down
1 change: 0 additions & 1 deletion src/lib/button/button.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,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;
}
Expand Down
26 changes: 25 additions & 1 deletion src/lib/button/button.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -41,6 +41,29 @@ 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;
Expand Down Expand Up @@ -259,6 +282,7 @@ class TestApp {
clickCount: number = 0;
isDisabled: boolean = false;
rippleDisabled: boolean = false;
buttonColor: ThemePalette;

increment() {
this.clickCount++;
Expand Down
13 changes: 13 additions & 0 deletions src/lib/button/button.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export const _MatButtonMixinBase = mixinColor(mixinDisabled(mixinDisableRipple(M
exportAs: 'matButton',
host: {
'[disabled]': 'disabled || null',
'[class.mat-plain-button]': '_isPlainButton()',
},
templateUrl: 'button.html',
styleUrls: ['button.css'],
Expand All @@ -120,6 +121,9 @@ export class MatButton extends _MatButtonMixinBase
/** Whether the button is icon button. */
_isIconButton: boolean = this._hasHostAttributes('mat-icon-button');

/** Whether the button is a flat button. */
_isFlatButton: boolean = this._hasHostAttributes('mat-button');

/** Reference to the MatRipple instance of the button. */
@ViewChild(MatRipple) ripple: MatRipple;

Expand Down Expand Up @@ -152,6 +156,14 @@ 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._isFlatButton);
}

/** 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.
Expand All @@ -176,6 +188,7 @@ export class MatButton extends _MatButtonMixinBase
'[attr.tabindex]': 'disabled ? -1 : 0',
'[attr.disabled]': 'disabled || null',
'[attr.aria-disabled]': 'disabled.toString()',
'[class.mat-plain-button]': '_isPlainButton()',
'(click)': '_haltDisabledEvents($event)',
},
inputs: ['disabled', 'disableRipple', 'color'],
Expand Down
6 changes: 6 additions & 0 deletions src/lib/toolbar/toolbar.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-plain-button {
color: inherit;
}
}

.mat-toolbar-multiple-rows {
Expand Down