-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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.
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
fix(button): no color set for flat and icon buttons #9536
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
<p> | ||
<mat-toolbar> | ||
<mat-icon class="demo-toolbar-icon">menu</mat-icon> | ||
<span>Default Toolbar</span> | ||
<button mat-icon-button> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
src/lib/button/button.scss
Outdated
.mat-button-wrapper > * { | ||
vertical-align: middle; | ||
} | ||
} | ||
|
||
// Buttons that don't have a background color can inherit the color from its parent. | ||
.mat-button, .mat-icon-button { | ||
color: inherit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inherit
or currentColor
? Is inherit
supported on IE11?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, just confirmed that inherited
is supported.
* Currently for flat 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. * Flat 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) Fixes angular#4614. Fixes angular#9231. Fixes angular#9634
05ba139
to
0806b29
Compare
@crisbeto Updated to what we discussed. Please have a look. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
<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)Fixes #4614. Fixes #9231. Fixes #9634