feat(button): Add ability to color icons separately from the text #2362
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2362 +/- ##
=======================================
Coverage 98.86% 98.86%
=======================================
Files 101 101
Lines 4151 4151
Branches 531 531
=======================================
Hits 4104 4104
Misses 47 47Continue to review full report at Codecov.
|
demos/button.html
Outdated
| <div> | ||
| <button class="mdc-button mdc-button--unelevated demo-text-color" data-demo-no-js> | ||
| <i class="material-icons mdc-button__icon">favorite</i> | ||
| Big Corner Radius |
There was a problem hiding this comment.
Update the labels on these buttons; these labels were copied from the previous two examples but don't make sense here.
demos/button.html
Outdated
| <fieldset> | ||
| <legend class="mdc-typography--title">Different Color Icons/Text</legend> | ||
| <div> | ||
| <button class="mdc-button mdc-button--unelevated demo-text-color" data-demo-no-js> |
There was a problem hiding this comment.
Should we include a demo for JS-upgraded buttons too?
demos/button.html
Outdated
| @@ -312,11 +312,11 @@ <h1 class="mdc-typography--display2">CSS Only</h1> | |||
| <div> | |||
| <button class="mdc-button mdc-button--unelevated demo-text-color" data-demo-no-js> | |||
| <i class="material-icons mdc-button__icon">favorite</i> | |||
| Big Corner Radius | |||
| Text Color | |||
There was a problem hiding this comment.
Nit: Ink Color to match mixin name and since it's really changing both? (Maybe update the classname also?)
packages/mdc-button/README.md
Outdated
| `mdc-button-stroke-width` | Sets the stroke width to the given number (defaults to 2px) | ||
| `mdc-button-container-fill-color` | Sets the container color to the given color. | ||
| `mdc-button-icon-color` | Sets the icon color to the given color. | ||
| `mdc-button-ink-color` | Sets the ink color to the given color. |
There was a problem hiding this comment.
Might want to add that "This affects both text and icon, unless mdc-button-icon-color is also used."
demos/button.scss
Outdated
|
|
||
| .demo-text-color { | ||
| @include mdc-button-icon-color(white); | ||
| @include mdc-button-ink-color(orange); |
There was a problem hiding this comment.
Should we also be applying mdc-states(orange) here? (I'm assuming that should still prioritize matching text over matching icon? Maybe we should ask design, if text/icon can be different colors, which color the "ink" and therefore the ripple is?)
demos/button.html
Outdated
| @@ -234,6 +234,20 @@ <h1 class="mdc-typography--display2">Ripple Enabled</h1> | |||
| </div> | |||
| </fieldset> | |||
|
|
|||
| <fieldset> | |||
| <legend class="mdc-typography--title">Different Color Icons/Text</legend> | |||
There was a problem hiding this comment.
Also replace Text w/ Ink here
demos/button.html
Outdated
| @@ -363,6 +377,20 @@ <h1 class="mdc-typography--display2">CSS Only</h1> | |||
| </button> | |||
| </div> | |||
| </fieldset> | |||
|
|
|||
| <fieldset> | |||
| <legend class="mdc-typography--title">Different Color Icons/Text</legend> | |||
williamernest
force-pushed
the
feat/button/color-text-icon-separately
branch
from
72d3035 to
1c52ac8
Compare
Fixes: #2335
I added the
mdc-button-icon-colormixin. The other mixin wasn't needed, since the icons use the text colors by default.