fix(chips): Fix choice-chips leading icon being hidden #2796
Conversation
Add some if statements to not remove the leading icon when selected. Change the color of the leading icon when selected. Change the tests to ensure it is checking the right chips.
Add test for selected chip with leading icon and no checkmark to not remove the leading icon.
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
Codecov Report
@@ Coverage Diff @@
## master #2796 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 98 98
Lines 4182 4182
Branches 532 532
=======================================
Hits 4115 4115
Misses 67 67Continue to review full report at Codecov.
|
| @@ -115,7 +115,7 @@ | |||
|
|
|||
| // Add leading checkmark to filter chips with a leading icon | |||
|
|
|||
| .mdc-chip__icon--leading { | |||
| .mdc-chip-set--filter .mdc-chip__icon--leading { | |||
There was a problem hiding this comment.
I think it'll be more readable if you put all these inside a single .mdc-chip-set--filter selector block like this:
.mdc-chip-set--filter {
.mdc-chip__icon--leading {
...
}
...
}
packages/mdc-chips/chip/adapter.js
Outdated
| * Returns true if contains the checkmark class. | ||
| * @return {boolean} | ||
| */ | ||
| hasCheckmark() {} |
There was a problem hiding this comment.
Just realized...if we're restricting the opacity changes to filter chips in CSS, then we don't need to check for checkmarks in the JS anymore :D
| @@ -161,7 +161,7 @@ class MDCChipFoundation extends MDCFoundation { | |||
| this.adapter_.hasClass(cssClasses.SELECTED)) { | |||
| this.adapter_.addClassToLeadingIcon(cssClasses.HIDDEN_LEADING_ICON); | |||
| } else if (this.adapter_.eventTargetHasClass(/** @type {!EventTarget} */ (evt.target), cssClasses.CHECKMARK) && | |||
| !this.adapter_.hasClass(cssClasses.SELECTED)) { | |||
| !this.adapter_.hasClass(cssClasses.SELECTED)) { | |||
Fixes #2728
Add a boolean to not remove the leading icon when selected chip does not contain a checkmark.
Change the color of the leading icon when selected. Change the tests to ensure it is checking the right chips. Add test for selected chip with leading icon and no checkmark, to not
remove the leading icon.